Merge #105301 #105559

105301: pgcryptocipher: add helper function for parsing cipher method r=rafiss a=andyyang890

**pgcryptocipher: create new package for pgcrypto cipher functions**

This patch creates a new package that will contain the implementation
of pgcrypto cipher functions, along with related helpers.

Release note: None

----

**pgcryptocipher: add helper function for parsing cipher method**

This patch adds a helper function for parsing the cipher method string
passed to pgcrypto cipher functions.

Release note: None

----

Informs #21001

105559: testccl/sqlccl: unskip TestExplainRedactDDL r=mgartner a=michae2

`TestExplainRedactDDL` is a randomized SQL test which runs variants of `EXPLAIN (REDACT)` on random SQL statements and checks that an injected poison string is always redacted in the output. It is very similar to another randomized test, `TestExplainRedact`, but also includes DDL in the random statements.

During development of v23.1 this test was skipped because the random DDL statements were running into other bugs unrelated to redaction. Now that things are more stable, let's unskip this test.

Fixes: #99005

Epic: None

Release note: None

Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>

pkg/BUILD.bazel

@@ -527,7 +527,7 @@ ALL_TESTS = [
     "//pkg/sql/schemachanger/screl:screl_test",
     "//pkg/sql/schemachanger/scrun:scrun_test",
     "//pkg/sql/schemachanger:schemachanger_test",
-    "//pkg/sql/sem/builtins/pgcrypto:pgcrypto_test",
+    "//pkg/sql/sem/builtins/pgcrypto/pgcryptocipher:pgcryptocipher_test",
     "//pkg/sql/sem/builtins/pgformat:pgformat_test",
     "//pkg/sql/sem/builtins:builtins_disallowed_imports_test",
     "//pkg/sql/sem/builtins:builtins_test",
@@ -1983,8 +1983,8 @@ GO_TARGETS = [
     "//pkg/sql/sem/asof:asof",
     "//pkg/sql/sem/builtins/builtinconstants:builtinconstants",
     "//pkg/sql/sem/builtins/builtinsregistry:builtinsregistry",
-    "//pkg/sql/sem/builtins/pgcrypto:pgcrypto",
-    "//pkg/sql/sem/builtins/pgcrypto:pgcrypto_test",
+    "//pkg/sql/sem/builtins/pgcrypto/pgcryptocipher:pgcryptocipher",
+    "//pkg/sql/sem/builtins/pgcrypto/pgcryptocipher:pgcryptocipher_test",
     "//pkg/sql/sem/builtins/pgformat:pgformat",
     "//pkg/sql/sem/builtins/pgformat:pgformat_test",
     "//pkg/sql/sem/builtins:builtins",

pkg/ccl/testccl/sqlccl/explain_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/lexbase"
 	"github.com/cockroachdb/cockroach/pkg/sql/tests"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
@@ -33,9 +32,6 @@ func TestExplainRedactDDL(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 99005)
-	skip.UnderStressWithIssue(t, 99005)
-
 	const numStatements = 10
 
 	ctx := context.Background()

pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/BUILD.bazel

@@ -0,0 +1,32 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "pgcryptocipher",
+    srcs = [
+        "cipher_method.go",
+        "doc.go",
+        "padding.go",
+    ],
+    importpath = "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/pgcrypto/pgcryptocipher",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/sql/pgwire/pgcode",
+        "//pkg/sql/pgwire/pgerror",
+        "//pkg/util/errorutil/unimplemented",
+        "@com_github_cockroachdb_errors//:errors",
+    ],
+)
+
+go_test(
+    name = "pgcryptocipher_test",
+    srcs = [
+        "cipher_method_test.go",
+        "padding_test.go",
+    ],
+    args = ["-test.timeout=295s"],
+    embed = [":pgcryptocipher"],
+    deps = [
+        "//pkg/util/leaktest",
+        "@com_github_stretchr_testify//require",
+    ],
+)

pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/cipher_method.go

@@ -0,0 +1,84 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package pgcryptocipher
+
+import (
+	"regexp"
+	"strings"
+
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
+)
+
+type cipherAlgorithm int
+
+const (
+	_ cipherAlgorithm = iota
+	aesCipher
+)
+
+type cipherMode int
+
+const (
+	cbcMode cipherMode = iota
+)
+
+type cipherPadding int
+
+const (
+	pkcsPadding cipherPadding = iota
+	noPadding
+)
+
+type cipherMethod struct {
+	algorithm cipherAlgorithm
+	mode      cipherMode
+	padding   cipherPadding
+}
+
+func parseCipherMethod(s string) (cipherMethod, error) {
+	cipherMethodRE := regexp.MustCompile("^(?P<algorithm>[[:alpha:]]+)(?:-(?P<mode>[[:alpha:]]+))?(?:/pad:(?P<padding>[[:alpha:]]+))?$")
+
+	submatches := cipherMethodRE.FindStringSubmatch(s)
+	if submatches == nil {
+		return cipherMethod{}, pgerror.Newf(pgcode.InvalidParameterValue, `cipher method has wrong format: "%s"`, s)
+	}
+
+	ret := cipherMethod{}
+
+	switch algorithm := submatches[cipherMethodRE.SubexpIndex("algorithm")]; strings.ToLower(algorithm) {
+	case "aes":
+		ret.algorithm = aesCipher
+	case "bf":
+		return cipherMethod{}, unimplemented.NewWithIssue(105466, "Blowfish is insecure and not supported")
+	default:
+		return cipherMethod{}, pgerror.Newf(pgcode.InvalidParameterValue, `cipher method has unsupported algorithm: "%s"`, algorithm)
+	}
+
+	switch mode := submatches[cipherMethodRE.SubexpIndex("mode")]; strings.ToLower(mode) {
+	case "", "cbc":
+	case "ecb":
+		return cipherMethod{}, unimplemented.NewWithIssue(105466, "ECB mode is insecure and not supported")
+	default:
+		return cipherMethod{}, pgerror.Newf(pgcode.InvalidParameterValue, `cipher method has unsupported mode: "%s"`, mode)
+	}
+
+	switch padding := submatches[cipherMethodRE.SubexpIndex("padding")]; strings.ToLower(padding) {
+	case "", "pkcs":
+	case "none":
+		ret.padding = noPadding
+	default:
+		return cipherMethod{}, pgerror.Newf(pgcode.InvalidParameterValue, `cipher method has unsupported padding: "%s"`, padding)
+	}
+
+	return ret, nil
+}

pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/cipher_method_test.go

@@ -0,0 +1,80 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package pgcryptocipher
+
+import (
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseCipherMethod(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	// Positive tests
+	for input, expected := range map[string]cipherMethod{
+		"aes": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   pkcsPadding,
+		},
+		"aes/pad:pkcs": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   pkcsPadding,
+		},
+		"aes/pad:none": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   noPadding,
+		},
+		"aes-cbc": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   pkcsPadding,
+		},
+		"aes-cbc/pad:pkcs": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   pkcsPadding,
+		},
+		"aes-cbc/pad:none": {
+			algorithm: aesCipher,
+			mode:      cbcMode,
+			padding:   noPadding,
+		},
+	} {
+		t.Run(input, func(t *testing.T) {
+			ct, err := parseCipherMethod(input)
+			require.NoError(t, err)
+			require.Equal(t, expected, ct)
+		})
+	}
+
+	// Negative tests
+	for input, expectedErr := range map[string]string{
+		// Unsupported algorithms and modes
+		"aes-ecb": `unimplemented: ECB mode is insecure and not supported`,
+		"bf":      `unimplemented: Blowfish is insecure and not supported`,
+
+		// Invalid values
+		"aes/pad=pkcs": `cipher method has wrong format: "aes/pad=pkcs"`,
+		"aescbc":       `cipher method has unsupported algorithm: "aescbc"`,
+		"aes-ctr":      `cipher method has unsupported mode: "ctr"`,
+		"aes/pad:zero": `cipher method has unsupported padding: "zero"`,
+	} {
+		t.Run(input, func(t *testing.T) {
+			_, err := parseCipherMethod(input)
+			require.EqualError(t, err, expectedErr)
+		})
+	}
+}

pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/doc.go

@@ -0,0 +1,13 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package pgcryptocipher contains the implementation of pgcrypto
+// cipher functions.
+package pgcryptocipher

pkg/sql/sem/builtins/pgcrypto/padding.go → pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/padding.go

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package pgcrypto
+package pgcryptocipher
 
 import (
 	"bytes"
@@ -17,10 +17,10 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// PKCSPad pads a slice of bytes to a multiple of the given block size
+// pkcsPad pads a slice of bytes to a multiple of the given block size
 // using the process specified in
 // https://datatracker.ietf.org/doc/html/rfc5652#section-6.3.
-func PKCSPad(data []byte, blockSize int) ([]byte, error) {
+func pkcsPad(data []byte, blockSize int) ([]byte, error) {
 	if blockSize <= 0 || blockSize > math.MaxUint8 {
 		return nil, errors.Newf("invalid block size for PKCS padding: %d", blockSize)
 	}
@@ -31,8 +31,8 @@ func PKCSPad(data []byte, blockSize int) ([]byte, error) {
 	return append(data, padding...), nil
 }
 
-// PKCSUnpad removes the padding added by PKCSPad.
-func PKCSUnpad(data []byte) ([]byte, error) {
+// pkcsUnpad removes the padding added by pkcsPad.
+func pkcsUnpad(data []byte) ([]byte, error) {
 	if len(data) == 0 {
 		return nil, errors.New("PKCS-padded data is empty")
 	}

pkg/sql/sem/builtins/pgcrypto/padding_test.go → pkg/sql/sem/builtins/pgcrypto/pgcryptocipher/padding_test.go

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package pgcrypto
+package pgcryptocipher
 
 import (
 	"testing"
@@ -53,7 +53,7 @@ func TestPKCSPad(t *testing.T) {
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
-			actual, err := PKCSPad(tc.data, tc.blockSize)
+			actual, err := pkcsPad(tc.data, tc.blockSize)
 			if tc.expectedErr != "" {
 				require.EqualError(t, err, tc.expectedErr)
 				return
@@ -102,7 +102,7 @@ func TestPKCSUnpad(t *testing.T) {
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
-			actual, err := PKCSUnpad(tc.data)
+			actual, err := pkcsUnpad(tc.data)
 			if tc.expectedErr != "" {
 				require.EqualError(t, err, tc.expectedErr)
 				return