From fcccb06696e2a5b47dbdb39ac7d0f3b1fd2b8f8e Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 14 Nov 2019 15:26:37 -0800 Subject: [PATCH] Fix some provisioner tests --- authority/provisioner/aws_test.go | 6 +++--- authority/provisioner/azure_test.go | 6 +++--- authority/provisioner/gcp_test.go | 6 +++--- authority/provisioner/jwk.go | 4 ++++ authority/provisioner/jwk_test.go | 14 +++++++------- authority/provisioner/oidc_test.go | 6 +++--- authority/provisioner/sign_ssh_options.go | 7 +++++-- authority/provisioner/utils_test.go | 8 ++++++-- authority/provisioner/x5c.go | 5 +++++ 9 files changed, 39 insertions(+), 23 deletions(-) diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go index bb8e74f8..e855bf9f 100644 --- a/authority/provisioner/aws_test.go +++ b/authority/provisioner/aws_test.go @@ -360,7 +360,7 @@ func TestAWS_AuthorizeSign(t *testing.T) { } } -func TestAWS_AuthorizeSign_SSH(t *testing.T) { +func TestAWS_AuthorizeSSHSign(t *testing.T) { tm, fn := mockNow() defer fn() @@ -425,9 +425,9 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - got, err := tt.aws.AuthorizeSign(ctx, tt.args.token) + got, err := tt.aws.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { - t.Errorf("AWS.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("AWS.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index 68e4adef..1760ed5c 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -310,7 +310,7 @@ func TestAzure_AuthorizeSign(t *testing.T) { } } -func TestAzure_AuthorizeSign_SSH(t *testing.T) { +func TestAzure_AuthorizeSSHSign(t *testing.T) { tm, fn := mockNow() defer fn() @@ -365,9 +365,9 @@ func TestAzure_AuthorizeSign_SSH(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - got, err := tt.azure.AuthorizeSign(ctx, tt.args.token) + got, err := tt.azure.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { - t.Errorf("Azure.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("Azure.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index 23e45b80..4764dfc7 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -345,7 +345,7 @@ func TestGCP_AuthorizeSign(t *testing.T) { } } -func TestGCP_AuthorizeSign_SSH(t *testing.T) { +func TestGCP_AuthorizeSSHSign(t *testing.T) { tm, fn := mockNow() defer fn() @@ -412,9 +412,9 @@ func TestGCP_AuthorizeSign_SSH(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - got, err := tt.gcp.AuthorizeSign(ctx, tt.args.token) + got, err := tt.gcp.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { - t.Errorf("GCP.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GCP.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 52f83846..f6ed9bbf 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -207,6 +207,10 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, if !opts.ValidBefore.IsZero() { signOptions = append(signOptions, sshCertificateValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix())) } + // Make sure to define the the KeyID + if opts.KeyID == "" { + signOptions = append(signOptions, sshCertificateKeyIDModifier(claims.Subject)) + } // Default to a user certificate with no principals if not set signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert}) diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index 4861b8c5..47a6e7cc 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -329,7 +329,7 @@ func TestJWK_AuthorizeRenew(t *testing.T) { } } -func TestJWK_AuthorizeSign_SSH(t *testing.T) { +func TestJWK_AuthorizeSSHSign(t *testing.T) { tm, fn := mockNow() defer fn() @@ -338,7 +338,7 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { jwk, err := decryptJSONWebKey(p1.EncryptedKey) assert.FatalError(t, err) - iss, aud := p1.Name, testAudiences.Sign[0] + iss, aud := p1.Name, testAudiences.SSHSign[0] t1, err := generateSimpleSSHUserToken(iss, aud, jwk) assert.FatalError(t, err) @@ -400,9 +400,9 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - got, err := tt.prov.AuthorizeSign(ctx, tt.args.token) + got, err := tt.prov.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { - t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("JWK.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { @@ -432,7 +432,7 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { jwk, err := decryptJSONWebKey(p1.EncryptedKey) assert.FatalError(t, err) - sub, iss, aud, iat := "subject@smallstep.com", p1.Name, testAudiences.Sign[0], time.Now() + sub, iss, aud, iat := "subject@smallstep.com", p1.Name, testAudiences.SSHSign[0], time.Now() key, err := generateJSONWebKey() assert.FatalError(t, err) @@ -514,8 +514,8 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) token, err := generateSSHToken(tt.args.sub, tt.args.iss, tt.args.aud, tt.args.iat, tt.args.tokSSHOpts, tt.args.jwk) assert.FatalError(t, err) - if got, err := tt.prov.AuthorizeSign(ctx, token); (err != nil) != tt.wantErr { - t.Errorf("JWK.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + if got, err := tt.prov.AuthorizeSSHSign(ctx, token); (err != nil) != tt.wantErr { + t.Errorf("JWK.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) } else if !tt.wantErr && assert.NotNil(t, got) { var opts SSHOptions if tt.args.userSSHOpts != nil { diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index e26ded0a..8e0c823c 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -330,7 +330,7 @@ func TestOIDC_AuthorizeSign(t *testing.T) { } } -func TestOIDC_AuthorizeSign_SSH(t *testing.T) { +func TestOIDC_AuthorizeSSHSign(t *testing.T) { tm, fn := mockNow() defer fn() @@ -427,9 +427,9 @@ func TestOIDC_AuthorizeSign_SSH(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - got, err := tt.prov.AuthorizeSign(ctx, tt.args.token) + got, err := tt.prov.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { - t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("OIDC.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 5b65c159..06ddf697 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -74,15 +74,18 @@ func (o SSHOptions) Modify(cert *ssh.Certificate) error { cert.KeyId = o.KeyID cert.ValidPrincipals = o.Principals + + t := now() if !o.ValidAfter.IsZero() { - cert.ValidAfter = uint64(o.ValidAfter.Time().Unix()) + cert.ValidAfter = uint64(o.ValidAfter.RelativeTime(t).Unix()) } if !o.ValidBefore.IsZero() { - cert.ValidBefore = uint64(o.ValidBefore.Time().Unix()) + cert.ValidBefore = uint64(o.ValidBefore.RelativeTime(t).Unix()) } if cert.ValidAfter > 0 && cert.ValidBefore > 0 && cert.ValidAfter > cert.ValidBefore { return errors.New("ssh certificate valid after cannot be greater than valid before") } + return nil } diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index 554e38ea..f02c53b4 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -38,8 +38,12 @@ var ( EnableSSHCA: &defaultEnableSSHCA, } testAudiences = Audiences{ - Sign: []string{"https://ca.smallstep.com/sign", "https://ca.smallstep.com/1.0/sign"}, - Revoke: []string{"https://ca.smallstep.com/revoke", "https://ca.smallstep.com/1.0/revoke"}, + Sign: []string{"https://ca.smallstep.com/1.0/sign", "https://ca.smallstep.com/sign"}, + Revoke: []string{"https://ca.smallstep.com/1.0/revoke", "https://ca.smallstep.com/revoke"}, + SSHSign: []string{"https://ca.smallstep.com/1.0/ssh/sign"}, + SSHRevoke: []string{"https://ca.smallstep.com/1.0/ssh/revoke"}, + SSHRenew: []string{"https://ca.smallstep.com/1.0/ssh/renew"}, + SSHRekey: []string{"https://ca.smallstep.com/1.0/ssh/rekey"}, } ) diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go index a282692e..4fa15a44 100644 --- a/authority/provisioner/x5c.go +++ b/authority/provisioner/x5c.go @@ -235,6 +235,7 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, } // Add modifiers from custom claims + // FIXME: this is also set in the sign method using SSHOptions.Modify. if opts.CertType != "" { signOptions = append(signOptions, sshCertificateCertTypeModifier(opts.CertType)) } @@ -248,6 +249,10 @@ func (p *X5C) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, if !opts.ValidBefore.IsZero() { signOptions = append(signOptions, sshCertificateValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix())) } + // Make sure to define the the KeyID + if opts.KeyID == "" { + signOptions = append(signOptions, sshCertificateKeyIDModifier(claims.Subject)) + } // Default to a user certificate with no principals if not set signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert})