Fix provisioning tests.

authority/provisioner/aws_test.go

@@ -708,6 +708,7 @@ func TestAWS_AuthorizeSSHSign(t *testing.T) {
 			} else if assert.NotNil(t, got) {
 				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 {
 					if tt.wantSignErr {

authority/provisioner/jwk_test.go

@@ -512,10 +512,6 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) {
 	}{
 		{"ok-user", p1, args{sub, iss, aud, iat, &SignSSHOptions{CertType: "user", Principals: []string{"name"}}, &SignSSHOptions{}, jwk}, expectedUserOptions, false, false},
 		{"ok-host", p1, args{sub, iss, aud, iat, &SignSSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, &SignSSHOptions{}, jwk}, expectedHostOptions, false, false},
-		{"ok-user-opts", p1, args{sub, iss, aud, iat, &SignSSHOptions{}, &SignSSHOptions{CertType: "user", Principals: []string{"name"}}, jwk}, expectedUserOptions, false, false},
-		{"ok-host-opts", p1, args{sub, iss, aud, iat, &SignSSHOptions{}, &SignSSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, jwk}, expectedHostOptions, false, false},
-		{"ok-user-mixed", p1, args{sub, iss, aud, iat, &SignSSHOptions{CertType: "user"}, &SignSSHOptions{Principals: []string{"name"}}, jwk}, expectedUserOptions, false, false},
-		{"ok-host-mixed", p1, args{sub, iss, aud, iat, &SignSSHOptions{Principals: []string{"smallstep.com"}}, &SignSSHOptions{CertType: "host"}, jwk}, expectedHostOptions, false, false},
 		{"ok-user-validAfter", p1, args{sub, iss, aud, iat, &SignSSHOptions{
 			CertType: "user", Principals: []string{"name"},
 		}, &SignSSHOptions{

authority/provisioner/k8sSA_test.go

@@ -361,9 +361,9 @@ func TestK8sSA_AuthorizeSSHSign(t *testing.T) {
 						tot := 0
 						for _, o := range opts {
 							switch v := o.(type) {
-							case sshCertDefaultsModifier:
+							case sshCertificateOptionsFunc:
-								assert.Equals(t, v.CertType, SSHUserCert)
+							case *sshCertOptionsRequireValidator:
-							case *sshDefaultExtensionModifier:
+								assert.Equals(t, v, &sshCertOptionsRequireValidator{CertType: true, KeyID: true, Principals: true})
 							case *sshCertValidityValidator:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
 							case *sshDefaultPublicKeyValidator:

authority/provisioner/oidc_test.go

@@ -565,33 +565,34 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
 		{"ok-rsa2048", p1, args{t1, SignSSHOptions{}, rsa2048.Public()}, expectedUserOptions, http.StatusOK, false, false},
 		{"ok-user", p1, args{t1, SignSSHOptions{CertType: "user"}, pub}, expectedUserOptions, http.StatusOK, false, false},
 		{"ok-principals", p1, args{t1, SignSSHOptions{Principals: []string{"name"}}, pub},
-			&SignSSHOptions{CertType: "user", Principals: []string{"name"},
+			&SignSSHOptions{CertType: "user", Principals: []string{"name", "name@smallstep.com"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
 		{"ok-principals-getIdentity", p4, args{okGetIdentityToken, SignSSHOptions{Principals: []string{"mariano"}}, pub},
-			&SignSSHOptions{CertType: "user", Principals: []string{"mariano"},
+			&SignSSHOptions{CertType: "user", Principals: []string{"max", "mariano"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
 		{"ok-emptyPrincipals-getIdentity", p4, args{okGetIdentityToken, SignSSHOptions{}, pub},
 			&SignSSHOptions{CertType: "user", Principals: []string{"max", "mariano"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
 		{"ok-options", p1, args{t1, SignSSHOptions{CertType: "user", Principals: []string{"name"}}, pub},
-			&SignSSHOptions{CertType: "user", Principals: []string{"name"},
+			&SignSSHOptions{CertType: "user", Principals: []string{"name", "name@smallstep.com"},
 				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
-		{"admin", p3, args{okAdmin, SignSSHOptions{}, pub}, expectedAdminOptions, http.StatusOK, false, false},
+		{"admin-user", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "root@example.com", Principals: []string{"root", "root@example.com"}}, pub},
-		{"admin-user", p3, args{okAdmin, SignSSHOptions{CertType: "user"}, pub}, expectedAdminOptions, http.StatusOK, false, false},
+			expectedAdminOptions, http.StatusOK, false, false},
-		{"admin-principals", p3, args{okAdmin, SignSSHOptions{Principals: []string{"root"}}, pub},
+		{"admin-host", p3, args{okAdmin, SignSSHOptions{CertType: "host", KeyID: "smallstep.com", Principals: []string{"smallstep.com"}}, pub},
-			&SignSSHOptions{CertType: "user", Principals: []string{"root"},
-				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
-		{"admin-options", p3, args{okAdmin, SignSSHOptions{CertType: "user", Principals: []string{"name"}}, pub},
-			&SignSSHOptions{CertType: "user", Principals: []string{"name"},
-				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
-		{"admin-host", p3, args{okAdmin, SignSSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, pub},
 			expectedHostOptions, http.StatusOK, false, false},
+		{"admin-options", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "name", Principals: []string{"name"}}, pub},
+			&SignSSHOptions{CertType: "user", Principals: []string{"name"},
+				ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false},
 		{"fail-rsa1024", p1, args{t1, SignSSHOptions{}, rsa1024.Public()}, expectedUserOptions, http.StatusOK, false, true},
 		{"fail-user-host", p1, args{t1, SignSSHOptions{CertType: "host"}, pub}, nil, http.StatusOK, false, true},
 		{"fail-user-principals", p1, args{t1, SignSSHOptions{Principals: []string{"root"}}, pub}, nil, http.StatusOK, false, true},
 		{"fail-email", p3, args{failEmail, SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false},
 		{"fail-getIdentity", p5, args{failGetIdentityToken, SignSSHOptions{}, pub}, nil, http.StatusInternalServerError, true, false},
 		{"fail-sshCA-disabled", p6, args{"foo", SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false},
+		// Missing parametrs
+		{"fail-admin-type", p3, args{okAdmin, SignSSHOptions{KeyID: "root@example.com", Principals: []string{"root@example.com"}}, pub}, nil, http.StatusUnauthorized, false, true},
+		{"fail-admin-key-id", p3, args{okAdmin, SignSSHOptions{CertType: "user", Principals: []string{"root@example.com"}}, pub}, nil, http.StatusUnauthorized, false, true},
+		{"fail-admin-principals", p3, args{okAdmin, SignSSHOptions{CertType: "user", KeyID: "root@example.com"}, pub}, nil, http.StatusUnauthorized, false, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

authority/provisioner/sign_ssh_options.go

@@ -47,7 +47,7 @@ type SignSSHOptions struct {
 	Principals   []string        `json:"principals"`
 	ValidAfter   TimeDuration    `json:"validAfter,omitempty"`
 	ValidBefore  TimeDuration    `json:"validBefore,omitempty"`
-	TemplateData json.RawMessage `json:"templateData"`
+	TemplateData json.RawMessage `json:"templateData,omitempty"`
 	Backdate     time.Duration   `json:"-"`
 }
 

authority/provisioner/sign_ssh_options_test.go

@@ -525,11 +525,6 @@ func Test_sshCertDefaultValidator_Valid(t *testing.T) {
 			&ssh.Certificate{Nonce: []byte("foo"), Key: sshPub, Serial: 1, CertType: 1},
 			errors.New("ssh certificate key id cannot be empty"),
 		},
-		{
-			"fail/empty-valid-principals",
-			&ssh.Certificate{Nonce: []byte("foo"), Key: sshPub, Serial: 1, CertType: 1, KeyId: "foo"},
-			errors.New("ssh certificate valid principals cannot be empty"),
-		},
 		{
 			"fail/zero-validAfter",
 			&ssh.Certificate{
@@ -571,20 +566,6 @@ func Test_sshCertDefaultValidator_Valid(t *testing.T) {
 			},
 			errors.New("ssh certificate validBefore cannot be before validAfter"),
 		},
-		{
-			"fail/empty-extensions",
-			&ssh.Certificate{
-				Nonce:           []byte("foo"),
-				Key:             sshPub,
-				Serial:          1,
-				CertType:        1,
-				KeyId:           "foo",
-				ValidPrincipals: []string{"foo"},
-				ValidAfter:      uint64(time.Now().Unix()),
-				ValidBefore:     uint64(time.Now().Add(10 * time.Minute).Unix()),
-			},
-			errors.New("ssh certificate extensions cannot be empty"),
-		},
 		{
 			"fail/nil-signature-key",
 			&ssh.Certificate{
@@ -655,6 +636,38 @@ func Test_sshCertDefaultValidator_Valid(t *testing.T) {
 			},
 			nil,
 		},
+		{
+			"ok/emptyPrincipals",
+			&ssh.Certificate{
+				Nonce:           []byte("foo"),
+				Key:             sshPub,
+				Serial:          1,
+				CertType:        1,
+				KeyId:           "foo",
+				ValidPrincipals: []string{},
+				ValidAfter:      uint64(time.Now().Unix()),
+				ValidBefore:     uint64(time.Now().Add(10 * time.Minute).Unix()),
+				SignatureKey:    sshPub,
+				Signature:       &ssh.Signature{},
+			},
+			nil,
+		},
+		{
+			"ok/empty-extensions",
+			&ssh.Certificate{
+				Nonce:           []byte("foo"),
+				Key:             sshPub,
+				Serial:          1,
+				CertType:        1,
+				KeyId:           "foo",
+				ValidPrincipals: []string{},
+				ValidAfter:      uint64(time.Now().Unix()),
+				ValidBefore:     uint64(time.Now().Add(10 * time.Minute).Unix()),
+				SignatureKey:    sshPub,
+				Signature:       &ssh.Signature{},
+			},
+			nil,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

authority/provisioner/ssh_test.go

@@ -2,11 +2,13 @@ package provisioner
 
 import (
 	"crypto"
-	"crypto/rand"
 	"fmt"
+	"net/http"
 	"reflect"
 	"time"
 
+	"github.com/smallstep/certificates/errs"
+	"github.com/smallstep/certificates/sshutil"
 	"golang.org/x/crypto/ssh"
 )
 
@@ -46,10 +48,14 @@ func signSSHCertificate(key crypto.PublicKey, opts SignSSHOptions, signOpts []Si
 	}
 
 	var mods []SSHCertModifier
+	var certOptions []sshutil.Option
 	var validators []SSHCertValidator
 
 	for _, op := range signOpts {
 		switch o := op.(type) {
+		// add options to NewCertificate
+		case SSHCertificateOptions:
+			certOptions = append(certOptions, o.Options(opts)...)
 		// modify the ssh.Certificate
 		case SSHCertModifier:
 			mods = append(mods, o)
@@ -66,22 +72,39 @@ func signSSHCertificate(key crypto.PublicKey, opts SignSSHOptions, signOpts []Si
 		}
 	}
 
-	// Build base certificate with the key and some random values
+	// Simulated certificate request with request options.
-	cert := &ssh.Certificate{
+	cr := sshutil.CertificateRequest{
-		Nonce:  []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 0},
+		Type:       opts.CertType,
+		KeyID:      opts.KeyID,
+		Principals: opts.Principals,
 		Key:        pub,
-		Serial: 1234567890,
 	}
 
-	// Use opts to modify the certificate
+	// Create certificate from template.
-	if err := opts.Modify(cert, opts); err != nil {
+	certificate, err := sshutil.NewCertificate(cr, certOptions...)
-		return nil, err
+	if err != nil {
+		if _, ok := err.(*sshutil.TemplateError); ok {
+			return nil, errs.NewErr(http.StatusBadRequest, err,
+				errs.WithMessage(err.Error()),
+				errs.WithKeyVal("signOptions", signOpts),
+			)
+		}
+		return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.SignSSH")
 	}
 
-	// Use provisioner modifiers
+	// Get actual *ssh.Certificate and continue with provisioner modifiers.
+	cert := certificate.GetCertificate()
+
+	// Use SignSSHOptions to modify the certificate validity. It will be later
+	// checked or set if not defined.
+	if err := opts.ModifyValidity(cert); err != nil {
+		return nil, errs.Wrap(http.StatusBadRequest, err, "authority.SignSSH")
+	}
+
+	// Use provisioner modifiers.
 	for _, m := range mods {
 		if err := m.Modify(cert, opts); err != nil {
-			return nil, err
+			return nil, errs.Wrap(http.StatusForbidden, err, "authority.SignSSH")
 		}
 	}
 
@@ -98,23 +121,17 @@ func signSSHCertificate(key crypto.PublicKey, opts SignSSHOptions, signOpts []Si
 	if err != nil {
 		return nil, err
 	}
-	cert.SignatureKey = signer.PublicKey()
 
-	// Get bytes for signing trailing the signature length.
+	// Sign certificate.
-	data := cert.Marshal()
+	cert, err = sshutil.CreateCertificate(cert, signer)
-	data = data[:len(data)-4]
-
-	// Sign the certificate
-	sig, err := signer.Sign(rand.Reader, data)
 	if err != nil {
-		return nil, err
+		return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.SignSSH: error signing certificate")
 	}
-	cert.Signature = sig
 
-	// User provisioners validators
+	// User provisioners validators.
 	for _, v := range validators {
 		if err := v.Valid(cert, opts); err != nil {
-			return nil, err
+			return nil, errs.Wrap(http.StatusForbidden, err, "authority.SignSSH")
 		}
 	}
 

authority/provisioner/x5c_test.go

@@ -698,6 +698,7 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) {
 				},
 				Step: &stepPayload{SSH: &SignSSHOptions{
 					CertType:    SSHHostCert,
+					KeyID:       "foo",
 					Principals:  []string{"max", "mariano", "alan"},
 					ValidAfter:  TimeDuration{d: 5 * time.Minute},
 					ValidBefore: TimeDuration{d: 10 * time.Minute},
@@ -753,19 +754,19 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) {
 				if assert.Nil(t, tc.err) {
 					if assert.NotNil(t, opts) {
 						tot := 0
+						firstValidator := true
 						nw := now()
 						for _, o := range opts {
 							switch v := o.(type) {
 							case sshCertOptionsValidator:
 								tc.claims.Step.SSH.ValidAfter.t = time.Time{}
 								tc.claims.Step.SSH.ValidBefore.t = time.Time{}
+								if firstValidator {
 									assert.Equals(t, SignSSHOptions(v), *tc.claims.Step.SSH)
-							case sshCertKeyIDModifier:
+								} else {
-								assert.Equals(t, string(v), "foo")
+									assert.Equals(t, SignSSHOptions(v), SignSSHOptions{KeyID: tc.claims.Subject})
-							case sshCertTypeModifier:
+								}
-								assert.Equals(t, string(v), tc.claims.Step.SSH.CertType)
+								firstValidator = false
-							case sshCertPrincipalsModifier:
-								assert.Equals(t, []string(v), tc.claims.Step.SSH.Principals)
 							case sshCertValidAfterModifier:
 								assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidAfter.RelativeTime(nw).Unix())
 							case sshCertValidBeforeModifier:
@@ -777,19 +778,16 @@ func TestX5C_AuthorizeSSHSign(t *testing.T) {
 								assert.Equals(t, v.NotAfter, x5cCerts[0].NotAfter)
 							case *sshCertValidityValidator:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
-							case *sshDefaultExtensionModifier, *sshDefaultPublicKeyValidator,
+							case *sshDefaultPublicKeyValidator, *sshCertDefaultValidator, sshCertificateOptionsFunc:
-								*sshCertDefaultValidator:
-							case sshCertKeyIDValidator:
-								assert.Equals(t, string(v), "foo")
 							default:
 								assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
 							}
 							tot++
 						}
 						if len(tc.claims.Step.SSH.CertType) > 0 {
-							assert.Equals(t, tot, 13)
-						} else {
 							assert.Equals(t, tot, 9)
+						} else {
+							assert.Equals(t, tot, 7)
 						}
 					}
 				}