From 5ca7065bda8205b82fa26685d0fb5cb6d7c28a36 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 18 May 2022 10:15:37 -0400 Subject: [PATCH] Warn on empty Subject field for issuers (#15494) * Warn on empty Subject field for issuers When generating a root or signing an intermediate certificate, it is possible to have Vault generate a certificate with an empty Subject. These don't validate in most TLS implementations well, so add a warning. Note that non-Common Name fields could be present to make a non-empty subject, so simply requiring a CommonName isn't strictly the best. For example: $ vault write pki/root/generate/exported common_name="" WARNING! The following warnings were returned from Vault: * This issuer certificate was generated without a Subject; this makes it likely that issuing leaf certs with this certificate will cause TLS validation libraries to reject this certificate. .... Signed-off-by: Alexander Scheel * Add changelog Signed-off-by: Alexander Scheel --- builtin/logical/pki/path_root.go | 26 ++++++++++++++++++++++++++ changelog/15494.txt | 3 +++ 2 files changed, 29 insertions(+) create mode 100644 changelog/15494.txt diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 3cc1f263d..8caa4ddc6 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -165,6 +165,19 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, }, } + if len(parsedBundle.Certificate.RawSubject) <= 2 { + // Strictly a subject is a SEQUENCE of SETs of SEQUENCES. + // + // The outer SEQUENCE is preserved, having byte value 30 00. + // + // Because of the tag and the length encoding each taking up + // at least one byte, it is impossible to have a non-empty + // subject in two or fewer bytes. We're also not here to validate + // our certificate's ASN.1 content, so let's just assume it holds + // and move on. + resp.AddWarning("This issuer certificate was generated without a Subject; this makes it likely that issuing leaf certs with this certificate will cause TLS validation libraries to reject this certificate.") + } + switch format { case "pem": resp.Data["certificate"] = cb.Certificate @@ -342,6 +355,19 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R resp.AddWarning("The expiration time for the signed certificate is after the CA's expiration time. If the new certificate is not treated as a root, validation paths with the certificate past the issuing CA's expiration time will fail.") } + if len(parsedBundle.Certificate.RawSubject) <= 2 { + // Strictly a subject is a SEQUENCE of SETs of SEQUENCES. + // + // The outer SEQUENCE is preserved, having byte value 30 00. + // + // Because of the tag and the length encoding each taking up + // at least one byte, it is impossible to have a non-empty + // subject in two or fewer bytes. We're also not here to validate + // our certificate's ASN.1 content, so let's just assume it holds + // and move on. + resp.AddWarning("This issuer certificate was generated without a Subject; this makes it likely that issuing leaf certs with this certificate will cause TLS validation libraries to reject this certificate.") + } + switch format { case "pem": resp.Data["certificate"] = cb.Certificate diff --git a/changelog/15494.txt b/changelog/15494.txt new file mode 100644 index 000000000..cd2caec68 --- /dev/null +++ b/changelog/15494.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Warn on empty Subject field during issuer generation (root/generate and root/sign-intermediate). +```