Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to configure the NotBefore property of certificates in role api #5325

Merged
merged 6 commits into from
Oct 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,6 @@ func checkCertsAndPrivateKey(keyType string, key crypto.Signer, usage x509.KeyUs
}
}

// 40 seconds since we add 30 second slack for clock skew
if math.Abs(float64(time.Now().Unix()-cert.NotBefore.Unix())) > 40 {
return nil, fmt.Errorf("validity period starts out of range")
}
if !cert.NotBefore.Before(time.Now().Add(-10 * time.Second)) {
return nil, fmt.Errorf("validity period not far enough in the past")
}

if math.Abs(float64(time.Now().Add(validity).Unix()-cert.NotAfter.Unix())) > 20 {
return nil, fmt.Errorf("certificate validity end: %s; expected within 20 seconds of %s", cert.NotAfter.Format(time.RFC3339), time.Now().Add(validity).Format(time.RFC3339))
}
Expand Down Expand Up @@ -976,6 +968,29 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
}
}

getNotBeforeCheck := func(role roleEntry) logicaltest.TestCheckFunc {
var certBundle certutil.CertBundle
return func(resp *logical.Response) error {
err := mapstructure.Decode(resp.Data, &certBundle)
if err != nil {
return err
}
parsedCertBundle, err := certBundle.ToParsedCertBundle()
if err != nil {
return fmt.Errorf("error checking generated certificate: %s", err)
}
cert := parsedCertBundle.Certificate

actualDiff := time.Now().Sub(cert.NotBefore)
certRoleDiff := role.NotBeforeDuration - actualDiff
// These times get truncated, so give a 1 second buffer on each side
if certRoleDiff > -1*time.Second && certRoleDiff < 1*time.Second {
return nil
}
return fmt.Errorf("validity period out of range diff: %v", certRoleDiff)
}
}

