diff --git a/deploy/crds/crd-clusterissuers.yaml b/deploy/crds/crd-clusterissuers.yaml index 91b8f3d82..a500e82f9 100644 --- a/deploy/crds/crd-clusterissuers.yaml +++ b/deploy/crds/crd-clusterissuers.yaml @@ -1156,11 +1156,11 @@ spec: description: 'Name of the resource being referred to. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' type: string caBundle: - description: PEM-encoded CA bundle (base64-encoded) used to validate Vault server certificate. Only used if the Server URL is using HTTPS protocol. This parameter is ignored for plain HTTP protocol connection. If not set the system root certificates are used to validate the TLS connection. Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, the cert-manager controller system root certificates are used to validate the TLS connection. + description: Base64-encoded bundle of PEM CAs which will be used to validate the certificate chain presented by Vault. Only used if using HTTPS to connect to Vault and ignored for HTTP connections. Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in the cert-manager controller container is used to validate the TLS connection. type: string format: byte caBundleSecretRef: - description: CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when connecting to Vault when using HTTPS. Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager controller system root certificates are used to validate the TLS connection. If no key for the Secret is specified, cert-manager will default to 'ca.crt'. + description: Reference to a Secret containing a bundle of PEM-encoded CAs to use when verifying the certificate chain presented by Vault when using HTTPS. Mutually exclusive with CABundle. If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in the cert-manager controller container is used to validate the TLS connection. If no key for the Secret is specified, cert-manager will default to 'ca.crt'. type: object required: - name @@ -1215,7 +1215,7 @@ spec: - url properties: caBundle: - description: CABundle is a PEM encoded TLS certificate to use to verify connections to the TPP instance. If specified, system roots will not be used and the issuing CA for the TPP instance must be verifiable using the provided root. If not specified, the connection will be verified using the cert-manager system root certificates. + description: Base64-encoded bundle of PEM CAs which will be used to validate the certificate chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. If undefined, the certificate bundle in the cert-manager controller container is used to validate the chain. type: string format: byte credentialsRef: diff --git a/deploy/crds/crd-issuers.yaml b/deploy/crds/crd-issuers.yaml index 1fe2570d2..bf1f83483 100644 --- a/deploy/crds/crd-issuers.yaml +++ b/deploy/crds/crd-issuers.yaml @@ -1156,11 +1156,11 @@ spec: description: 'Name of the resource being referred to. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' type: string caBundle: - description: PEM-encoded CA bundle (base64-encoded) used to validate Vault server certificate. Only used if the Server URL is using HTTPS protocol. This parameter is ignored for plain HTTP protocol connection. If not set the system root certificates are used to validate the TLS connection. Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, the cert-manager controller system root certificates are used to validate the TLS connection. + description: Base64-encoded bundle of PEM CAs which will be used to validate the certificate chain presented by Vault. Only used if using HTTPS to connect to Vault and ignored for HTTP connections. Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in the cert-manager controller container is used to validate the TLS connection. type: string format: byte caBundleSecretRef: - description: CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when connecting to Vault when using HTTPS. Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager controller system root certificates are used to validate the TLS connection. If no key for the Secret is specified, cert-manager will default to 'ca.crt'. + description: Reference to a Secret containing a bundle of PEM-encoded CAs to use when verifying the certificate chain presented by Vault when using HTTPS. Mutually exclusive with CABundle. If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in the cert-manager controller container is used to validate the TLS connection. If no key for the Secret is specified, cert-manager will default to 'ca.crt'. type: object required: - name @@ -1215,7 +1215,7 @@ spec: - url properties: caBundle: - description: CABundle is a PEM encoded TLS certificate to use to verify connections to the TPP instance. If specified, system roots will not be used and the issuing CA for the TPP instance must be verifiable using the provided root. If not specified, the connection will be verified using the cert-manager system root certificates. + description: Base64-encoded bundle of PEM CAs which will be used to validate the certificate chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. If undefined, the certificate bundle in the cert-manager controller container is used to validate the chain. type: string format: byte credentialsRef: diff --git a/internal/apis/certmanager/types_issuer.go b/internal/apis/certmanager/types_issuer.go index 786a52d14..606b74413 100644 --- a/internal/apis/certmanager/types_issuer.go +++ b/internal/apis/certmanager/types_issuer.go @@ -137,12 +137,10 @@ type VenafiTPP struct { // The secret must contain two keys, 'username' and 'password'. CredentialsRef cmmeta.LocalObjectReference - // CABundle is a PEM encoded TLS certificate to use to verify connections to - // the TPP instance. - // If specified, system roots will not be used and the issuing CA for the - // TPP instance must be verifiable using the provided root. - // If not specified, the connection will be verified using the cert-manager - // system root certificates. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. + // If undefined, the certificate bundle in the cert-manager controller container + // is used to validate the chain. CABundle []byte } @@ -182,19 +180,20 @@ type VaultIssuer struct { // More about namespaces can be found here https://www.vaultproject.io/docs/enterprise/namespaces Namespace string - // PEM-encoded CA bundle (base64-encoded) used to validate Vault server - // certificate. Only used if the Server URL is using HTTPS protocol. This - // parameter is ignored for plain HTTP protocol connection. If not set the - // system root certificates are used to validate the TLS connection. - // Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, - // the cert-manager controller system root certificates are used to validate the TLS connection. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by Vault. Only used if using HTTPS to connect to Vault and + // ignored for HTTP connections. + // Mutually exclusive with CABundleSecretRef. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // +optional CABundle []byte - // CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when - // connecting to Vault when using HTTPS. - // Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager - // controller system root certificates are used to validate the TLS connection. + // Reference to a Secret containing a bundle of PEM-encoded CAs to use when + // verifying the certificate chain presented by Vault when using HTTPS. + // Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // If no key for the Secret is specified, cert-manager will default to 'ca.crt'. // +optional CABundleSecretRef *cmmeta.SecretKeySelector diff --git a/internal/apis/certmanager/v1alpha2/types_issuer.go b/internal/apis/certmanager/v1alpha2/types_issuer.go index 90893c1cf..aedc5bd34 100644 --- a/internal/apis/certmanager/v1alpha2/types_issuer.go +++ b/internal/apis/certmanager/v1alpha2/types_issuer.go @@ -150,12 +150,10 @@ type VenafiTPP struct { // The secret must contain two keys, 'username' and 'password'. CredentialsRef cmmeta.LocalObjectReference `json:"credentialsRef"` - // CABundle is a PEM encoded TLS certificate to use to verify connections to - // the TPP instance. - // If specified, system roots will not be used and the issuing CA for the - // TPP instance must be verifiable using the provided root. - // If not specified, the connection will be verified using the cert-manager - // system root certificates. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. + // If undefined, the certificate bundle in the cert-manager controller container + // is used to validate the chain. // +optional CABundle []byte `json:"caBundle,omitempty"` } @@ -199,19 +197,20 @@ type VaultIssuer struct { // +optional Namespace string `json:"namespace,omitempty"` - // PEM-encoded CA bundle (base64-encoded) used to validate Vault server - // certificate. Only used if the Server URL is using HTTPS protocol. This - // parameter is ignored for plain HTTP protocol connection. If not set the - // system root certificates are used to validate the TLS connection. - // Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, - // the cert-manager controller system root certificates are used to validate the TLS connection. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by Vault. Only used if using HTTPS to connect to Vault and + // ignored for HTTP connections. + // Mutually exclusive with CABundleSecretRef. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // +optional CABundle []byte `json:"caBundle,omitempty"` - // CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when - // connecting to Vault when using HTTPS. - // Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager - // controller system root certificates are used to validate the TLS connection. + // Reference to a Secret containing a bundle of PEM-encoded CAs to use when + // verifying the certificate chain presented by Vault when using HTTPS. + // Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // If no key for the Secret is specified, cert-manager will default to 'ca.crt'. // +optional CABundleSecretRef *cmmeta.SecretKeySelector `json:"caBundleSecretRef,omitempty"` diff --git a/internal/apis/certmanager/v1alpha3/types_issuer.go b/internal/apis/certmanager/v1alpha3/types_issuer.go index 29ccd7ac5..ead9f921a 100644 --- a/internal/apis/certmanager/v1alpha3/types_issuer.go +++ b/internal/apis/certmanager/v1alpha3/types_issuer.go @@ -150,12 +150,10 @@ type VenafiTPP struct { // The secret must contain two keys, 'username' and 'password'. CredentialsRef cmmeta.LocalObjectReference `json:"credentialsRef"` - // CABundle is a PEM encoded TLS certificate to use to verify connections to - // the TPP instance. - // If specified, system roots will not be used and the issuing CA for the - // TPP instance must be verifiable using the provided root. - // If not specified, the connection will be verified using the cert-manager - // system root certificates. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. + // If undefined, the certificate bundle in the cert-manager controller container + // is used to validate the chain. // +optional CABundle []byte `json:"caBundle,omitempty"` } @@ -199,19 +197,20 @@ type VaultIssuer struct { // +optional Namespace string `json:"namespace,omitempty"` - // PEM-encoded CA bundle (base64-encoded) used to validate Vault server - // certificate. Only used if the Server URL is using HTTPS protocol. This - // parameter is ignored for plain HTTP protocol connection. If not set the - // system root certificates are used to validate the TLS connection. - // Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, - // the cert-manager controller system root certificates are used to validate the TLS connection. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by Vault. Only used if using HTTPS to connect to Vault and + // ignored for HTTP connections. + // Mutually exclusive with CABundleSecretRef. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // +optional CABundle []byte `json:"caBundle,omitempty"` - // CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when - // connecting to Vault when using HTTPS. - // Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager - // controller system root certificates are used to validate the TLS connection. + // Reference to a Secret containing a bundle of PEM-encoded CAs to use when + // verifying the certificate chain presented by Vault when using HTTPS. + // Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // If no key for the Secret is specified, cert-manager will default to 'ca.crt'. // +optional CABundleSecretRef *cmmeta.SecretKeySelector `json:"caBundleSecretRef,omitempty"` diff --git a/internal/apis/certmanager/v1beta1/types_issuer.go b/internal/apis/certmanager/v1beta1/types_issuer.go index baec4fdca..091ae1adc 100644 --- a/internal/apis/certmanager/v1beta1/types_issuer.go +++ b/internal/apis/certmanager/v1beta1/types_issuer.go @@ -152,12 +152,10 @@ type VenafiTPP struct { // The secret must contain two keys, 'username' and 'password'. CredentialsRef cmmeta.LocalObjectReference `json:"credentialsRef"` - // CABundle is a PEM encoded TLS certificate to use to verify connections to - // the TPP instance. - // If specified, system roots will not be used and the issuing CA for the - // TPP instance must be verifiable using the provided root. - // If not specified, the connection will be verified using the cert-manager - // system root certificates. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. + // If undefined, the certificate bundle in the cert-manager controller container + // is used to validate the chain. // +optional CABundle []byte `json:"caBundle,omitempty"` } @@ -201,19 +199,20 @@ type VaultIssuer struct { // +optional Namespace string `json:"namespace,omitempty"` - // PEM-encoded CA bundle (base64-encoded) used to validate Vault server - // certificate. Only used if the Server URL is using HTTPS protocol. This - // parameter is ignored for plain HTTP protocol connection. If not set the - // system root certificates are used to validate the TLS connection. - // Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, - // the cert-manager controller system root certificates are used to validate the TLS connection. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by Vault. Only used if using HTTPS to connect to Vault and + // ignored for HTTP connections. + // Mutually exclusive with CABundleSecretRef. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // +optional CABundle []byte `json:"caBundle,omitempty"` - // CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when - // connecting to Vault when using HTTPS. - // Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager - // controller system root certificates are used to validate the TLS connection. + // Reference to a Secret containing a bundle of PEM-encoded CAs to use when + // verifying the certificate chain presented by Vault when using HTTPS. + // Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // If no key for the Secret is specified, cert-manager will default to 'ca.crt'. // +optional CABundleSecretRef *cmmeta.SecretKeySelector `json:"caBundleSecretRef,omitempty"` diff --git a/internal/apis/certmanager/validation/issuer.go b/internal/apis/certmanager/validation/issuer.go index 67153fa6a..6db2f5651 100644 --- a/internal/apis/certmanager/validation/issuer.go +++ b/internal/apis/certmanager/validation/issuer.go @@ -226,36 +226,40 @@ func ValidateSelfSignedIssuerConfig(iss *certmanager.SelfSignedIssuer, fldPath * func ValidateVaultIssuerConfig(iss *certmanager.VaultIssuer, fldPath *field.Path) field.ErrorList { el := field.ErrorList{} + if len(iss.Server) == 0 { el = append(el, field.Required(fldPath.Child("server"), "")) } + if len(iss.Path) == 0 { el = append(el, field.Required(fldPath.Child("path"), "")) } - // check if caBundle is valid - certs := iss.CABundle - if len(certs) > 0 { - caCertPool := x509.NewCertPool() - ok := caCertPool.AppendCertsFromPEM(certs) - if !ok { - el = append(el, field.Invalid(fldPath.Child("caBundle"), "", "Specified CA bundle is invalid")) + if len(iss.CABundle) > 0 { + if err := validateCABundleNotEmpty(iss.CABundle); err != nil { + el = append(el, field.Invalid(fldPath.Child("caBundle"), "", err.Error())) } } if len(iss.CABundle) > 0 && iss.CABundleSecretRef != nil { - el = append(el, field.Invalid(fldPath.Child("caBundle"), iss.CABundle, "specified caBundle and caBundleSecretRef cannot be used together")) + // We don't use iss.CABundle for the "value interface{}" argument to field.Invalid for caBundle + // since printing the whole bundle verbatim won't help diagnose any issues + el = append(el, field.Invalid(fldPath.Child("caBundle"), "", "specified caBundle and caBundleSecretRef cannot be used together")) el = append(el, field.Invalid(fldPath.Child("caBundleSecretRef"), iss.CABundleSecretRef.Name, "specified caBundleSecretRef and caBundle cannot be used together")) } - return el // TODO: add validation for Vault authentication types + + return el } func ValidateVenafiTPP(tpp *certmanager.VenafiTPP, fldPath *field.Path) (el field.ErrorList) { if tpp.URL == "" { el = append(el, field.Required(fldPath.Child("url"), "")) } + + // TODO: validate CABundle using validateCABundleNotEmpty + return el } @@ -500,3 +504,22 @@ func ValidateSecretKeySelector(sks *cmmeta.SecretKeySelector, fldPath *field.Pat } return el } + +// validateCABundleNotEmpty performs a soft check on the CA bundle to see if there's at least one +// valid CA certificate inside. +// This uses the standard library crypto/x509.CertPool.AppendCertsFromPEM function, which +// skips over invalid certificates rather than rejecting them. +func validateCABundleNotEmpty(bundle []byte) error { + // TODO: Change this function to actually validate certificates so that invalid certs + // are rejected or at least warned on. + // For example, something like: https://github.com/cert-manager/trust-manager/blob/21c839ff1128990e049eaf23000a9a8d6716c89e/pkg/util/pem.go#L26-L81 + + pool := x509.NewCertPool() + + ok := pool.AppendCertsFromPEM(bundle) + if !ok { + return fmt.Errorf("cert bundle didn't contain any valid certificates") + } + + return nil +} diff --git a/internal/apis/certmanager/validation/issuer_test.go b/internal/apis/certmanager/validation/issuer_test.go index e8df884be..0741567db 100644 --- a/internal/apis/certmanager/validation/issuer_test.go +++ b/internal/apis/certmanager/validation/issuer_test.go @@ -88,7 +88,7 @@ func TestValidateVaultIssuerConfig(t *testing.T) { }, }, errs: []*field.Error{ - field.Invalid(fldPath.Child("caBundle"), caBundle, "specified caBundle and caBundleSecretRef cannot be used together"), + field.Invalid(fldPath.Child("caBundle"), "", "specified caBundle and caBundleSecretRef cannot be used together"), field.Invalid(fldPath.Child("caBundleSecretRef"), "test-secret", "specified caBundleSecretRef and caBundle cannot be used together"), }, }, @@ -102,14 +102,14 @@ func TestValidateVaultIssuerConfig(t *testing.T) { field.Required(fldPath.Child("path"), ""), }, }, - "vault issuer with invalid fields": { + "vault issuer with a CA bundle containing no valid certificates": { spec: &cmapi.VaultIssuer{ Server: "something", Path: "a/b/c", CABundle: []byte("invalid"), }, errs: []*field.Error{ - field.Invalid(fldPath.Child("caBundle"), "", "Specified CA bundle is invalid"), + field.Invalid(fldPath.Child("caBundle"), "", "cert bundle didn't contain any valid certificates"), }, }, } diff --git a/pkg/apis/certmanager/v1/types_issuer.go b/pkg/apis/certmanager/v1/types_issuer.go index 363d66920..6b708fcc4 100644 --- a/pkg/apis/certmanager/v1/types_issuer.go +++ b/pkg/apis/certmanager/v1/types_issuer.go @@ -154,12 +154,10 @@ type VenafiTPP struct { // The secret must contain two keys, 'username' and 'password'. CredentialsRef cmmeta.LocalObjectReference `json:"credentialsRef"` - // CABundle is a PEM encoded TLS certificate to use to verify connections to - // the TPP instance. - // If specified, system roots will not be used and the issuing CA for the - // TPP instance must be verifiable using the provided root. - // If not specified, the connection will be verified using the cert-manager - // system root certificates. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by the TPP server. Only used if using HTTPS; ignored for HTTP. + // If undefined, the certificate bundle in the cert-manager controller container + // is used to validate the chain. // +optional CABundle []byte `json:"caBundle,omitempty"` } @@ -203,19 +201,20 @@ type VaultIssuer struct { // +optional Namespace string `json:"namespace,omitempty"` - // PEM-encoded CA bundle (base64-encoded) used to validate Vault server - // certificate. Only used if the Server URL is using HTTPS protocol. This - // parameter is ignored for plain HTTP protocol connection. If not set the - // system root certificates are used to validate the TLS connection. - // Mutually exclusive with CABundleSecretRef. If neither CABundle nor CABundleSecretRef are defined, - // the cert-manager controller system root certificates are used to validate the TLS connection. + // Base64-encoded bundle of PEM CAs which will be used to validate the certificate + // chain presented by Vault. Only used if using HTTPS to connect to Vault and + // ignored for HTTP connections. + // Mutually exclusive with CABundleSecretRef. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // +optional CABundle []byte `json:"caBundle,omitempty"` - // CABundleSecretRef is a reference to a Secret which contains the CABundle which will be used when - // connecting to Vault when using HTTPS. - // Mutually exclusive with CABundle. If neither CABundleSecretRef nor CABundle are defined, the cert-manager - // controller system root certificates are used to validate the TLS connection. + // Reference to a Secret containing a bundle of PEM-encoded CAs to use when + // verifying the certificate chain presented by Vault when using HTTPS. + // Mutually exclusive with CABundle. + // If neither CABundle nor CABundleSecretRef are defined, the certificate bundle in + // the cert-manager controller container is used to validate the TLS connection. // If no key for the Secret is specified, cert-manager will default to 'ca.crt'. // +optional CABundleSecretRef *cmmeta.SecretKeySelector `json:"caBundleSecretRef,omitempty"` diff --git a/test/e2e/suite/issuers/vault/issuer.go b/test/e2e/suite/issuers/vault/issuer.go index 46ecfad7e..8fc30526c 100644 --- a/test/e2e/suite/issuers/vault/issuer.go +++ b/test/e2e/suite/issuers/vault/issuer.go @@ -233,8 +233,7 @@ var _ = framework.CertManagerDescribe("Vault Issuer", func() { _, err := f.CertManagerClientSet.CertmanagerV1().Issuers(f.Namespace.Name).Create(context.TODO(), vaultIssuer, metav1.CreateOptions{}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(fmt.Sprintf( - "spec.vault.caBundle: Invalid value: %#+v: specified caBundle and caBundleSecretRef cannot be used together", - vault.Details().VaultCA, + "spec.vault.caBundle: Invalid value: \"\": specified caBundle and caBundleSecretRef cannot be used together", ))) Expect(err.Error()).To(ContainSubstring("spec.vault.caBundleSecretRef: Invalid value: \"ca-bundle\": specified caBundleSecretRef and caBundle cannot be used together")) })