From 7eed5db86fd890e8801cc7e966af6bca8c20667e Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 19 Nov 2015 16:51:27 -0500 Subject: [PATCH] Update documentation, some comments, make code cleaner, and make generated roots be revoked when their TTL is up --- builtin/logical/pki/cert_util.go | 259 ++++++++++-------- builtin/logical/pki/path_intermediate.go | 3 +- builtin/logical/pki/path_issue_sign.go | 6 + builtin/logical/pki/path_root.go | 39 ++- builtin/logical/pki/secret_certs.go | 2 +- website/source/docs/secrets/pki/index.html.md | 15 +- 6 files changed, 187 insertions(+), 137 deletions(-) diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 5bf0307ac2..ee7f403068 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -394,6 +394,9 @@ func signCert(b *backend, return parsedBundle, nil } +// generateCreationBundle is a shared function that reads parameters supplied +// from the various endpoints and generates a creationBundle with the +// parameters that can be used to issue or sign func generateCreationBundle(b *backend, role *roleEntry, signingBundle *caInfoBundle, @@ -401,140 +404,159 @@ func generateCreationBundle(b *backend, req *logical.Request, data *framework.FieldData) (*creationBundle, error) { var err error + var ok bool - // Get the common name(s) + // Get the common name var cn string - if csr != nil { - if role.UseCSRCommonName { - cn = csr.Subject.CommonName + { + if csr != nil { + if role.UseCSRCommonName { + cn = csr.Subject.CommonName + } } - } - if cn == "" { - cn = data.Get("common_name").(string) if cn == "" { - return nil, certutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true`} + cn = data.Get("common_name").(string) + if cn == "" { + return nil, certutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true`} + } } } + // Read in alternate names -- DNS and email addresses dnsNames := []string{} emailAddresses := []string{} - if strings.Contains(cn, "@") { - emailAddresses = append(emailAddresses, cn) - } else { - dnsNames = append(dnsNames, cn) - } - cnAltInt, ok := data.GetOk("alt_names") - if ok { - cnAlt := cnAltInt.(string) - if len(cnAlt) != 0 { - for _, v := range strings.Split(cnAlt, ",") { - if strings.Contains(v, "@") { - emailAddresses = append(emailAddresses, cn) - } else { - dnsNames = append(dnsNames, v) - } - } - } - } - - // Get any IP SANs - ipAddresses := []net.IP{} - ipAltInt, ok := data.GetOk("ip_sans") - if ok { - ipAlt := ipAltInt.(string) - if len(ipAlt) != 0 { - if !role.AllowIPSANs { - return nil, certutil.UserError{Err: fmt.Sprintf( - "IP Subject Alternative Names are not allowed in this role, but was provided %s", ipAlt)} - } - for _, v := range strings.Split(ipAlt, ",") { - parsedIP := net.ParseIP(v) - if parsedIP == nil { - return nil, certutil.UserError{Err: fmt.Sprintf( - "the value '%s' is not a valid IP address", v)} - } - ipAddresses = append(ipAddresses, parsedIP) - } - } - } - - var ttlField string - ttlFieldInt, ok := data.GetOk("ttl") - if !ok { - ttlField = role.TTL - } else { - ttlField = ttlFieldInt.(string) - } - - var ttl time.Duration - if len(ttlField) == 0 { - ttl = b.System().DefaultLeaseTTL() - } else { - ttl, err = time.ParseDuration(ttlField) - if err != nil { - return nil, certutil.UserError{Err: fmt.Sprintf( - "invalid requested ttl: %s", err)} - } - } - - var maxTTL time.Duration - if len(role.MaxTTL) == 0 { - maxTTL = b.System().MaxLeaseTTL() - } else { - maxTTL, err = time.ParseDuration(role.MaxTTL) - if err != nil { - return nil, certutil.UserError{Err: fmt.Sprintf( - "invalid ttl: %s", err)} - } - } - - if ttl > maxTTL { - // Don't error if they were using system defaults, only error if - // they specifically chose a bad TTL - if len(ttlField) == 0 { - ttl = maxTTL + { + if strings.Contains(cn, "@") { + emailAddresses = append(emailAddresses, cn) } else { + dnsNames = append(dnsNames, cn) + } + cnAltInt, ok := data.GetOk("alt_names") + if ok { + cnAlt := cnAltInt.(string) + if len(cnAlt) != 0 { + for _, v := range strings.Split(cnAlt, ",") { + if strings.Contains(v, "@") { + emailAddresses = append(emailAddresses, cn) + } else { + dnsNames = append(dnsNames, v) + } + } + } + } + + // Check for bad email and/or DNS names + badName, err := validateNames(req, dnsNames, role) + if len(badName) != 0 { return nil, certutil.UserError{Err: fmt.Sprintf( - "ttl is larger than maximum allowed (%d)", maxTTL/time.Second)} + "name %s not allowed by this role", badName)} + } else if err != nil { + return nil, certutil.InternalError{Err: fmt.Sprintf( + "error validating name %s: %s", badName, err)} + } + + badName, err = validateNames(req, emailAddresses, role) + if len(badName) != 0 { + return nil, certutil.UserError{Err: fmt.Sprintf( + "email %s not allowed by this role", badName)} + } else if err != nil { + return nil, certutil.InternalError{Err: fmt.Sprintf( + "error validating name %s: %s", badName, err)} } } - if signingBundle != nil && - time.Now().Add(ttl).After(signingBundle.Certificate.NotAfter) { - return nil, certutil.UserError{Err: fmt.Sprintf( - "cannot satisfy request, as TTL is beyond the expiration of the CA certificate")} + // Get and verify any IP SANs + ipAddresses := []net.IP{} + var ipAltInt interface{} + { + ipAltInt, ok = data.GetOk("ip_sans") + if ok { + ipAlt := ipAltInt.(string) + if len(ipAlt) != 0 { + if !role.AllowIPSANs { + return nil, certutil.UserError{Err: fmt.Sprintf( + "IP Subject Alternative Names are not allowed in this role, but was provided %s", ipAlt)} + } + for _, v := range strings.Split(ipAlt, ",") { + parsedIP := net.ParseIP(v) + if parsedIP == nil { + return nil, certutil.UserError{Err: fmt.Sprintf( + "the value '%s' is not a valid IP address", v)} + } + ipAddresses = append(ipAddresses, parsedIP) + } + } + } } - badName, err := validateNames(req, dnsNames, role) - if len(badName) != 0 { - return nil, certutil.UserError{Err: fmt.Sprintf( - "name %s not allowed by this role", badName)} - } else if err != nil { - return nil, certutil.InternalError{Err: fmt.Sprintf( - "error validating name %s: %s", badName, err)} - } - - badName, err = validateNames(req, emailAddresses, role) - if len(badName) != 0 { - return nil, certutil.UserError{Err: fmt.Sprintf( - "email %s not allowed by this role", badName)} - } else if err != nil { - return nil, certutil.InternalError{Err: fmt.Sprintf( - "error validating name %s: %s", badName, err)} + // Get the TTL and very it against the max allowed + var ttlField string + var ttl time.Duration + var maxTTL time.Duration + var ttlFieldInt interface{} + { + ttlFieldInt, ok = data.GetOk("ttl") + if !ok { + ttlField = role.TTL + } else { + ttlField = ttlFieldInt.(string) + } + + if len(ttlField) == 0 { + ttl = b.System().DefaultLeaseTTL() + } else { + ttl, err = time.ParseDuration(ttlField) + if err != nil { + return nil, certutil.UserError{Err: fmt.Sprintf( + "invalid requested ttl: %s", err)} + } + } + + if len(role.MaxTTL) == 0 { + maxTTL = b.System().MaxLeaseTTL() + } else { + maxTTL, err = time.ParseDuration(role.MaxTTL) + if err != nil { + return nil, certutil.UserError{Err: fmt.Sprintf( + "invalid ttl: %s", err)} + } + } + + if ttl > maxTTL { + // Don't error if they were using system defaults, only error if + // they specifically chose a bad TTL + if len(ttlField) == 0 { + ttl = maxTTL + } else { + return nil, certutil.UserError{Err: fmt.Sprintf( + "ttl is larger than maximum allowed (%d)", maxTTL/time.Second)} + } + } + + // If it's not self-signed, verify that the issued certificate won't be + // valid past the lifetime of the CA certificate + if signingBundle != nil && + time.Now().Add(ttl).After(signingBundle.Certificate.NotAfter) { + return nil, certutil.UserError{Err: fmt.Sprintf( + "cannot satisfy request, as TTL is beyond the expiration of the CA certificate")} + } } + // Build up usages var usage certUsage - if role.ServerFlag { - usage = usage | serverUsage - } - if role.ClientFlag { - usage = usage | clientUsage - } - if role.CodeSigningFlag { - usage = usage | codeSigningUsage - } - if role.EmailProtectionFlag { - usage = usage | emailProtectionUsage + { + if role.ServerFlag { + usage = usage | serverUsage + } + if role.ClientFlag { + usage = usage | clientUsage + } + if role.CodeSigningFlag { + usage = usage | codeSigningUsage + } + if role.EmailProtectionFlag { + usage = usage | emailProtectionUsage + } } creationBundle := &creationBundle{ @@ -549,11 +571,18 @@ func generateCreationBundle(b *backend, Usage: usage, } + // Don't deal with URLs or max path length if it's self-signed, as these + // normally come from the signing bundle if signingBundle == nil { return creationBundle, nil } + // This will have been read in from the getURLs function creationBundle.URLs = signingBundle.URLs + + // If the max path length in the role is not nil, it was specified at + // generation time with the max_path_length parameter; otherwise derive it + // from the signing certificate if role.MaxPathLength != nil { creationBundle.MaxPathLength = *role.MaxPathLength } else { diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index c3dee371a8..b4401abe42 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -201,7 +201,7 @@ func (b *backend) pathSetSignedIntermediate( } // For ease of later use, also store just the certificate at a known - // location, plus a fresh CRL + // location entry.Key = "ca" entry.Value = inputBundle.CertificateBytes err = req.Storage.Put(entry) @@ -209,6 +209,7 @@ func (b *backend) pathSetSignedIntermediate( return nil, err } + // Build a fresh CRL err = buildCRL(b, req) return nil, err diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 9bd753a6c2..65b49a8616 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -75,6 +75,8 @@ basic constraints.`, return ret } +// pathIssue issues a certificate and private key from given parameters, +// subject to role restrictions func (b *backend) pathIssue( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { roleName := data.Get("role").(string) @@ -91,6 +93,8 @@ func (b *backend) pathIssue( return b.pathIssueSignCert(req, data, role, false, false) } +// pathSign issues a certificate from a submitted CSR, subject to role +// restrictions func (b *backend) pathSign( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { roleName := data.Get("role").(string) @@ -107,6 +111,8 @@ func (b *backend) pathSign( return b.pathIssueSignCert(req, data, role, true, false) } +// pathSignVerbatim issues a certificate from a submitted CSR, *not* subject to +// role restrictions func (b *backend) pathSignVerbatim( req *logical.Request, data *framework.FieldData) (*logical.Response, error) { diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 0102413dad..bd1f571dd4 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -82,7 +82,6 @@ func (b *backend) pathCAGenerateRoot( role.MaxPathLength = &maxPathLength } - var resp *logical.Response parsedBundle, err := generateCert(b, role, nil, true, req, data) if err != nil { switch err.(type) { @@ -98,17 +97,19 @@ func (b *backend) pathCAGenerateRoot( return nil, fmt.Errorf("error converting raw cert bundle to cert bundle: %s", err) } - resp = &logical.Response{ - Data: map[string]interface{}{ - "serial_number": cb.SerialNumber, + resp := b.Secret(SecretCertsType).Response( + map[string]interface{}{ "expiration": int64(parsedBundle.Certificate.NotAfter.Unix()), + "serial_number": cb.SerialNumber, + "certificate": cb.Certificate, + "issuing_ca": cb.IssuingCA, }, - } + map[string]interface{}{ + "serial_number": cb.SerialNumber, + }) switch format { case "pem": - resp.Data["certificate"] = cb.Certificate - resp.Data["issuing_ca"] = cb.IssuingCA if exported { resp.Data["private_key"] = cb.PrivateKey resp.Data["private_key_type"] = cb.PrivateKeyType @@ -122,6 +123,9 @@ func (b *backend) pathCAGenerateRoot( } } + resp.Secret.TTL = parsedBundle.Certificate.NotAfter.Sub(time.Now()) + + // Store it as the CA bundle entry, err := logical.StorageEntryJSON("config/ca_bundle", cb) if err != nil { return nil, err @@ -131,8 +135,18 @@ func (b *backend) pathCAGenerateRoot( return nil, err } + // Also store it as just the certificate identified by serial number, so it + // can be revoked + err = req.Storage.Put(&logical.StorageEntry{ + Key: "certs/" + cb.SerialNumber, + Value: parsedBundle.CertificateBytes, + }) + if err != nil { + return nil, fmt.Errorf("Unable to store certificate locally") + } + // For ease of later use, also store just the certificate at a known - // location, plus a fresh CRL + // location entry.Key = "ca" entry.Value = parsedBundle.CertificateBytes err = req.Storage.Put(entry) @@ -140,6 +154,7 @@ func (b *backend) pathCAGenerateRoot( return nil, err } + // Build a fresh CRL err = buildCRL(b, req) if err != nil { return nil, err @@ -213,16 +228,14 @@ func (b *backend) pathCASignIntermediate( map[string]interface{}{ "expiration": int64(parsedBundle.Certificate.NotAfter.Unix()), "serial_number": cb.SerialNumber, + "certificate": cb.Certificate, + "issuing_ca": cb.IssuingCA, }, map[string]interface{}{ "serial_number": cb.SerialNumber, }) - switch format { - case "pem": - resp.Data["certificate"] = cb.Certificate - resp.Data["issuing_ca"] = cb.IssuingCA - case "der": + if format == "der" { resp.Data["certificate"] = base64.StdEncoding.EncodeToString(parsedBundle.CertificateBytes) resp.Data["issuing_ca"] = base64.StdEncoding.EncodeToString(parsedBundle.IssuingCABytes) } diff --git a/builtin/logical/pki/secret_certs.go b/builtin/logical/pki/secret_certs.go index 06be0caa52..80ddf390e9 100644 --- a/builtin/logical/pki/secret_certs.go +++ b/builtin/logical/pki/secret_certs.go @@ -33,7 +33,7 @@ reference`, }, DefaultDuration: 168 * time.Hour, - DefaultGracePeriod: 10 * time.Minute, + DefaultGracePeriod: time.Duration(0), Revoke: b.secretCredsRevoke, } diff --git a/website/source/docs/secrets/pki/index.html.md b/website/source/docs/secrets/pki/index.html.md index 3651e86c21..4e581a1421 100644 --- a/website/source/docs/secrets/pki/index.html.md +++ b/website/source/docs/secrets/pki/index.html.md @@ -158,6 +158,8 @@ Now, we generate our root certificate: ```text $ vault write pki/root/generate/internal common_name=myvault.com ttl=87600h Key Value +lease_id pki/root/generate/internal/aa959dd4-467e-e5ff-642b-371add518b40 +lease_duration 315359999 certificate -----BEGIN CERTIFICATE----- MIIDvTCCAqWgAwIBAgIUAsza+fvOw+Xh9ifYQ0gNN0ruuWcwDQYJKoZIhvcNAQEL BQAwFjEUMBIGA1UEAxMLbXl2YXVsdC5jb20wHhcNMTUxMTE5MTYwNDU5WhcNMjUx @@ -1124,7 +1126,6 @@ subpath for interactive help output. - #### DELETE
@@ -1161,10 +1162,10 @@ subpath for interactive help output. overwrite any previously-existing private key and certificate._ If the path ends with `exported`, the private key will be returned in the response; if it is `internal` the private key will not be returned and *cannot be - retrieved later*. Distribution points use the values set via - `config/urls`.

