return an error when trying to include all keys of TLS secrets

Signed-off-by: Julio Camarero <[email protected]>
cert-manager · Oct 23, 2024 · 9769df6 · 9769df6
1 parent 2cf654a
commit 9769df6
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 9 deletions.
diff --git a/pkg/bundle/source.go b/pkg/bundle/source.go
@@ -36,6 +36,8 @@ type notFoundError struct{ error }
 
 type selectsNothingError struct{ error }
 
+type invalidSecretSourceError struct{ error }
+
 // bundleData holds the result of a call to buildSourceBundle. It contains the resulting PEM-encoded
 // certificate data from concatenating all the sources together, binary data for any additional formats and
 // any metadata from the sources which needs to be exposed on the Bundle resource's status field.
@@ -207,6 +209,11 @@ func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKey
 			results.Write(data)
 			results.WriteByte('\n')
 		} else if ref.IncludeAllKeys {
+			// This is done to prevent mistakes. All keys should never be included for a TLS secret, since that would include the private key.
+			if secret.Type == corev1.SecretTypeTLS {
+				return "", invalidSecretSourceError{fmt.Errorf("includeAllKeys is not supported for TLS Secrets such as %s/%s", secret.Namespace, secret.Name)}
+			}
+
 			for _, data := range secret.Data {
 				results.Write(data)
 				results.WriteByte('\n')

diff --git a/pkg/bundle/source_test.go b/pkg/bundle/source_test.go
@@ -47,15 +47,17 @@ const (
 
 func Test_buildSourceBundle(t *testing.T) {
 	tests := map[string]struct {
-		sources          []trustapi.BundleSource
-		formats          *trustapi.AdditionalFormats
-		objects          []runtime.Object
-		expData          string
-		expError         bool
-		expNotFoundError bool
-		expJKS           bool
-		expPKCS12        bool
-		expPassword      *string
+		sources                     []trustapi.BundleSource
+		formats                     *trustapi.AdditionalFormats
+		objects                     []runtime.Object
+		expData                     string
+		expError                    bool
+		expNotFoundError            bool
+		expInvalidSecretSourceError bool
+		bool
+		expJKS      bool
+		expPKCS12   bool
+		expPassword *string
 	}{
 		"if no sources defined, should return an error": {
 			objects:          []runtime.Object{},
@@ -209,6 +211,19 @@ func Test_buildSourceBundle(t *testing.T) {
 			expError:         true,
 			expNotFoundError: true,
 		},
+		"if single Secret source of type TLS including all keys, return invalidSecretSourceError": {
+			sources: []trustapi.BundleSource{
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", IncludeAllKeys: true}},
+			},
+			objects: []runtime.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+				Type:       corev1.SecretTypeTLS,
+				Data:       map[string][]byte{"cert-1": []byte(dummy.TestCertificate1), "cert-2": []byte(dummy.TestCertificate2)},
+			}},
+			expData:                     "",
+			expError:                    true,
+			expInvalidSecretSourceError: true,
+		},
 		"if single Secret source referencing single key, return data": {
 			sources: []trustapi.BundleSource{
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
@@ -318,6 +333,29 @@ func Test_buildSourceBundle(t *testing.T) {
 			expError:         false,
 			expNotFoundError: false,
 		},
+		"if selects at least one Secret source of type TLS including all keys, return invalidSecretSourceError": {
+			sources: []trustapi.BundleSource{
+				{Secret: &trustapi.SourceObjectKeySelector{IncludeAllKeys: true, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}},
+			},
+			objects: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: "secret1", Labels: map[string]string{"trust-bundle.certs": "includes"}},
+					Data: map[string][]byte{
+						"cert-1": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2),
+						"cert-3": []byte(dummy.TestCertificate3),
+					},
+				},
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: "secret2", Labels: map[string]string{"trust-bundle.certs": "includes"}},
+					Type:       corev1.SecretTypeTLS,
+					Data: map[string][]byte{
+						"cert-4": []byte(dummy.TestCertificate4),
+					},
+				}},
+			expData:                     "",
+			expError:                    true,
+			expInvalidSecretSourceError: true,
+		},
 		"if has JKS target, return binaryData with encoded JKS": {
 			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
@@ -441,6 +479,9 @@ func Test_buildSourceBundle(t *testing.T) {
 			if errors.As(err, &notFoundError{}) != test.expNotFoundError {
 				t.Errorf("unexpected notFoundError, exp=%t got=%v", test.expNotFoundError, err)
 			}
+			if errors.As(err, &invalidSecretSourceError{}) != test.expInvalidSecretSourceError {
+				t.Errorf("unexpected invalidSecretSourceError, exp=%t got=%v", test.expInvalidSecretSourceError, err)
+			}
 
 			if resolvedBundle.Data.Data != test.expData {
 				t.Errorf("unexpected data, exp=%q got=%q", test.expData, resolvedBundle.Data.Data)