// Returns a TestCheckFunc that performs various validity checks on the
// returned certificate information, mostly within checkCertsAndPrivateKey
getCnCheck := func(name string, role roleEntry, key crypto.Signer, usage x509.KeyUsage, extUsage x509.ExtKeyUsage, validity time.Duration) logicaltest.TestCheckFunc {
Expand Down Expand Up @@ -1324,6 +1339,16 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
roleVals.PostalCode = []string{"f00", "b4r"}
addTests(getPostalCodeCheck(roleVals))
}
// NotBefore tests
{
roleVals.NotBeforeDuration = 10 * time.Second
addTests(getNotBeforeCheck(roleVals))

roleVals.NotBeforeDuration = 30 * time.Second
addTests(getNotBeforeCheck(roleVals))

roleVals.NotBeforeDuration = 0
}
// IP SAN tests
{
roleVals.UseCSRSANs = true
Expand Down Expand Up @@ -1961,8 +1986,8 @@ func TestBackend_SignSelfIssued(t *testing.T) {
Subject: pkix.Name{
CommonName: "foo.bar.com",
},
SerialNumber: big.NewInt(1234),
IsCA: false,
SerialNumber: big.NewInt(1234),
IsCA: false,
BasicConstraintsValid: true,
}

Expand Down Expand Up @@ -1992,8 +2017,8 @@ func TestBackend_SignSelfIssued(t *testing.T) {
Subject: pkix.Name{
CommonName: "bar.foo.com",
},
SerialNumber: big.NewInt(2345),
IsCA: true,
SerialNumber: big.NewInt(2345),
IsCA: true,
BasicConstraintsValid: true,
}
ss, ssCert := getSelfSigned(template, issuer)
Expand Down Expand Up @@ -2573,10 +2598,9 @@ func setCerts() {
DNSNames: []string{"root.localhost"},
KeyUsage: x509.KeyUsage(x509.KeyUsageCertSign | x509.KeyUsageCRLSign),
SerialNumber: big.NewInt(mathrand.Int63()),
NotBefore: time.Now().Add(-30 * time.Second),
NotAfter: time.Now().Add(262980 * time.Hour),
BasicConstraintsValid: true,
IsCA: true,
IsCA: true,
}
caBytes, err := x509.CreateCertificate(rand.Reader, caCertTemplate, caCertTemplate, cak.Public(), cak)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion builtin/logical/pki/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ func TestBackend_CA_Steps(t *testing.T) {
DNSNames: []string{"root.localhost"},
KeyUsage: x509.KeyUsage(x509.KeyUsageCertSign | x509.KeyUsageCRLSign),
SerialNumber: big.NewInt(mathrand.Int63()),
NotBefore: time.Now().Add(-30 * time.Second),
NotAfter: time.Now().Add(262980 * time.Hour),
BasicConstraintsValid: true,
IsCA: true,
Expand Down
12 changes: 10 additions & 2 deletions builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ type creationParameters struct {

// The maximum path length to encode
MaxPathLength int

// The duration the certificate will use NotBefore
NotBeforeDuration time.Duration
}

type caInfoBundle struct {
Expand Down Expand Up @@ -1019,6 +1022,7 @@ func generateCreationBundle(b *backend, data *dataBundle) error {
ExtKeyUsageOIDs: data.role.ExtKeyUsageOIDs,
PolicyIdentifiers: data.role.PolicyIdentifiers,
BasicConstraintsValidForNonCA: data.role.BasicConstraintsValidForNonCA,
NotBeforeDuration: data.role.NotBeforeDuration,
}

// Don't deal with URLs or max path length if it's self-signed, as these
Expand Down Expand Up @@ -1165,7 +1169,6 @@ func createCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {

certTemplate := &x509.Certificate{
SerialNumber: serialNumber,
NotBefore: time.Now().Add(-30 * time.Second),
NotAfter: data.params.NotAfter,
IsCA: false,
SubjectKeyId: subjKeyID,
Expand All @@ -1175,6 +1178,9 @@ func createCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {
IPAddresses: data.params.IPAddresses,
URIs: data.params.URIs,
}
if data.params.NotBeforeDuration > 0 {
certTemplate.NotBefore = time.Now().Add(-1 * data.params.NotBeforeDuration)
}

if err := handleOtherSANs(certTemplate, data.params.OtherSANs); err != nil {
return nil, errutil.InternalError{Err: errwrap.Wrapf("error marshaling other SANs: {{err}}", err).Error()}
Expand Down Expand Up @@ -1365,11 +1371,13 @@ func signCertificate(data *dataBundle) (*certutil.ParsedCertBundle, error) {
certTemplate := &x509.Certificate{
SerialNumber: serialNumber,
Subject: data.params.Subject,
NotBefore: time.Now().Add(-30 * time.Second),
NotAfter: data.params.NotAfter,
SubjectKeyId: subjKeyID[:],
AuthorityKeyId: caCert.SubjectKeyId,
}
if data.params.NotBeforeDuration > 0 {
certTemplate.NotBefore = time.Now().Add(-1 * data.params.NotBeforeDuration)
}

switch data.signingBundle.PrivateKeyType {
case certutil.RSAPrivateKey:
Expand Down
8 changes: 8 additions & 0 deletions builtin/logical/pki/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@ for "generate_lease".`,
Type: framework.TypeBool,
Description: `Mark Basic Constraints valid when issuing non-CA certificates.`,
},
"not_before_duration": &framework.FieldSchema{
Type: framework.TypeDurationSecond,
Default: 30,
chrishoffman marked this conversation as resolved.
Show resolved Hide resolved
Description: `The duration before now the cert needs to be created / signed.`,
},
},

Callbacks: map[logical.Operation]framework.OperationFunc{
Expand Down Expand Up @@ -493,6 +498,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
AllowedSerialNumbers: data.Get("allowed_serial_numbers").([]string),
PolicyIdentifiers: data.Get("policy_identifiers").([]string),
BasicConstraintsValidForNonCA: data.Get("basic_constraints_valid_for_non_ca").(bool),
NotBeforeDuration: time.Duration(data.Get("not_before_duration").(int)) * time.Second,
}

otherSANs := data.Get("allowed_other_sans").([]string)
Expand Down Expand Up @@ -683,6 +689,7 @@ type roleEntry struct {
PolicyIdentifiers []string `json:"policy_identifiers" mapstructure:"policy_identifiers"`
ExtKeyUsageOIDs []string `json:"ext_key_usage_oids" mapstructure:"ext_key_usage_oids"`
BasicConstraintsValidForNonCA bool `json:"basic_constraints_valid_for_non_ca" mapstructure:"basic_constraints_valid_for_non_ca"`
NotBeforeDuration time.Duration `json:"not_before_duration" mapstructure:"not_before_duration"`

// Used internally for signing intermediates
AllowExpirationPastCA bool
Expand Down Expand Up @@ -726,6 +733,7 @@ func (r *roleEntry) ToResponseData() map[string]interface{} {
"require_cn": r.RequireCN,
"policy_identifiers": r.PolicyIdentifiers,
"basic_constraints_valid_for_non_ca": r.BasicConstraintsValidForNonCA,
"not_before_duration": int64(r.NotBeforeDuration.Seconds()),
}
if r.MaxPathLength != nil {
responseData["max_path_length"] = r.MaxPathLength
Expand Down
6 changes: 6 additions & 0 deletions ui/app/models/role-pki.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ export default DS.Model.extend({
basicConstraintsValidForNonCA: attr('boolean', {
label: 'Mark Basic Constraints valid when issuing non-CA certificates.',
}),
notBeforeDuration: attr({
label: 'Not Before Duration',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to have this line as it will "humanize" the attribute name, but fine to leave it in.

editType: 'ttl',
defaultValue: '30s',
}),

updatePath: lazyCapabilities(apiPath`${'backend'}/roles/${'id'}`, 'backend', 'id'),
canDelete: computed.alias('updatePath.canDelete'),
Expand Down Expand Up @@ -137,6 +142,7 @@ export default DS.Model.extend({
'organization',
'keyUsage',
'allowedOtherSans',
'notBeforeDuration',
],
},
{
Expand Down
4 changes: 3 additions & 1 deletion website/source/api/secret/pki/index.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -883,11 +883,13 @@ request is denied.
optional while generating a certificate.

- `policy_identifiers` `(list: [])` – A comma-separated string or list of policy
oids.
OIDs.

- `basic_constraints_valid_for_non_ca` `(bool: false)` - Mark Basic Constraints
valid when issuing non-CA certificates.

- `not_before_duration` `(duration: "30s")` – Specifies the duration by which to backdate the NotBefore property.


### Sample Payload

Expand Down