Vault does _not_ revoke this certificate (since - it could not sign the CRL with an expired certificate), however, this - endpoint does honor the maximum mount TTL. + retrieved later*. Distribution points use the values set via `config/urls`. +

As with other issued certificates, Vault will automatically + revoke the generated root at the end of its lease period; the CA + certificate will sign its own CRL.
Method
@@ -1234,9 +1235,9 @@ subpath for interactive help output. ```javascript { - "lease_id": "", + "lease_id": "pki/root/generate/internal/aa959dd4-467e-e5ff-642b-371add518b40", + "lease_duration": 315359999, "renewable": false, - "lease_duration": 21600, "data": { "certificate": "-----BEGIN CERTIFICATE-----\nMIIDzDCCAragAwIBAgIUOd0ukLcjH43TfTHFG9qE0FtlMVgwCwYJKoZIhvcNAQEL\n...\numkqeYeO30g1uYvDuWLXVA==\n-----END CERTIFICATE-----\n", "issuing_ca": "-----BEGIN CERTIFICATE-----\nMIIDzDCCAragAwIBAgIUOd0ukLcjH43TfTHFG9qE0FtlMVgwCwYJKoZIhvcNAQEL\n...\numkqeYeO30g1uYvDuWLXVA==\n-----END CERTIFICATE-----\n",