From 396b4222aab213fc44ea1d642629fb2b59ff239a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 10 Sep 2019 17:04:13 -0700 Subject: [PATCH] Implement validator for ssh keys. Fixes #100 --- authority/provisioner/aws.go | 2 + authority/provisioner/aws_test.go | 29 ++++++---- authority/provisioner/azure.go | 2 + authority/provisioner/azure_test.go | 27 ++++++--- authority/provisioner/gcp.go | 2 + authority/provisioner/gcp_test.go | 31 ++++++---- authority/provisioner/jwk.go | 2 + authority/provisioner/jwk_test.go | 31 ++++++---- authority/provisioner/oidc.go | 2 + authority/provisioner/oidc_test.go | 37 +++++++----- authority/provisioner/sign_ssh_options.go | 70 +++++++++++++++++++++++ 11 files changed, 184 insertions(+), 51 deletions(-) diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index dae817f8..c6360dbd 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -470,6 +470,8 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) { &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{p.claimer}, + // validate public key + &sshDefaultPublicKeyValidator{}, // require all the fields in the SSH certificate &sshCertificateDefaultValidator{}, ), nil diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go index 4fe9367a..dd82ec9b 100644 --- a/authority/provisioner/aws_test.go +++ b/authority/provisioner/aws_test.go @@ -377,6 +377,12 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + pub := key.Public().Key + rsa2048, err := rsa.GenerateKey(rand.Reader, 2048) + assert.FatalError(t, err) + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + assert.FatalError(t, err) + hostDuration := p1.claimer.DefaultHostSSHCertDuration() expectedHostOptions := &SSHOptions{ CertType: "host", Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}, @@ -394,6 +400,7 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) { type args struct { token string sshOpts SSHOptions + key interface{} } tests := []struct { name string @@ -403,15 +410,17 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) { wantErr bool wantSignErr bool }{ - {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, - {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, - {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptions, false, false}, - {"ok-principal-ip", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1"}}}, expectedHostOptionsIP, false, false}, - {"ok-principal-hostname", p1, args{t1, SSHOptions{Principals: []string{"ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptionsHostname, false, false}, - {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptions, false, false}, - {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, - {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, - {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal", "smallstep.com"}}}, nil, false, true}, + {"ok", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false}, + {"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}, pub}, expectedHostOptions, false, false}, + {"ok-principal-ip", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1"}}, pub}, expectedHostOptionsIP, false, false}, + {"ok-principal-hostname", p1, args{t1, SSHOptions{Principals: []string{"ip-127-0-0-1.us-west-1.compute.internal"}}, pub}, expectedHostOptionsHostname, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}, pub}, expectedHostOptions, false, false}, + {"fail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedHostOptions, false, true}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}, pub}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}, pub}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal", "smallstep.com"}}, pub}, nil, false, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -424,7 +433,7 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) { if err != nil { assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + cert, err := signSSHCertificate(tt.args.key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index 7619d202..e5e858dd 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -327,6 +327,8 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{p.claimer}, + // validate public key + &sshDefaultPublicKeyValidator{}, // require all the fields in the SSH certificate &sshCertificateDefaultValidator{}, ), nil diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index 21ebb59d..82ff095e 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -3,6 +3,8 @@ package provisioner import ( "context" "crypto" + "crypto/rand" + "crypto/rsa" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -325,6 +327,12 @@ func TestAzure_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + pub := key.Public().Key + rsa2048, err := rsa.GenerateKey(rand.Reader, 2048) + assert.FatalError(t, err) + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + assert.FatalError(t, err) + hostDuration := p1.claimer.DefaultHostSSHCertDuration() expectedHostOptions := &SSHOptions{ CertType: "host", Principals: []string{"virtualMachine"}, @@ -334,6 +342,7 @@ func TestAzure_AuthorizeSign_SSH(t *testing.T) { type args struct { token string sshOpts SSHOptions + key interface{} } tests := []struct { name string @@ -343,13 +352,15 @@ func TestAzure_AuthorizeSign_SSH(t *testing.T) { wantErr bool wantSignErr bool }{ - {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, - {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, - {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine"}}}, expectedHostOptions, false, false}, - {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"virtualMachine"}}}, expectedHostOptions, false, false}, - {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, - {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, - {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine", "smallstep.com"}}}, nil, false, true}, + {"ok", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false}, + {"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine"}}, pub}, expectedHostOptions, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"virtualMachine"}}, pub}, expectedHostOptions, false, false}, + {"fail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedHostOptions, false, true}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}, pub}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}, pub}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine", "smallstep.com"}}, pub}, nil, false, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -362,7 +373,7 @@ func TestAzure_AuthorizeSign_SSH(t *testing.T) { if err != nil { assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + cert, err := signSSHCertificate(tt.args.key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index bb8c4ede..63492fd4 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -382,6 +382,8 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) { &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{p.claimer}, + // validate public key + &sshDefaultPublicKeyValidator{}, // require all the fields in the SSH certificate &sshCertificateDefaultValidator{}, ), nil diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index 077537a1..13a735ae 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -3,6 +3,8 @@ package provisioner import ( "context" "crypto" + "crypto/rand" + "crypto/rsa" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -362,6 +364,12 @@ func TestGCP_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + pub := key.Public().Key + rsa2048, err := rsa.GenerateKey(rand.Reader, 2048) + assert.FatalError(t, err) + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + assert.FatalError(t, err) + hostDuration := p1.claimer.DefaultHostSSHCertDuration() expectedHostOptions := &SSHOptions{ CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}, @@ -379,6 +387,7 @@ func TestGCP_AuthorizeSign_SSH(t *testing.T) { type args struct { token string sshOpts SSHOptions + key interface{} } tests := []struct { name string @@ -388,15 +397,17 @@ func TestGCP_AuthorizeSign_SSH(t *testing.T) { wantErr bool wantSignErr bool }{ - {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, - {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, - {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}}, expectedHostOptions, false, false}, - {"ok-principal1", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal"}}}, expectedHostOptionsPrincipal1, false, false}, - {"ok-principal2", p1, args{t1, SSHOptions{Principals: []string{"instance-name.zone.c.project-id.internal"}}}, expectedHostOptionsPrincipal2, false, false}, - {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}}, expectedHostOptions, false, false}, - {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, - {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, - {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal", "smallstep.com"}}}, nil, false, true}, + {"ok", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false}, + {"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptions, false, false}, + {"ok-principal1", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal"}}, pub}, expectedHostOptionsPrincipal1, false, false}, + {"ok-principal2", p1, args{t1, SSHOptions{Principals: []string{"instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptionsPrincipal2, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptions, false, false}, + {"fail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedHostOptions, false, true}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}, pub}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}, pub}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal", "smallstep.com"}}, pub}, nil, false, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -409,7 +420,7 @@ func TestGCP_AuthorizeSign_SSH(t *testing.T) { if err != nil { assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + cert, err := signSSHCertificate(tt.args.key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 9bf38f17..cdc90ff6 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -210,6 +210,8 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{p.claimer}, + // validate public key + &sshDefaultPublicKeyValidator{}, // require all the fields in the SSH certificate &sshCertificateDefaultValidator{}, ), nil diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index a13db307..0e7ed57a 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -3,6 +3,8 @@ package provisioner import ( "context" "crypto" + "crypto/rand" + "crypto/rsa" "crypto/x509" "errors" "strings" @@ -356,6 +358,12 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + pub := key.Public().Key + rsa2048, err := rsa.GenerateKey(rand.Reader, 2048) + assert.FatalError(t, err) + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + assert.FatalError(t, err) + userDuration := p1.claimer.DefaultUserSSHCertDuration() hostDuration := p1.claimer.DefaultHostSSHCertDuration() expectedUserOptions := &SSHOptions{ @@ -370,6 +378,7 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { type args struct { token string sshOpts SSHOptions + key interface{} } tests := []struct { name string @@ -379,15 +388,17 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { wantErr bool wantSignErr bool }{ - {"user", p1, args{t1, SSHOptions{}}, expectedUserOptions, false, false}, - {"user-type", p1, args{t1, SSHOptions{CertType: "user"}}, expectedUserOptions, false, false}, - {"user-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}}, expectedUserOptions, false, false}, - {"user-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, - {"host", p1, args{t2, SSHOptions{}}, expectedHostOptions, false, false}, - {"host-type", p1, args{t2, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, - {"host-principals", p1, args{t2, SSHOptions{Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, - {"host-options", p1, args{t2, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, - {"fail-signature", p1, args{failSig, SSHOptions{}}, nil, true, false}, + {"user", p1, args{t1, SSHOptions{}, pub}, expectedUserOptions, false, false}, + {"user-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedUserOptions, false, false}, + {"user-type", p1, args{t1, SSHOptions{CertType: "user"}, pub}, expectedUserOptions, false, false}, + {"user-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}, pub}, expectedUserOptions, false, false}, + {"user-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub}, expectedUserOptions, false, false}, + {"host", p1, args{t2, SSHOptions{}, pub}, expectedHostOptions, false, false}, + {"host-type", p1, args{t2, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false}, + {"host-principals", p1, args{t2, SSHOptions{Principals: []string{"smallstep.com"}}, pub}, expectedHostOptions, false, false}, + {"host-options", p1, args{t2, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, pub}, expectedHostOptions, false, false}, + {"fail-signature", p1, args{failSig, SSHOptions{}, pub}, nil, true, false}, + {"rail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedUserOptions, false, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -400,7 +411,7 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { if err != nil { assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + cert, err := signSSHCertificate(tt.args.key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 01b59625..12d1d1e0 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -336,6 +336,8 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{o.claimer}, + // validate public key + &sshDefaultPublicKeyValidator{}, // require all the fields in the SSH certificate &sshCertificateDefaultValidator{}, ), nil diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 9a756c5d..7d149076 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -3,6 +3,8 @@ package provisioner import ( "context" "crypto" + "crypto/rand" + "crypto/rsa" "crypto/x509" "fmt" "strings" @@ -343,6 +345,12 @@ func TestOIDC_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + pub := key.Public().Key + rsa2048, err := rsa.GenerateKey(rand.Reader, 2048) + assert.FatalError(t, err) + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + assert.FatalError(t, err) + userDuration := p1.claimer.DefaultUserSSHCertDuration() hostDuration := p1.claimer.DefaultHostSSHCertDuration() expectedUserOptions := &SSHOptions{ @@ -361,6 +369,7 @@ func TestOIDC_AuthorizeSign_SSH(t *testing.T) { type args struct { token string sshOpts SSHOptions + key interface{} } tests := []struct { name string @@ -370,18 +379,20 @@ func TestOIDC_AuthorizeSign_SSH(t *testing.T) { wantErr bool wantSignErr bool }{ - {"ok", p1, args{t1, SSHOptions{}}, expectedUserOptions, false, false}, - {"ok-user", p1, args{t1, SSHOptions{CertType: "user"}}, expectedUserOptions, false, false}, - {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}}, expectedUserOptions, false, false}, - {"ok-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, - {"admin", p3, args{okAdmin, SSHOptions{}}, expectedAdminOptions, false, false}, - {"admin-user", p3, args{okAdmin, SSHOptions{CertType: "user"}}, expectedAdminOptions, false, false}, - {"admin-principals", p3, args{okAdmin, SSHOptions{Principals: []string{"root"}}}, expectedAdminOptions, false, false}, - {"admin-options", p3, args{okAdmin, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, - {"admin-host", p3, args{okAdmin, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, - {"fail-user-host", p1, args{t1, SSHOptions{CertType: "host"}}, nil, false, true}, - {"fail-user-principals", p1, args{t1, SSHOptions{Principals: []string{"root"}}}, nil, false, true}, - {"fail-email", p3, args{failEmail, SSHOptions{}}, nil, true, false}, + {"ok", p1, args{t1, SSHOptions{}, pub}, expectedUserOptions, false, false}, + {"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedUserOptions, false, false}, + {"ok-user", p1, args{t1, SSHOptions{CertType: "user"}, pub}, expectedUserOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}, pub}, expectedUserOptions, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub}, expectedUserOptions, false, false}, + {"admin", p3, args{okAdmin, SSHOptions{}, pub}, expectedAdminOptions, false, false}, + {"admin-user", p3, args{okAdmin, SSHOptions{CertType: "user"}, pub}, expectedAdminOptions, false, false}, + {"admin-principals", p3, args{okAdmin, SSHOptions{Principals: []string{"root"}}, pub}, expectedAdminOptions, false, false}, + {"admin-options", p3, args{okAdmin, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub}, expectedUserOptions, false, false}, + {"admin-host", p3, args{okAdmin, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, pub}, expectedHostOptions, false, false}, + {"fail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedUserOptions, false, true}, + {"fail-user-host", p1, args{t1, SSHOptions{CertType: "host"}, pub}, nil, false, true}, + {"fail-user-principals", p1, args{t1, SSHOptions{Principals: []string{"root"}}, pub}, nil, false, true}, + {"fail-email", p3, args{failEmail, SSHOptions{}, pub}, nil, true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -394,7 +405,7 @@ func TestOIDC_AuthorizeSign_SSH(t *testing.T) { if err != nil { assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + cert, err := signSSHCertificate(tt.args.key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 9ca2a95c..7cc809a9 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -1,6 +1,9 @@ package provisioner import ( + "crypto/rsa" + "encoding/binary" + "math/big" "time" "github.com/pkg/errors" @@ -275,6 +278,35 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error { } } +// sshDefaultPublicKeyValidator implements a validator for the certificate key. +type sshDefaultPublicKeyValidator struct{} + +// Valid checks that certificate request common name matches the one configured. +func (v sshDefaultPublicKeyValidator) Valid(cert *ssh.Certificate) error { + if cert.Key == nil { + return errors.New("ssh certificate key cannot be nil") + } + switch cert.Key.Type() { + case ssh.KeyAlgoRSA: + _, in, ok := sshParseString(cert.Key.Marshal()) + if !ok { + return errors.New("ssh certificate key is invalid") + } + key, err := sshParseRSAPublicKey(in) + if err != nil { + return err + } + if key.Size() < 256 { + return errors.New("ssh certificate key must be at least 2048 bits (256 bytes)") + } + return nil + case ssh.KeyAlgoDSA: + return errors.New("ssh certificate key algorithm (DSA) is not supported") + default: + return nil + } +} + // sshCertTypeUInt32 func sshCertTypeUInt32(ct string) uint32 { switch ct { @@ -304,3 +336,41 @@ func containsAllMembers(group, subgroup []string) bool { } return true } + +func sshParseString(in []byte) (out, rest []byte, ok bool) { + if len(in) < 4 { + return + } + length := binary.BigEndian.Uint32(in) + in = in[4:] + if uint32(len(in)) < length { + return + } + out = in[:length] + rest = in[length:] + ok = true + return +} + +func sshParseRSAPublicKey(in []byte) (*rsa.PublicKey, error) { + var w struct { + E *big.Int + N *big.Int + Rest []byte `ssh:"rest"` + } + if err := ssh.Unmarshal(in, &w); err != nil { + return nil, errors.Wrap(err, "error unmarshalling public key") + } + if w.E.BitLen() > 24 { + return nil, errors.New("invalid public key: exponent too large") + } + e := w.E.Int64() + if e < 3 || e&1 == 0 { + return nil, errors.New("invalid public key: incorrect exponent") + } + + var key rsa.PublicKey + key.E = int(e) + key.N = w.N + return &key, nil +}