From e67ccd9e3d71a3e82a970358b032ecc10cc343c2 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 2 Jan 2020 17:48:28 -0800 Subject: [PATCH 01/10] Add fault tolerance against clock skew accross system on TLS certificates. --- authority/config.go | 21 +++++++++++++++++---- authority/provisioner/sign_options.go | 27 +++++++++++++++++++++------ authority/tls.go | 13 ++++++++++--- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/authority/config.go b/authority/config.go index 8cc742e4..75f55a12 100644 --- a/authority/config.go +++ b/authority/config.go @@ -28,6 +28,7 @@ var ( MaxVersion: 1.2, Renegotiation: false, } + defaultBackdate = time.Minute defaultDisableRenewal = false defaultEnableSSHCA = false globalProvisionerClaims = provisioner.Claims{ @@ -65,10 +66,11 @@ type Config struct { // AuthConfig represents the configuration options for the authority. type AuthConfig struct { - Provisioners provisioner.List `json:"provisioners"` - Template *x509util.ASN1DN `json:"template,omitempty"` - Claims *provisioner.Claims `json:"claims,omitempty"` - DisableIssuedAtCheck bool `json:"disableIssuedAtCheck,omitempty"` + Provisioners provisioner.List `json:"provisioners"` + Template *x509util.ASN1DN `json:"template,omitempty"` + Claims *provisioner.Claims `json:"claims,omitempty"` + DisableIssuedAtCheck bool `json:"disableIssuedAtCheck,omitempty"` + Backdate *provisioner.Duration `json:"backdate,omitempty"` } // Validate validates the authority configuration. @@ -91,6 +93,17 @@ func (c *AuthConfig) Validate(audiences provisioner.Audiences) error { if c.Template == nil { c.Template = &x509util.ASN1DN{} } + + if c.Backdate != nil { + if c.Backdate.Duration < 0 { + return errors.New("authority.backdate cannot be less than 0") + } + } else { + c.Backdate = &provisioner.Duration{ + Duration: defaultBackdate, + } + } + return nil } diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index 53921a3c..ddc985e3 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -15,10 +15,12 @@ import ( "golang.org/x/crypto/ed25519" ) -// Options contains the options that can be passed to the Sign method. +// Options contains the options that can be passed to the Sign method. Backdate +// is automatically filled and can only be configured in the CA. type Options struct { - NotAfter TimeDuration `json:"notAfter"` - NotBefore TimeDuration `json:"notBefore"` + NotAfter TimeDuration `json:"notAfter"` + NotBefore TimeDuration `json:"notBefore"` + Backdate time.Duration `json:"-"` } // SignOption is the interface used to collect all extra options used in the @@ -189,12 +191,22 @@ func (v emailAddressesValidator) Valid(req *x509.CertificateRequest) error { type profileDefaultDuration time.Duration func (v profileDefaultDuration) Option(so Options) x509util.WithOption { + var backdate time.Duration notBefore := so.NotBefore.Time() if notBefore.IsZero() { notBefore = time.Now() + backdate = -1 * so.Backdate } notAfter := so.NotAfter.RelativeTime(notBefore) - return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v)) + return func(p x509util.Profile) error { + fn := x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v)) + if err := fn(p); err != nil { + return err + } + crt := p.Subject() + crt.NotBefore = crt.NotBefore.Add(backdate) + return nil + } } // profileLimitDuration is an x509 profile option that modifies an x509 validity @@ -208,10 +220,12 @@ type profileLimitDuration struct { // certificate to one that is superficially imposed. func (v profileLimitDuration) Option(so Options) x509util.WithOption { return func(p x509util.Profile) error { + var backdate time.Duration n := now() notBefore := so.NotBefore.Time() if notBefore.IsZero() { notBefore = n + backdate = -1 * so.Backdate } if notBefore.After(v.notAfter) { return errors.Errorf("provisioning credential expiration (%s) is before "+ @@ -232,7 +246,7 @@ func (v profileLimitDuration) Option(so Options) x509util.WithOption { } } crt := p.Subject() - crt.NotBefore = notBefore + crt.NotBefore = notBefore.Add(backdate) crt.NotAfter = notAfter return nil } @@ -255,9 +269,10 @@ func (v *validityValidator) Valid(crt *x509.Certificate) error { var ( na = crt.NotAfter nb = crt.NotBefore - d = na.Sub(nb) now = time.Now() ) + // Get duration from to not take into account the backdate. + var d = na.Sub(now) if na.Before(now) { return errors.Errorf("NotAfter: %v cannot be in the past", na) diff --git a/authority/tls.go b/authority/tls.go index 0dd4f323..eb7cb86a 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -65,6 +65,10 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti certValidators = []provisioner.CertificateValidator{} issIdentity = a.intermediateIdentity ) + + // Set backdate with the configured value + signOpts.Backdate = a.config.AuthorityConfig.Backdate.Duration + for _, op := range extraOpts { switch k := op.(type) { case provisioner.CertificateValidator: @@ -136,14 +140,17 @@ func (a *Authority) Renew(oldCert *x509.Certificate) ([]*x509.Certificate, error // Issuer issIdentity := a.intermediateIdentity - now := time.Now().UTC() + // Durations + backdate := a.config.AuthorityConfig.Backdate.Duration duration := oldCert.NotAfter.Sub(oldCert.NotBefore) + now := time.Now().UTC() + newCert := &x509.Certificate{ PublicKey: oldCert.PublicKey, Issuer: issIdentity.Crt.Subject, Subject: oldCert.Subject, - NotBefore: now, - NotAfter: now.Add(duration), + NotBefore: now.Add(-1 * backdate), + NotAfter: now.Add(duration - backdate), KeyUsage: oldCert.KeyUsage, UnhandledCriticalExtensions: oldCert.UnhandledCriticalExtensions, ExtKeyUsage: oldCert.ExtKeyUsage, From 50717b3ffa0d50f31b5d6a85c20c9aec9c7b91c4 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 3 Jan 2020 13:27:45 -0800 Subject: [PATCH 02/10] Update assert package. --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 5da04acb..407e6164 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/pkg/errors v0.8.1 github.com/rs/xid v1.2.1 github.com/sirupsen/logrus v1.4.2 - github.com/smallstep/assert v0.0.0-20180720014142-de77670473b5 + github.com/smallstep/assert v0.0.0-20200103212524-b99dc1097b15 github.com/smallstep/cli v0.14.0-rc.1.0.20191218000521-3e7348324838 github.com/smallstep/nosql v0.2.0 github.com/urfave/cli v1.20.1-0.20181029213200-b67dcf995b6a diff --git a/go.sum b/go.sum index 8053807c..c200eeb8 100644 --- a/go.sum +++ b/go.sum @@ -109,6 +109,8 @@ github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/smallstep/assert v0.0.0-20180720014142-de77670473b5 h1:lX6ybsQW9Agn3qK/W1Z39Z4a6RyEMGem/gXUYW0axYk= github.com/smallstep/assert v0.0.0-20180720014142-de77670473b5/go.mod h1:TC9A4+RjIOS+HyTH7wG17/gSqVv95uDw2J64dQZx7RE= +github.com/smallstep/assert v0.0.0-20200103212524-b99dc1097b15 h1:kSImCuenAkXtCaBeQ1UhmzzJGRhSm8sVH7I3sHE2Qdg= +github.com/smallstep/assert v0.0.0-20200103212524-b99dc1097b15/go.mod h1:MyOHs9Po2fbM1LHej6sBUT8ozbxmMOFG+E+rx/GSGuc= github.com/smallstep/certificates v0.14.0-rc.1.0.20191023014154-4669bef8c700/go.mod h1:/WOAB2LkcjkEbKG5rDol+A22Lp3UsttkLPLkY7tVtuk= github.com/smallstep/certificates v0.14.0-rc.1.0.20191025192352-8ef9b020ed24/go.mod h1:043iBnsMvNhQ+QFwSh0N6JR3H2yamHPPAc78vCf+8Tc= github.com/smallstep/certificates v0.14.0-rc.1.0.20191126035953-e88034bea402/go.mod h1:r2UTcAZNriKlwvNNXymNAcF3iKL6mTYOYrOCtBYYGJU= From 76c14560b0dd2c44822e1fdd192ef3a03b8523b9 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 3 Jan 2020 17:41:16 -0800 Subject: [PATCH 03/10] Use errs package for HTTP errors. --- ca/client_test.go | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/ca/client_test.go b/ca/client_test.go index 7a36a64b..c2e0063e 100644 --- a/ca/client_test.go +++ b/ca/client_test.go @@ -16,6 +16,8 @@ import ( "testing" "time" + "github.com/smallstep/certificates/errs" + "github.com/smallstep/assert" "github.com/smallstep/certificates/api" "github.com/smallstep/certificates/authority" @@ -152,8 +154,8 @@ func equalJSON(t *testing.T, a interface{}, b interface{}) bool { func TestClient_Version(t *testing.T) { ok := &api.VersionResponse{Version: "test"} - internal := api.InternalServerError(fmt.Errorf("Internal Server Error")) - notFound := api.NotFound(fmt.Errorf("Not Found")) + internal := errs.InternalServerError(fmt.Errorf("Internal Server Error")) + notFound := errs.NotFound(fmt.Errorf("Not Found")) tests := []struct { name string @@ -207,7 +209,7 @@ func TestClient_Version(t *testing.T) { func TestClient_Health(t *testing.T) { ok := &api.HealthResponse{Status: "ok"} - nok := api.InternalServerError(fmt.Errorf("Internal Server Error")) + nok := errs.InternalServerError(fmt.Errorf("Internal Server Error")) tests := []struct { name string @@ -262,7 +264,7 @@ func TestClient_Root(t *testing.T) { ok := &api.RootResponse{ RootPEM: api.Certificate{Certificate: parseCertificate(rootPEM)}, } - notFound := api.NotFound(fmt.Errorf("Not Found")) + notFound := errs.NotFound(fmt.Errorf("Not Found")) tests := []struct { name string @@ -332,8 +334,8 @@ func TestClient_Sign(t *testing.T) { NotBefore: api.NewTimeDuration(time.Now()), NotAfter: api.NewTimeDuration(time.Now().AddDate(0, 1, 0)), } - unauthorized := api.Unauthorized(fmt.Errorf("Unauthorized")) - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + unauthorized := errs.Unauthorized(fmt.Errorf("Unauthorized")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string @@ -407,8 +409,8 @@ func TestClient_Revoke(t *testing.T) { OTT: "the-ott", ReasonCode: 4, } - unauthorized := api.Unauthorized(fmt.Errorf("Unauthorized")) - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + unauthorized := errs.Unauthorized(fmt.Errorf("Unauthorized")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string @@ -483,8 +485,8 @@ func TestClient_Renew(t *testing.T) { {Certificate: parseCertificate(rootPEM)}, }, } - unauthorized := api.Unauthorized(fmt.Errorf("Unauthorized")) - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + unauthorized := errs.Unauthorized(fmt.Errorf("Unauthorized")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string @@ -541,7 +543,7 @@ func TestClient_Provisioners(t *testing.T) { ok := &api.ProvisionersResponse{ Provisioners: provisioner.List{}, } - internalServerError := api.InternalServerError(fmt.Errorf("Internal Server Error")) + internalServerError := errs.InternalServerError(fmt.Errorf("Internal Server Error")) tests := []struct { name string @@ -603,7 +605,7 @@ func TestClient_ProvisionerKey(t *testing.T) { ok := &api.ProvisionerKeyResponse{ Key: "an encrypted key", } - notFound := api.NotFound(fmt.Errorf("Not Found")) + notFound := errs.NotFound(fmt.Errorf("Not Found")) tests := []struct { name string @@ -664,8 +666,8 @@ func TestClient_Roots(t *testing.T) { {Certificate: parseCertificate(rootPEM)}, }, } - unauthorized := api.Unauthorized(fmt.Errorf("Unauthorized")) - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + unauthorized := errs.Unauthorized(fmt.Errorf("Unauthorized")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string @@ -724,8 +726,8 @@ func TestClient_Federation(t *testing.T) { {Certificate: parseCertificate(rootPEM)}, }, } - unauthorized := api.Unauthorized(fmt.Errorf("Unauthorized")) - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + unauthorized := errs.Unauthorized(fmt.Errorf("Unauthorized")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string @@ -788,7 +790,7 @@ func TestClient_SSHRoots(t *testing.T) { HostKeys: []api.SSHPublicKey{{PublicKey: key}}, UserKeys: []api.SSHPublicKey{{PublicKey: key}}, } - notFound := api.NotFound(fmt.Errorf("Not Found")) + notFound := errs.NotFound(fmt.Errorf("Not Found")) tests := []struct { name string @@ -879,7 +881,7 @@ func Test_parseEndpoint(t *testing.T) { func TestClient_RootFingerprint(t *testing.T) { ok := &api.HealthResponse{Status: "ok"} - nok := api.InternalServerError(fmt.Errorf("Internal Server Error")) + nok := errs.InternalServerError(fmt.Errorf("Internal Server Error")) httpsServer := httptest.NewTLSServer(nil) defer httpsServer.Close() @@ -946,7 +948,7 @@ func TestClient_SSHBastion(t *testing.T) { Hostname: "bastion.local", }, } - badRequest := api.BadRequest(fmt.Errorf("Bad Request")) + badRequest := errs.BadRequest(fmt.Errorf("Bad Request")) tests := []struct { name string From 64e0a2ca6f291c7d52f40b3e3298667bf9b6eba5 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 3 Jan 2020 18:16:45 -0800 Subject: [PATCH 04/10] Disable backdata on ca tests. --- ca/testdata/ca.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ca/testdata/ca.json b/ca/testdata/ca.json index f29f24c6..b094c02e 100644 --- a/ca/testdata/ca.json +++ b/ca/testdata/ca.json @@ -18,6 +18,7 @@ ] }, "authority": { + "backdate": "0s", "provisioners": [ { "name": "max", From 935d0d454244e434941f2f07bedc679ef13b6c71 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 3 Jan 2020 18:22:02 -0800 Subject: [PATCH 05/10] Add support for backdate to SSH certificates. --- authority/provisioner/aws.go | 2 +- authority/provisioner/azure.go | 4 +- authority/provisioner/claims.go | 16 ++ authority/provisioner/claims_test.go | 51 ++++++ authority/provisioner/gcp.go | 2 +- authority/provisioner/jwk.go | 3 +- authority/provisioner/k8sSA.go | 6 +- authority/provisioner/oidc.go | 2 +- authority/provisioner/sign_options.go | 17 +- authority/provisioner/sign_ssh_options.go | 157 +++++++++++------- .../provisioner/sign_ssh_options_test.go | 71 ++++++-- authority/provisioner/utils_test.go | 2 +- authority/provisioner/x5c.go | 3 +- authority/provisioner/x5c_test.go | 4 +- authority/ssh.go | 12 +- 15 files changed, 263 insertions(+), 89 deletions(-) create mode 100644 authority/provisioner/claims_test.go diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index a58ffb7e..74fa3a1f 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -469,7 +469,7 @@ func (p *AWS) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Set the default extensions. &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(p.claimer), + &sshDefaultDuration{p.claimer}, // Validate public key &sshDefaultPublicKeyValidator{}, // Validate the validity period. diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index 5e338e18..998ef6e1 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -209,7 +209,7 @@ func (p *Azure) Init(config Config) (err error) { return nil } -// parseToken returuns the claims, name, group, error. +// parseToken returns the claims, name, group, error. func (p *Azure) parseToken(token string) (*azurePayload, string, string, error) { jwt, err := jose.ParseSigned(token) if err != nil { @@ -335,7 +335,7 @@ func (p *Azure) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio // Set the default extensions. &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(p.claimer), + &sshDefaultDuration{p.claimer}, // Validate public key &sshDefaultPublicKeyValidator{}, // Validate the validity period. diff --git a/authority/provisioner/claims.go b/authority/provisioner/claims.go index 4eba5ad7..997d9ba3 100644 --- a/authority/provisioner/claims.go +++ b/authority/provisioner/claims.go @@ -4,6 +4,7 @@ import ( "time" "github.com/pkg/errors" + "golang.org/x/crypto/ssh" ) // Claims so that individual provisioners can override global claims. @@ -95,6 +96,21 @@ func (c *Claimer) IsDisableRenewal() bool { return *c.claims.DisableRenewal } +// DefaultSSHCertDuration returns the default SSH certificate duration for the +// given certificate type. +func (c *Claimer) DefaultSSHCertDuration(certType uint32) (time.Duration, error) { + switch certType { + case ssh.UserCert: + return c.DefaultUserSSHCertDuration(), nil + case ssh.HostCert: + return c.DefaultHostSSHCertDuration(), nil + case 0: + return 0, errors.New("ssh certificate type has not been set") + default: + return 0, errors.Errorf("ssh certificate has an unknown type: %d", certType) + } +} + // DefaultUserSSHCertDuration returns the default SSH user cert duration for the // provisioner. If the default is not set within the provisioner, then the // global default from the authority configuration will be used. diff --git a/authority/provisioner/claims_test.go b/authority/provisioner/claims_test.go new file mode 100644 index 00000000..d4794d3c --- /dev/null +++ b/authority/provisioner/claims_test.go @@ -0,0 +1,51 @@ +package provisioner + +import ( + "testing" + "time" + + "golang.org/x/crypto/ssh" +) + +func TestClaimer_DefaultSSHCertDuration(t *testing.T) { + duration := Duration{ + Duration: time.Hour, + } + type fields struct { + global Claims + claims *Claims + } + type args struct { + certType uint32 + } + tests := []struct { + name string + fields fields + args args + want time.Duration + wantErr bool + }{ + {"user", fields{globalProvisionerClaims, &Claims{DefaultUserSSHDur: &duration}}, args{1}, time.Hour, false}, + {"user global", fields{globalProvisionerClaims, nil}, args{ssh.UserCert}, 16 * time.Hour, false}, + {"host global", fields{globalProvisionerClaims, &Claims{DefaultHostSSHDur: &duration}}, args{2}, time.Hour, false}, + {"host global", fields{globalProvisionerClaims, nil}, args{ssh.HostCert}, 30 * 24 * time.Hour, false}, + {"invalid", fields{globalProvisionerClaims, nil}, args{0}, 0, true}, + {"invalid global", fields{globalProvisionerClaims, nil}, args{3}, 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Claimer{ + global: tt.fields.global, + claims: tt.fields.claims, + } + got, err := c.DefaultSSHCertDuration(tt.args.certType) + if (err != nil) != tt.wantErr { + t.Errorf("Claimer.DefaultSSHCertDuration() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("Claimer.DefaultSSHCertDuration() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 30a65909..bc531e92 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -378,7 +378,7 @@ func (p *GCP) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Set the default extensions &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(p.claimer), + &sshDefaultDuration{p.claimer}, // Validate public key &sshDefaultPublicKeyValidator{}, // Validate the validity period. diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 231b1580..b5add3f4 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -188,6 +188,7 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, if claims.Step == nil || claims.Step.SSH == nil { return nil, errors.New("authorization token must be an SSH provisioning token") } + opts := claims.Step.SSH signOptions := []SignOption{ // validates user's SSHOptions with the ones in the token @@ -222,7 +223,7 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Set the default extensions. &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(p.claimer), + &sshDefaultDuration{p.claimer}, // Validate that the keyID is equivalent to the token subject. sshCertKeyIDValidator(claims.Subject), // Validate public key diff --git a/authority/provisioner/k8sSA.go b/authority/provisioner/k8sSA.go index 0c90552c..e7d45236 100644 --- a/authority/provisioner/k8sSA.go +++ b/authority/provisioner/k8sSA.go @@ -235,13 +235,15 @@ func (p *K8sSA) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOptio } // Default to a user certificate with no principals if not set - signOptions := []SignOption{sshCertificateDefaultsModifier{CertType: SSHUserCert}} + signOptions := []SignOption{ + sshCertificateDefaultsModifier{CertType: SSHUserCert}, + } return append(signOptions, // Set the default extensions. &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(p.claimer), + &sshDefaultDuration{p.claimer}, // Validate public key &sshDefaultPublicKeyValidator{}, // Validate the validity period. diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 4538ef81..4c4b68d2 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -360,7 +360,7 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption // Set the default extensions &sshDefaultExtensionModifier{}, // Set the validity bounds if not set. - sshDefaultValidityModifier(o.claimer), + &sshDefaultDuration{o.claimer}, // Validate public key &sshDefaultPublicKeyValidator{}, // Validate the validity period. diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index ddc985e3..2c3cfa16 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -267,12 +267,19 @@ func newValidityValidator(min, max time.Duration) *validityValidator { // and total duration. func (v *validityValidator) Valid(crt *x509.Certificate) error { var ( - na = crt.NotAfter - nb = crt.NotBefore - now = time.Now() + na = crt.NotAfter.Truncate(time.Second) + nb = crt.NotBefore.Truncate(time.Second) + now = time.Now().Truncate(time.Second) ) - // Get duration from to not take into account the backdate. - var d = na.Sub(now) + + // To not take into account the backdate, time.Now() will be used to + // calculate the duration if NotBefore is in the past. + var d time.Duration + if now.After(nb) { + d = na.Sub(now) + } else { + d = na.Sub(nb) + } if na.Before(now) { return errors.Errorf("NotAfter: %v cannot be in the past", na) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index ceb57105..f5ad8662 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -46,13 +46,22 @@ type SSHCertificateOptionsValidator interface { Valid(got SSHOptions) error } +// sshModifierFunc is an adapter to allow the use of ordinary functions as SSH +// certificate modifiers. +type sshModifierFunc func(cert *ssh.Certificate) error + +func (f sshModifierFunc) Modify(cert *ssh.Certificate) error { + return f(cert) +} + // SSHOptions contains the options that can be passed to the SignSSH method. type SSHOptions struct { - CertType string `json:"certType"` - KeyID string `json:"keyID"` - Principals []string `json:"principals"` - ValidAfter TimeDuration `json:"validAfter,omitempty"` - ValidBefore TimeDuration `json:"validBefore,omitempty"` + CertType string `json:"certType"` + KeyID string `json:"keyID"` + Principals []string `json:"principals"` + ValidAfter TimeDuration `json:"validAfter,omitempty"` + ValidBefore TimeDuration `json:"validBefore,omitempty"` + Backdate time.Duration `json:"-"` } // Type returns the uint32 representation of the CertType. @@ -199,67 +208,92 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { } } -// sshValidityModifier is an SSHCertificateModifier that checks the -// validity bounds, setting them if they are not provided. It will fail if a +// sshDefaultDuration is an SSHCertificateModifier that sets the certificate +// ValidAfter and ValidBefore if they have not been set. It will fail if a // CertType has not been set or is not valid. -type sshValidityModifier struct { +type sshDefaultDuration struct { *Claimer - validBefore time.Time } -func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error { - var d time.Duration - - switch cert.CertType { - case ssh.UserCert: - d = m.DefaultUserSSHCertDuration() - case ssh.HostCert: - d = m.DefaultHostSSHCertDuration() - case 0: - return errors.New("ssh certificate type has not been set") - default: - return errors.Errorf("unknown ssh certificate type %d", cert.CertType) - } - - hasLimit := !m.validBefore.IsZero() - - n := now() - if cert.ValidAfter == 0 { - cert.ValidAfter = uint64(n.Truncate(time.Second).Unix()) - } - certValidAfter := time.Unix(int64(cert.ValidAfter), 0) - if hasLimit && certValidAfter.After(m.validBefore) { - return errors.Errorf("provisioning credential expiration (%s) is before "+ - "requested certificate validAfter (%s)", m.validBefore, certValidAfter) - } - - if cert.ValidBefore == 0 { - certValidBefore := certValidAfter.Add(d) - if hasLimit && m.validBefore.Before(certValidBefore) { - certValidBefore = m.validBefore +func (m *sshDefaultDuration) Option(o SSHOptions) SSHCertificateModifier { + return sshModifierFunc(func(cert *ssh.Certificate) error { + d, err := m.DefaultSSHCertDuration(cert.CertType) + if err != nil { + return err } - cert.ValidBefore = uint64(certValidBefore.Unix()) - } else if hasLimit { - certValidBefore := time.Unix(int64(cert.ValidBefore), 0) - if m.validBefore.Before(certValidBefore) { - return errors.Errorf("provisioning credential expiration (%s) is before "+ - "requested certificate validBefore (%s)", m.validBefore, certValidBefore) + + var backdate uint64 + if cert.ValidAfter == 0 { + backdate = uint64(o.Backdate / time.Second) + cert.ValidAfter = uint64(now().Truncate(time.Second).Unix()) } + if cert.ValidBefore == 0 { + cert.ValidBefore = cert.ValidAfter + uint64(d/time.Second) + } + // Apply backdate safely + if cert.ValidAfter > backdate { + cert.ValidAfter -= backdate + } + return nil + }) +} + +// sshLimitDuration adjusts the duration to min(default, remaining provisioning +// credential duration). E.g. if the default is 12hrs but the remaining validity +// of the provisioning credential is only 4hrs, this option will set the value +// to 4hrs (the min of the two values). It will fail if a CertType has not been +// set or is not valid. +type sshLimitDuration struct { + *Claimer + NotAfter time.Time +} + +func (m *sshLimitDuration) Option(o SSHOptions) SSHCertificateModifier { + if m.NotAfter.IsZero() { + defaultDuration := &sshDefaultDuration{m.Claimer} + return defaultDuration.Option(o) } - return nil -} + return sshModifierFunc(func(cert *ssh.Certificate) error { + d, err := m.DefaultSSHCertDuration(cert.CertType) + if err != nil { + return err + } -func sshDefaultValidityModifier(c *Claimer) SSHCertificateModifier { - return &sshValidityModifier{c, time.Time{}} -} + var backdate uint64 + if cert.ValidAfter == 0 { + backdate = uint64(o.Backdate / time.Second) + cert.ValidAfter = uint64(now().Truncate(time.Second).Unix()) + } -// sshLimitValidityModifier adjusts the duration to -// min(default, remaining provisioning credential duration). -// E.g. if the default is 12hrs but the remaining validity of the provisioning -// credential is only 4hrs, this option will set the value to 4hrs (the min of the two values). -func sshLimitValidityModifier(c *Claimer, validBefore time.Time) SSHCertificateModifier { - return &sshValidityModifier{c, validBefore} + certValidAfter := time.Unix(int64(cert.ValidAfter), 0) + if certValidAfter.After(m.NotAfter) { + return errors.Errorf("provisioning credential expiration (%s) is before requested certificate validAfter (%s)", + m.NotAfter, certValidAfter) + } + + if cert.ValidBefore == 0 { + certValidBefore := certValidAfter.Add(d) + if m.NotAfter.Before(certValidBefore) { + certValidBefore = m.NotAfter + println(2, certValidBefore.String()) + } + cert.ValidBefore = uint64(certValidBefore.Unix()) + } else { + certValidBefore := time.Unix(int64(cert.ValidBefore), 0) + if m.NotAfter.Before(certValidBefore) { + return errors.Errorf("provisioning credential expiration (%s) is before requested certificate validBefore (%s)", + m.NotAfter, certValidBefore) + } + } + + // Apply backdate safely + if cert.ValidAfter > backdate { + cert.ValidAfter -= backdate + } + + return nil + }) } // sshCertificateOptionsValidator validates the user SSHOptions with the ones @@ -301,8 +335,15 @@ func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error { return errors.Errorf("unknown ssh certificate type %d", cert.CertType) } - // seconds - dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second + // To not take into account the backdate, time.Now() will be used to + // calculate the duration if ValidAfter is in the past. + var dur time.Duration + if t := now().Unix(); t > int64(cert.ValidAfter) { + dur = time.Duration(int64(cert.ValidBefore)-t) * time.Second + } else { + dur = time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second + } + switch { case dur < min: return errors.Errorf("requested duration of %s is less than minimum "+ diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go index 25a44121..3db0f95e 100644 --- a/authority/provisioner/sign_ssh_options_test.go +++ b/authority/provisioner/sign_ssh_options_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "fmt" "testing" "time" @@ -10,6 +11,32 @@ import ( "golang.org/x/crypto/ssh" ) +func TestSSHOptions_Type(t *testing.T) { + type fields struct { + CertType string + } + tests := []struct { + name string + fields fields + want uint32 + }{ + {"user", fields{"user"}, 1}, + {"host", fields{"host"}, 2}, + {"empty", fields{""}, 0}, + {"invalid", fields{"invalid"}, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := SSHOptions{ + CertType: tt.fields.CertType, + } + if got := o.Type(); got != tt.want { + t.Errorf("SSHOptions.Type() = %v, want %v", got, tt.want) + } + }) + } +} + func Test_sshCertificateDefaultValidator_Valid(t *testing.T) { pub, _, err := keys.GenerateDefaultKeyPair() assert.FatalError(t, err) @@ -276,7 +303,7 @@ func Test_sshValidityModifier(t *testing.T) { p, err := generateX5C(nil) assert.FatalError(t, err) type test struct { - svm *sshValidityModifier + svm *sshLimitDuration cert *ssh.Certificate valid func(*ssh.Certificate) err error @@ -284,7 +311,7 @@ func Test_sshValidityModifier(t *testing.T) { tests := map[string]func() test{ "fail/type-not-set": func() test { return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(6 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(6 * time.Hour)}, cert: &ssh.Certificate{ ValidAfter: uint64(n.Unix()), ValidBefore: uint64(n.Add(8 * time.Hour).Unix()), @@ -294,18 +321,18 @@ func Test_sshValidityModifier(t *testing.T) { }, "fail/type-not-recognized": func() test { return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(6 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(6 * time.Hour)}, cert: &ssh.Certificate{ CertType: 4, ValidAfter: uint64(n.Unix()), ValidBefore: uint64(n.Add(8 * time.Hour).Unix()), }, - err: errors.New("unknown ssh certificate type 4"), + err: errors.New("ssh certificate has an unknown type: 4"), } }, "fail/requested-validAfter-after-limit": func() test { return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(1 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(1 * time.Hour)}, cert: &ssh.Certificate{ CertType: 1, ValidAfter: uint64(n.Add(2 * time.Hour).Unix()), @@ -316,7 +343,7 @@ func Test_sshValidityModifier(t *testing.T) { }, "fail/requested-validBefore-after-limit": func() test { return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(1 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(1 * time.Hour)}, cert: &ssh.Certificate{ CertType: 1, ValidAfter: uint64(n.Unix()), @@ -328,7 +355,7 @@ func Test_sshValidityModifier(t *testing.T) { "ok/valid-requested-validBefore": func() test { va, vb := uint64(n.Unix()), uint64(n.Add(2*time.Hour).Unix()) return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(3 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(3 * time.Hour)}, cert: &ssh.Certificate{ CertType: 1, ValidAfter: va, @@ -343,21 +370,21 @@ func Test_sshValidityModifier(t *testing.T) { "ok/empty-requested-validBefore-limit-after-default": func() test { va := uint64(n.Unix()) return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(5 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(24 * time.Hour)}, cert: &ssh.Certificate{ CertType: 1, ValidAfter: va, }, valid: func(cert *ssh.Certificate) { assert.Equals(t, cert.ValidAfter, va) - assert.Equals(t, cert.ValidBefore, uint64(n.Add(4*time.Hour).Unix())) + assert.Equals(t, cert.ValidBefore, uint64(n.Add(16*time.Hour).Unix())) }, } }, "ok/empty-requested-validBefore-limit-before-default": func() test { va := uint64(n.Unix()) return test{ - svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(3 * time.Hour)}, + svm: &sshLimitDuration{Claimer: p.claimer, NotAfter: n.Add(3 * time.Hour)}, cert: &ssh.Certificate{ CertType: 1, ValidAfter: va, @@ -372,7 +399,7 @@ func Test_sshValidityModifier(t *testing.T) { for name, run := range tests { t.Run(name, func(t *testing.T) { tt := run() - if err := tt.svm.Modify(tt.cert); err != nil { + if err := tt.svm.Option(SSHOptions{}).Modify(tt.cert); err != nil { if assert.NotNil(t, tt.err) { assert.HasPrefix(t, err.Error(), tt.err.Error()) } @@ -384,3 +411,25 @@ func Test_sshValidityModifier(t *testing.T) { }) } } + +func Test_sshModifierFunc_Modify(t *testing.T) { + type args struct { + cert *ssh.Certificate + } + tests := []struct { + name string + f sshModifierFunc + args args + wantErr bool + }{ + {"ok", func(cert *ssh.Certificate) error { return nil }, args{&ssh.Certificate{}}, false}, + {"fail", func(cert *ssh.Certificate) error { return fmt.Errorf("an error") }, args{&ssh.Certificate{}}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.f.Modify(tt.args.cert); (err != nil) != tt.wantErr { + t.Errorf("sshModifierFunc.Modify() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index f02c53b4..76c9a567 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -31,7 +31,7 @@ var ( DisableRenewal: &defaultDisableRenewal, MinUserSSHDur: &Duration{Duration: 5 * time.Minute}, // User SSH certs MaxUserSSHDur: &Duration{Duration: 24 * time.Hour}, - DefaultUserSSHDur: &Duration{Duration: 4 * time.Hour}, + DefaultUserSSHDur: &Duration{Duration: 16 * time.Hour}, MinHostSSHDur: &Duration{Duration: 5 * time.Minute}, // Host SSH certs MaxHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go index 651cd136..1be728db 100644 --- a/authority/provisioner/x5c.go +++ b/authority/provisioner/x5c.go @@ -228,6 +228,7 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, if claims.Step == nil || claims.Step.SSH == nil { return nil, errors.New("authorization token must be an SSH provisioning token") } + opts := claims.Step.SSH signOptions := []SignOption{ // validates user's SSHOptions with the ones in the token @@ -261,7 +262,7 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, // Set the default extensions. &sshDefaultExtensionModifier{}, // Checks the validity bounds, and set the validity if has not been set. - sshLimitValidityModifier(p.claimer, claims.chains[0][0].NotAfter), + &sshLimitDuration{p.claimer, claims.chains[0][0].NotAfter}, // set the key id to the token subject sshCertKeyIDValidator(claims.Subject), // Validate public key. diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go index 94018b55..65147d24 100644 --- a/authority/provisioner/x5c_test.go +++ b/authority/provisioner/x5c_test.go @@ -646,9 +646,9 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) { assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidBefore.RelativeTime(nw).Unix()) case sshCertificateDefaultsModifier: assert.Equals(t, SSHOptions(v), SSHOptions{CertType: SSHUserCert}) - case *sshValidityModifier: + case *sshLimitDuration: assert.Equals(t, v.Claimer, tc.p.claimer) - assert.Equals(t, v.validBefore, tc.claims.chains[0][0].NotAfter) + assert.Equals(t, v.NotAfter, tc.claims.chains[0][0].NotAfter) case *sshCertificateValidityValidator: assert.Equals(t, v.Claimer, tc.p.claimer) case *sshDefaultExtensionModifier, *sshDefaultPublicKeyValidator, diff --git a/authority/ssh.go b/authority/ssh.go index 8148a6bd..e5b2955a 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -209,6 +209,9 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign var mods []provisioner.SSHCertificateModifier var validators []provisioner.SSHCertificateValidator + // Set backdate with the configured value + opts.Backdate = a.config.AuthorityConfig.Backdate.Duration + for _, op := range signOpts { switch o := op.(type) { // modify the ssh.Certificate @@ -365,9 +368,12 @@ func (a *Authority) RenewSSH(oldCert *ssh.Certificate) (*ssh.Certificate, error) if oldCert.ValidAfter == 0 || oldCert.ValidBefore == 0 { return nil, errors.New("rewnewSSH: cannot renew certificate without validity period") } - dur := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second - va := time.Now() - vb := va.Add(dur) + + backdate := a.config.AuthorityConfig.Backdate.Duration + duration := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second + now := time.Now() + va := now.Add(-1 * backdate) + vb := now.Add(duration - backdate) // Build base certificate with the key and some random values cert := &ssh.Certificate{ From f06db4099e43cfe939456b34b3ec53b931434f23 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 3 Jan 2020 18:30:17 -0800 Subject: [PATCH 06/10] Add backdate support on ssh rekey. --- authority/ssh.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/authority/ssh.go b/authority/ssh.go index e5b2955a..cfd5ed37 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -496,9 +496,12 @@ func (a *Authority) RekeySSH(oldCert *ssh.Certificate, pub ssh.PublicKey, signOp if oldCert.ValidAfter == 0 || oldCert.ValidBefore == 0 { return nil, errors.New("rekeySSH: cannot rekey certificate without validity period") } - dur := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second - va := time.Now() - vb := va.Add(dur) + + backdate := a.config.AuthorityConfig.Backdate.Duration + duration := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second + now := time.Now() + va := now.Add(-1 * backdate) + vb := now.Add(duration - backdate) // Build base certificate with the key and some random values cert := &ssh.Certificate{ From 7e33aeb8d3d707b105bd2300c263648159bcfef3 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 6 Jan 2020 12:19:00 -0800 Subject: [PATCH 07/10] Add unit test for profileDefaultDuration. --- authority/provisioner/sign_options.go | 2 +- authority/provisioner/sign_options_test.go | 41 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index 2c3cfa16..1e6547b7 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -194,7 +194,7 @@ func (v profileDefaultDuration) Option(so Options) x509util.WithOption { var backdate time.Duration notBefore := so.NotBefore.Time() if notBefore.IsZero() { - notBefore = time.Now() + notBefore = now() backdate = -1 * so.Backdate } notAfter := so.NotAfter.RelativeTime(notBefore) diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go index 8a452dab..c805f9d7 100644 --- a/authority/provisioner/sign_options_test.go +++ b/authority/provisioner/sign_options_test.go @@ -5,6 +5,7 @@ import ( "crypto/x509/pkix" "net" "net/url" + "reflect" "testing" "time" @@ -357,3 +358,43 @@ func Test_profileLimitDuration_Option(t *testing.T) { }) } } + +func Test_profileDefaultDuration_Option(t *testing.T) { + tm, fn := mockNow() + defer fn() + + v := profileDefaultDuration(24 * time.Hour) + type args struct { + so Options + } + tests := []struct { + name string + v profileDefaultDuration + args args + want *x509.Certificate + }{ + {"default", v, args{Options{}}, &x509.Certificate{NotBefore: tm, NotAfter: tm.Add(24 * time.Hour)}}, + {"backdate", v, args{Options{Backdate: 1 * time.Minute}}, &x509.Certificate{NotBefore: tm.Add(-1 * time.Minute), NotAfter: tm.Add(24 * time.Hour)}}, + {"notBefore", v, args{Options{NotBefore: NewTimeDuration(tm.Add(10 * time.Second))}}, &x509.Certificate{NotBefore: tm.Add(10 * time.Second), NotAfter: tm.Add(24*time.Hour + 10*time.Second)}}, + {"notAfter", v, args{Options{NotAfter: NewTimeDuration(tm.Add(1 * time.Hour))}}, &x509.Certificate{NotBefore: tm, NotAfter: tm.Add(1 * time.Hour)}}, + {"notBefore and notAfter", v, args{Options{NotBefore: NewTimeDuration(tm.Add(10 * time.Second)), NotAfter: NewTimeDuration(tm.Add(1 * time.Hour))}}, + &x509.Certificate{NotBefore: tm.Add(10 * time.Second), NotAfter: tm.Add(1 * time.Hour)}}, + {"notBefore and backdate", v, args{Options{Backdate: 1 * time.Minute, NotBefore: NewTimeDuration(tm.Add(10 * time.Second))}}, + &x509.Certificate{NotBefore: tm.Add(10 * time.Second), NotAfter: tm.Add(24*time.Hour + 10*time.Second)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cert := &x509.Certificate{} + profile := &x509util.Leaf{} + profile.SetSubject(cert) + + fn := tt.v.Option(tt.args.so) + if err := fn(profile); err != nil { + t.Errorf("profileDefaultDuration.Option() error %v", err) + } + if !reflect.DeepEqual(cert, tt.want) { + t.Errorf("profileDefaultDuration.Option() = %v, \nwant %v", cert, tt.want) + } + }) + } +} From 165a91858e2e6289527d48bf2262f7edb7e408e1 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 6 Jan 2020 14:21:13 -0800 Subject: [PATCH 08/10] Add tests for backdate and sshDefaultDuration --- authority/provisioner/sign_options_test.go | 2 +- .../provisioner/sign_ssh_options_test.go | 126 +++++++++++++++++- go.sum | 1 + 3 files changed, 127 insertions(+), 2 deletions(-) diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go index c805f9d7..6c22625f 100644 --- a/authority/provisioner/sign_options_test.go +++ b/authority/provisioner/sign_options_test.go @@ -390,7 +390,7 @@ func Test_profileDefaultDuration_Option(t *testing.T) { fn := tt.v.Option(tt.args.so) if err := fn(profile); err != nil { - t.Errorf("profileDefaultDuration.Option() error %v", err) + t.Errorf("profileDefaultDuration.Option() error = %v", err) } if !reflect.DeepEqual(cert, tt.want) { t.Errorf("profileDefaultDuration.Option() = %v, \nwant %v", cert, tt.want) diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go index 3db0f95e..e447065b 100644 --- a/authority/provisioner/sign_ssh_options_test.go +++ b/authority/provisioner/sign_ssh_options_test.go @@ -2,6 +2,7 @@ package provisioner import ( "fmt" + "reflect" "testing" "time" @@ -299,7 +300,9 @@ func Test_sshCertificateValidityValidator(t *testing.T) { } func Test_sshValidityModifier(t *testing.T) { - n := now() + n, fn := mockNow() + defer fn() + p, err := generateX5C(nil) assert.FatalError(t, err) type test struct { @@ -352,6 +355,32 @@ func Test_sshValidityModifier(t *testing.T) { err: errors.New("provisioning credential expiration ("), } }, + "ok/no-limit": func() test { + va, vb := uint64(n.Unix()), uint64(n.Add(16*time.Hour).Unix()) + return test{ + svm: &sshLimitDuration{Claimer: p.claimer}, + cert: &ssh.Certificate{ + CertType: 1, + }, + valid: func(cert *ssh.Certificate) { + assert.Equals(t, cert.ValidAfter, va) + assert.Equals(t, cert.ValidBefore, vb) + }, + } + }, + "ok/defaults": func() test { + va, vb := uint64(n.Unix()), uint64(n.Add(16*time.Hour).Unix()) + return test{ + svm: &sshLimitDuration{Claimer: p.claimer}, + cert: &ssh.Certificate{ + CertType: 1, + }, + valid: func(cert *ssh.Certificate) { + assert.Equals(t, cert.ValidAfter, va) + assert.Equals(t, cert.ValidBefore, vb) + }, + } + }, "ok/valid-requested-validBefore": func() test { va, vb := uint64(n.Unix()), uint64(n.Add(2*time.Hour).Unix()) return test{ @@ -433,3 +462,98 @@ func Test_sshModifierFunc_Modify(t *testing.T) { }) } } + +func Test_sshDefaultDuration_Option(t *testing.T) { + tm, fn := mockNow() + defer fn() + + newClaimer := func(claims *Claims) *Claimer { + c, err := NewClaimer(claims, globalProvisionerClaims) + if err != nil { + t.Fatal(err) + } + return c + } + unix := func(d time.Duration) uint64 { + return uint64(tm.Add(d).Unix()) + } + + type fields struct { + Claimer *Claimer + } + type args struct { + o SSHOptions + cert *ssh.Certificate + } + tests := []struct { + name string + fields fields + args args + want *ssh.Certificate + wantErr bool + }{ + {"user", fields{newClaimer(nil)}, args{SSHOptions{}, &ssh.Certificate{CertType: ssh.UserCert}}, + &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(0), ValidBefore: unix(16 * time.Hour)}, false}, + {"host", fields{newClaimer(nil)}, args{SSHOptions{}, &ssh.Certificate{CertType: ssh.HostCert}}, + &ssh.Certificate{CertType: ssh.HostCert, ValidAfter: unix(0), ValidBefore: unix(30 * 24 * time.Hour)}, false}, + {"user claim", fields{newClaimer(&Claims{DefaultUserSSHDur: &Duration{1 * time.Hour}})}, args{SSHOptions{}, &ssh.Certificate{CertType: ssh.UserCert}}, + &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(0), ValidBefore: unix(1 * time.Hour)}, false}, + {"host claim", fields{newClaimer(&Claims{DefaultHostSSHDur: &Duration{1 * time.Hour}})}, args{SSHOptions{}, &ssh.Certificate{CertType: ssh.HostCert}}, + &ssh.Certificate{CertType: ssh.HostCert, ValidAfter: unix(0), ValidBefore: unix(1 * time.Hour)}, false}, + {"user backdate", fields{newClaimer(nil)}, args{SSHOptions{Backdate: 1 * time.Minute}, &ssh.Certificate{CertType: ssh.UserCert}}, + &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(-1 * time.Minute), ValidBefore: unix(16 * time.Hour)}, false}, + {"host backdate", fields{newClaimer(nil)}, args{SSHOptions{Backdate: 1 * time.Minute}, &ssh.Certificate{CertType: ssh.HostCert}}, + &ssh.Certificate{CertType: ssh.HostCert, ValidAfter: unix(-1 * time.Minute), ValidBefore: unix(30 * 24 * time.Hour)}, false}, + {"user validAfter", fields{newClaimer(nil)}, args{SSHOptions{Backdate: 1 * time.Minute}, &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(1 * time.Hour)}}, + &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(time.Minute), ValidBefore: unix(17 * time.Hour)}, false}, + {"user validBefore", fields{newClaimer(nil)}, args{SSHOptions{Backdate: 1 * time.Minute}, &ssh.Certificate{CertType: ssh.UserCert, ValidBefore: unix(1 * time.Hour)}}, + &ssh.Certificate{CertType: ssh.UserCert, ValidAfter: unix(-1 * time.Minute), ValidBefore: unix(time.Hour)}, false}, + {"host validAfter validBefore", fields{newClaimer(nil)}, args{SSHOptions{Backdate: 1 * time.Minute}, &ssh.Certificate{CertType: ssh.HostCert, ValidAfter: unix(1 * time.Minute), ValidBefore: unix(2 * time.Minute)}}, + &ssh.Certificate{CertType: ssh.HostCert, ValidAfter: unix(1 * time.Minute), ValidBefore: unix(2 * time.Minute)}, false}, + {"fail zero", fields{newClaimer(nil)}, args{SSHOptions{}, &ssh.Certificate{}}, &ssh.Certificate{}, true}, + {"fail type", fields{newClaimer(nil)}, args{SSHOptions{}, &ssh.Certificate{CertType: 3}}, &ssh.Certificate{CertType: 3}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &sshDefaultDuration{ + Claimer: tt.fields.Claimer, + } + v := m.Option(tt.args.o) + if err := v.Modify(tt.args.cert); (err != nil) != tt.wantErr { + t.Errorf("sshDefaultDuration.Option() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.args.cert, tt.want) { + t.Errorf("sshDefaultDuration.Option() = %v, want %v", tt.args.cert, tt.want) + } + }) + } +} + +func Test_sshLimitDuration_Option(t *testing.T) { + type fields struct { + Claimer *Claimer + NotAfter time.Time + } + type args struct { + o SSHOptions + } + tests := []struct { + name string + fields fields + args args + want SSHCertificateModifier + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &sshLimitDuration{ + Claimer: tt.fields.Claimer, + NotAfter: tt.fields.NotAfter, + } + if got := m.Option(tt.args.o); !reflect.DeepEqual(got, tt.want) { + t.Errorf("sshLimitDuration.Option() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/go.sum b/go.sum index c200eeb8..e914534f 100644 --- a/go.sum +++ b/go.sum @@ -166,6 +166,7 @@ golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 h1:0hQKqeLdqlt5iIwVOBErRi golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191227163750-53104e6ec876 h1:sKJQZMuxjOAR/Uo2LBfU90onWEf1dF4C+0hPJCc9Mpc= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190424112056-4829fb13d2c6/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= From f46dc031119ab26cf5e3f7bf99286ffed5dc61a2 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 6 Jan 2020 14:34:59 -0800 Subject: [PATCH 09/10] Add tests of profileLimitDuration with backdate. --- authority/provisioner/sign_options_test.go | 32 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go index 6c22625f..d462780e 100644 --- a/authority/provisioner/sign_options_test.go +++ b/authority/provisioner/sign_options_test.go @@ -276,7 +276,9 @@ func Test_validityValidator_Valid(t *testing.T) { } func Test_profileLimitDuration_Option(t *testing.T) { - n := now() + n, fn := mockNow() + defer fn() + type test struct { pld profileLimitDuration so Options @@ -310,7 +312,7 @@ func Test_profileLimitDuration_Option(t *testing.T) { assert.FatalError(t, err) return test{ pld: profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)}, - so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), NotAfter: d}, + so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), NotAfter: d, Backdate: 1 * time.Minute}, cert: new(x509.Certificate), valid: func(cert *x509.Certificate) { assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour)) @@ -321,7 +323,7 @@ func Test_profileLimitDuration_Option(t *testing.T) { "ok/valid-notAfter-nil-limit-over-default": func() test { return test{ pld: profileLimitDuration{def: 1 * time.Hour, notAfter: n.Add(6 * time.Hour)}, - so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour))}, + so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), Backdate: 1 * time.Minute}, cert: new(x509.Certificate), valid: func(cert *x509.Certificate) { assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour)) @@ -332,7 +334,7 @@ func Test_profileLimitDuration_Option(t *testing.T) { "ok/valid-notAfter-nil-limit-under-default": func() test { return test{ pld: profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)}, - so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour))}, + so: Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), Backdate: 1 * time.Minute}, cert: new(x509.Certificate), valid: func(cert *x509.Certificate) { assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour)) @@ -340,6 +342,28 @@ func Test_profileLimitDuration_Option(t *testing.T) { }, } }, + "ok/over-limit-with-backdate": func() test { + return test{ + pld: profileLimitDuration{def: 24 * time.Hour, notAfter: n.Add(6 * time.Hour)}, + so: Options{Backdate: 1 * time.Minute}, + cert: new(x509.Certificate), + valid: func(cert *x509.Certificate) { + assert.Equals(t, cert.NotBefore, n.Add(-time.Minute)) + assert.Equals(t, cert.NotAfter, n.Add(6*time.Hour)) + }, + } + }, + "ok/under-limit-with-backdate": func() test { + return test{ + pld: profileLimitDuration{def: 24 * time.Hour, notAfter: n.Add(30 * time.Hour)}, + so: Options{Backdate: 1 * time.Minute}, + cert: new(x509.Certificate), + valid: func(cert *x509.Certificate) { + assert.Equals(t, cert.NotBefore, n.Add(-time.Minute)) + assert.Equals(t, cert.NotAfter, n.Add(24*time.Hour)) + }, + } + }, } for name, run := range tests { t.Run(name, func(t *testing.T) { From 77af30bfa395af5a025bb045315359c6fad7977c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 8 Jan 2020 11:46:33 -0800 Subject: [PATCH 10/10] Remove debug statement. --- authority/provisioner/sign_ssh_options.go | 1 - 1 file changed, 1 deletion(-) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index f5ad8662..643e0645 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -276,7 +276,6 @@ func (m *sshLimitDuration) Option(o SSHOptions) SSHCertificateModifier { certValidBefore := certValidAfter.Add(d) if m.NotAfter.Before(certValidBefore) { certValidBefore = m.NotAfter - println(2, certValidBefore.String()) } cert.ValidBefore = uint64(certValidBefore.Unix()) } else {