Skip to content

Commit

Permalink
Use TypeDurationSecond for TTL values in PKI.
Browse files Browse the repository at this point in the history
This has the side effect of simplifying logic quite a bit.

This also fixes/changes parseutil.ParseDurationSecond to return a
duration of 0 if the input is a string but empty.
  • Loading branch information
jefferai committed Aug 31, 2017
1 parent e54a3db commit 822ab79
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 36 deletions.
28 changes: 25 additions & 3 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ func checkCertsAndPrivateKey(keyType string, key crypto.Signer, usage x509.KeyUs
}

if math.Abs(float64(time.Now().Add(validity).Unix()-cert.NotAfter.Unix())) > 10 {
return nil, fmt.Errorf("Validity period of %d too large vs max of 10", cert.NotAfter.Unix())
return nil, fmt.Errorf("Certificate validity end: %s; expected within 10 seconds of %s", cert.NotAfter.Format(time.RFC3339), time.Now().Add(validity).Format(time.RFC3339))
}

return parsedCertBundle, nil
Expand Down Expand Up @@ -1845,6 +1845,8 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
addTests(nil)

roleTestStep.ErrorOk = false
roleVals.TTL = ""
roleVals.MaxTTL = "12h"
}

// Listing test
Expand Down Expand Up @@ -2126,12 +2128,31 @@ func TestBackend_SignVerbatim(t *testing.T) {
"ttl": "12h",
},
})
if resp != nil && !resp.IsError() {
t.Fatalf("sign-verbatim signed too-large-ttl'd CSR: %#v", *resp)
if err != nil {
t.Fatal(err)
}
if resp != nil && resp.IsError() {
t.Fatalf(resp.Error().Error())
}
if resp.Data == nil || resp.Data["certificate"] == nil {
t.Fatal("did not get expected data")
}
certString := resp.Data["certificate"].(string)
block, _ := pem.Decode([]byte(certString))
if block == nil {
t.Fatal("nil pem block")
}
certs, err := x509.ParseCertificates(block.Bytes)
if err != nil {
t.Fatal(err)
}
if len(certs) != 1 {
t.Fatalf("expected a single cert, got %d", len(certs))
}
cert := certs[0]
if math.Abs(float64(time.Now().Add(12*time.Hour).Unix()-cert.NotAfter.Unix())) < 10 {
t.Fatalf("sign-verbatim did not properly cap validiaty period on signed CSR")
}

// now check that if we set generate-lease it takes it from the role and the TTLs match
roleData = map[string]interface{}{
Expand Down Expand Up @@ -2326,6 +2347,7 @@ func TestBackend_Permitted_DNS_Domains(t *testing.T) {

// Direct issuing from root
_, err = client.Logical().Write("root/root/generate/internal", map[string]interface{}{
"ttl": "40h",
"common_name": "myvault.com",
"permitted_dns_domains": []string{"foobar.com", ".zipzap.com"},
})
Expand Down
4 changes: 3 additions & 1 deletion builtin/logical/pki/ca_util.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package pki

import (
"time"

"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)
Expand All @@ -27,7 +29,7 @@ func (b *backend) getGenerationParams(
}

role = &roleEntry{
TTL: data.Get("ttl").(string),
TTL: (time.Duration(data.Get("ttl").(int)) * time.Second).String(),
KeyType: data.Get("key_type").(string),
KeyBits: data.Get("key_bits").(int),
AllowLocalhost: true,
Expand Down
42 changes: 16 additions & 26 deletions builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,48 +728,38 @@ func generateCreationBundle(b *backend,
}

// Get the TTL and very it against the max allowed
var ttlField string
var ttl time.Duration
var maxTTL time.Duration
var notAfter time.Time
var ttlFieldInt interface{}
{
ttlFieldInt, ok = data.GetOk("ttl")
if !ok {
ttlField = role.TTL
} else {
ttlField = ttlFieldInt.(string)
}
ttl = time.Duration(data.Get("ttl").(int)) * time.Second

if len(ttlField) == 0 {
ttl = b.System().DefaultLeaseTTL()
} else {
ttl, err = parseutil.ParseDurationSecond(ttlField)
if err != nil {
return nil, errutil.UserError{Err: fmt.Sprintf(
"invalid requested ttl: %s", err)}
if ttl == 0 {
if role.TTL != "" {
ttl, err = parseutil.ParseDurationSecond(role.TTL)
if err != nil {
return nil, errutil.UserError{Err: fmt.Sprintf(
"invalid requested ttl: %s", err)}
}
}
}

if len(role.MaxTTL) == 0 {
maxTTL = b.System().MaxLeaseTTL()
} else {
if role.MaxTTL != "" {
maxTTL, err = parseutil.ParseDurationSecond(role.MaxTTL)
if err != nil {
return nil, errutil.UserError{Err: fmt.Sprintf(
"invalid ttl: %s", err)}
}
}

if ttl == 0 {
ttl = b.System().DefaultLeaseTTL()
}
if maxTTL == 0 {
maxTTL = b.System().MaxLeaseTTL()
}
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, errutil.UserError{Err: fmt.Sprintf(
"ttl is larger than maximum allowed (%d)", maxTTL/time.Second)}
}
ttl = maxTTL
}

notAfter = time.Now().Add(ttl)
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ email addresses.`,
}

fields["ttl"] = &framework.FieldSchema{
Type: framework.TypeString,
Type: framework.TypeDurationSecond,
Description: `The requested Time To Live for the certificate;
sets the expiration date. If not specified
the role default, backend default, or system
Expand Down Expand Up @@ -92,7 +92,7 @@ must still be specified in alt_names or ip_sans.`,
}

fields["ttl"] = &framework.FieldSchema{
Type: framework.TypeString,
Type: framework.TypeDurationSecond,
Description: `The requested Time To Live for the certificate;
sets the expiration date. If not specified
the role default, backend default, or system
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func pathRoles(b *backend) *framework.Path {
},

"ttl": &framework.FieldSchema{
Type: framework.TypeString,
Type: framework.TypeDurationSecond,
Default: "",
Description: `The lease duration if no specific lease duration is
requested. The lease duration controls the expiration
Expand Down Expand Up @@ -383,7 +383,7 @@ func (b *backend) pathRoleCreate(

entry := &roleEntry{
MaxTTL: data.Get("max_ttl").(string),
TTL: data.Get("ttl").(string),
TTL: (time.Duration(data.Get("ttl").(int)) * time.Second).String(),
AllowLocalhost: data.Get("allow_localhost").(bool),
AllowedDomains: data.Get("allowed_domains").(string),
AllowBareDomains: data.Get("allow_bare_domains").(bool),
Expand Down
45 changes: 43 additions & 2 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package pki
import (
"encoding/base64"
"fmt"
"time"

"github.com/hashicorp/vault/helper/errutil"
"github.com/hashicorp/vault/logical"
Expand Down Expand Up @@ -81,6 +82,34 @@ the non-repudiation flag.`,
return ret
}

/*
func pathSignSelfIssued(b *backend) *framework.Path {
ret := &framework.Path{
Pattern: "root/sign-self-issued",
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.pathCASignIntermediate,
},
HelpSynopsis: pathSignSelfIssuedHelpSyn,
HelpDescription: pathSignSelfIssuedHelpDesc,
}
ret.Fields["certificate"] = &framework.FieldSchema{
Type: framework.TypeString,
Default: "",
Description: `PEM-format self-issued certificate to be signed.`,
}
ret.Fields["ttl"] = &framework.FieldSchema{
Type: framework.TypeDurationSecond,
Description: `Time-to-live for the signed certificate. This is not bounded by the lifetime of this root CA.`,
}
return ret
}
*/

func (b *backend) pathCADeleteRoot(
req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
return nil, req.Storage.Delete("config/ca_bundle")
Expand Down Expand Up @@ -214,7 +243,7 @@ func (b *backend) pathCASignIntermediate(
}

role := &roleEntry{
TTL: data.Get("ttl").(string),
TTL: (time.Duration(data.Get("ttl").(int)) * time.Second).String(),
AllowLocalhost: true,
AllowAnyName: true,
AllowIPSANs: true,
Expand Down Expand Up @@ -340,5 +369,17 @@ Issue an intermediate CA certificate based on the provided CSR.
`

const pathSignIntermediateHelpDesc = `
See the API documentation for more information.
see the API documentation for more information.
`

/*
const pathSignSelfIssuedHelpSyn = `
Signs another CA's self-issued certificate.
`
const pathSignSelfIssuedHelpDesc = `
Signs another CA's self-issued certificate. This is most often used for rolling roots; unless you know you need this you probably want to use sign-intermediate instead.
Note that this is a very "god-mode" operation and should be extremely restricted in terms of who is allowed to use it. All values will be taken directly from the incoming certificate and no verification of host names, path lengths, or any other values will be performed.
`
*/
3 changes: 3 additions & 0 deletions helper/parseutil/parseutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ func ParseDurationSecond(in interface{}) (time.Duration, error) {
switch in.(type) {
case string:
inp := in.(string)
if inp == "" {
return time.Duration(0), nil
}
var err error
// Look for a suffix otherwise its a plain second value
if strings.HasSuffix(inp, "s") || strings.HasSuffix(inp, "m") || strings.HasSuffix(inp, "h") {
Expand Down

0 comments on commit 822ab79

Please sign in to comment.