Skip to content

Commit

Permalink
Do not mutate input in Sign and SSHSign.
Browse files Browse the repository at this point in the history 
This commit also adds more documentation on which fields will be
automatically pupulated if they are not set.
  • Loading branch information
@maraino
maraino committed Mar 16, 2022
1 parent 99fbede commit cd2e52d
Show file tree
Hide file tree
Showing 2 changed files with 227 additions and 37 deletions.
56 changes: 38 additions & 18 deletions minica/minica.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,26 @@ func New(opts ...Option) (*CA, error) {
}

// Sign signs an X.509 certificate template using the intermediate certificate.
// Sign will automatically populate the following fields if they are not
// specified:
//
// - NotBefore will be set to the current time.
// - NotAfter will be set to 24 hours after NotBefore.
// - SerialNumber will be automatically generated.
// - SubjectKeyId will be automatically generated.
func (c *CA) Sign(template *x509.Certificate) (*x509.Certificate, error) {
if template.NotBefore.IsZero() {
template.NotBefore = time.Now()
mut := *template
if mut.NotBefore.IsZero() {
mut.NotBefore = time.Now()
}
if template.NotAfter.IsZero() {
template.NotAfter = template.NotBefore.Add(24 * time.Hour)
if mut.NotAfter.IsZero() {
mut.NotAfter = mut.NotBefore.Add(24 * time.Hour)
}
return x509util.CreateCertificate(template, c.Intermediate, template.PublicKey, c.Signer)
return x509util.CreateCertificate(&mut, c.Intermediate, mut.PublicKey, c.Signer)
}

// SignCSR signs an X.509 certificate signing request. The custom options allows to change the template used for
// SignCSR signs an X.509 certificate signing request. The custom options allows
// to change the template used to convert the CSR to a certificate.
func (c *CA) SignCSR(csr *x509.CertificateRequest, opts ...SignOption) (*x509.Certificate, error) {
sans := append([]string{}, csr.DNSNames...)
sans = append(sans, csr.EmailAddresses...)
Expand All @@ -137,20 +146,31 @@ func (c *CA) SignCSR(csr *x509.CertificateRequest, opts ...SignOption) (*x509.Ce
return c.Sign(cert)
}

// SignSSH signs an SSH host or user certificate.
func (c *CA) SignSSH(cert *ssh.Certificate) (*ssh.Certificate, error) {
if cert.ValidAfter == 0 {
cert.ValidAfter = uint64(time.Now().Unix())
}
if cert.ValidBefore == 0 {
cert.ValidBefore = cert.ValidAfter + 24*60*60
}

switch cert.CertType {
// SignSSH signs an SSH host or user certificate. SignSSH will automatically
// populate the following fields if they are not specified:
//
// - ValidAfter will be set to the current time unless ValidBefore is set to ssh.CertTimeInfinity.
// - ValidBefore will be set to 24 hours after ValidAfter.
// - Nonce will be automatically generated.
// - Serial will be automatically generated.
//
// If the SSH signer is an RSA key, it will use rsa-sha2-256 instead of the
// default ssh-rsa (SHA-1), this method is currently deprecated and
// rsa-sha2-256/512 are supported since OpenSSH 7.2 (2016).
func (c *CA) SignSSH(template *ssh.Certificate) (*ssh.Certificate, error) {
mut := *template
if mut.ValidAfter == 0 && mut.ValidBefore != ssh.CertTimeInfinity {
mut.ValidAfter = uint64(time.Now().Unix())
}
if mut.ValidBefore == 0 {
mut.ValidBefore = mut.ValidAfter + 24*60*60
}

switch mut.CertType {
case ssh.HostCert:
return sshutil.CreateCertificate(cert, c.SSHHostSigner)
return sshutil.CreateCertificate(&mut, c.SSHHostSigner)
case ssh.UserCert:
return sshutil.CreateCertificate(cert, c.SSHUserSigner)
return sshutil.CreateCertificate(&mut, c.SSHUserSigner)
default:
return nil, fmt.Errorf("unknown certificate type")
}
Expand Down
208 changes: 189 additions & 19 deletions minica/minica_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,33 +122,33 @@ func TestNew(t *testing.T) {
}
} else {
if got.Root == nil {
t.Errorf("MiniCA.Root should not be nil")
t.Errorf("CA.Root should not be nil")
}
if got.Intermediate == nil {
t.Errorf("MiniCA.Intermediate should not be nil")
t.Errorf("CA.Intermediate should not be nil")
}
if got.Signer == nil {
t.Errorf("MiniCA.Signer should not be nil")
t.Errorf("CA.Signer should not be nil")
}
if got.SSHHostSigner == nil {
t.Errorf("MiniCA.SSHHostSigner should not be nil")
t.Errorf("CA.SSHHostSigner should not be nil")
}
if got.SSHUserSigner == nil {
t.Errorf("MiniCA.SSHUserSigner should not be nil")
t.Errorf("CA.SSHUserSigner should not be nil")
}

// Check common names
if cn := got.Root.Subject.CommonName; cn != tt.wantName+" Root CA" {
t.Errorf("MiniCA.Root.Subject.CommonName = %s, want %s Root CA", cn, tt.wantName)
t.Errorf("CA.Root.Subject.CommonName = %s, want %s Root CA", cn, tt.wantName)
}
if cn := got.Root.Issuer.CommonName; cn != tt.wantName+" Root CA" {
t.Errorf("MiniCA.Root.Issuer.CommonName = %s, want %s Root CA", cn, tt.wantName)
t.Errorf("CA.Root.Issuer.CommonName = %s, want %s Root CA", cn, tt.wantName)
}
if cn := got.Intermediate.Subject.CommonName; cn != tt.wantName+" Intermediate CA" {
t.Errorf("MiniCA.Intermediate.Subject.CommonName = %s, want %s Intermediate CA", cn, tt.wantName)
t.Errorf("CA.Intermediate.Subject.CommonName = %s, want %s Intermediate CA", cn, tt.wantName)
}
if cn := got.Intermediate.Issuer.CommonName; cn != tt.wantName+" Root CA" {
t.Errorf("MiniCA.Root.Intermediate.Issuer.CommonName = %s, want %s Root CA", cn, tt.wantName)
t.Errorf("CA.Root.Intermediate.Issuer.CommonName = %s, want %s Root CA", cn, tt.wantName)
}

// Verify intermediate
Expand All @@ -157,14 +157,14 @@ func TestNew(t *testing.T) {
if _, err := got.Intermediate.Verify(x509.VerifyOptions{
Roots: pool,
}); err != nil {
t.Errorf("MiniCA.Intermediate.Verify() error = %v", err)
t.Errorf("CA.Intermediate.Verify() error = %v", err)
}
}
})
}
}

func TestMiniCA_Sign(t *testing.T) {
func TestCA_Sign(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -198,12 +198,12 @@ func TestMiniCA_Sign(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.ca.Sign(tt.args.template)
if (err != nil) != tt.wantErr {
t.Errorf("MiniCA.Sign() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("CA.Sign() error = %v, wantErr %v", err, tt.wantErr)
return
}
if tt.wantErr {
if got != nil {
t.Errorf("MiniCA.Sign() = %v, want nil", got)
t.Errorf("CA.Sign() = %v, want nil", got)
}
} else {
roots := x509.NewCertPool()
Expand All @@ -224,7 +224,65 @@ func TestMiniCA_Sign(t *testing.T) {
}
}

func TestMiniCA_SignCSR(t *testing.T) {
func TestCA_Sign_mutation(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
}

ca := mustCA(t)
template := &x509.Certificate{
DNSNames: []string{"leaf.test.com"},
PublicKey: signer.Public(),
}
got, err := ca.Sign(template)
if err != nil {
t.Fatal(err)
}

// Verify certificate
roots := x509.NewCertPool()
roots.AddCert(ca.Root)
ints := x509.NewCertPool()
ints.AddCert(ca.Intermediate)
if _, err := got.Verify(x509.VerifyOptions{
Roots: roots,
Intermediates: ints,
DNSName: "leaf.test.com",
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
}); err != nil {
t.Errorf("Certificate.Verify() error = %v", err)
}

// Check mutation
if !template.NotBefore.IsZero() {
t.Errorf("CA.Sign() mutated template.NotBefore")
}
if !template.NotAfter.IsZero() {
t.Errorf("CA.Sign() mutated template.NotAfter")
}
if template.SerialNumber != nil {
t.Errorf("CA.Sign() mutated template.SerialNumber")
}
if template.SubjectKeyId != nil {
t.Errorf("CA.Sign() mutated template.SubjectKeyId")
}

if got.NotBefore.IsZero() {
t.Errorf("CA.Sign() got.NotBefore should not be 0")
}
if got.NotAfter.IsZero() {
t.Errorf("CA.Sign() got.NotAfter should not be 0")
}
if got.SerialNumber == nil {
t.Errorf("CA.Sign() got.SerialNumber should not be nil")
}
if got.SubjectKeyId == nil {
t.Errorf("CA.Sign() got.SubjectKeyId should not be nil")
}
}

func TestCA_SignCSR(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -261,12 +319,12 @@ func TestMiniCA_SignCSR(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.ca.SignCSR(tt.args.csr, tt.args.opts...)
if (err != nil) != tt.wantErr {
t.Errorf("MiniCA.SignCSR() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("CA.SignCSR() error = %v, wantErr %v", err, tt.wantErr)
return
}
if tt.wantErr {
if got != nil {
t.Errorf("MiniCA.Sign() = %v, want nil", got)
t.Errorf("CA.Sign() = %v, want nil", got)
}
} else {
roots := x509.NewCertPool()
Expand All @@ -287,7 +345,7 @@ func TestMiniCA_SignCSR(t *testing.T) {
}
}

func TestMiniCA_SignSSH(t *testing.T) {
func TestCA_SignSSH(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -324,6 +382,14 @@ func TestMiniCA_SignSSH(t *testing.T) {
ValidAfter: uint64(time.Now().Unix()),
ValidBefore: uint64(time.Now().Add(time.Hour).Unix()),
}}, ssh.UserCert, "jane", false},
{"ok infinity", mustCA(t), args{&ssh.Certificate{
Key: publicKey,
Serial: 1234,
CertType: ssh.UserCert,
KeyId: "jane@test.com",
ValidPrincipals: []string{"jane"},
ValidBefore: ssh.CertTimeInfinity,
}}, ssh.UserCert, "jane", false},
{"fail type", mustCA(t), args{&ssh.Certificate{
Key: publicKey,
Serial: 1234,
Expand All @@ -336,13 +402,13 @@ func TestMiniCA_SignSSH(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.ca.SignSSH(tt.args.cert)
if (err != nil) != tt.wantErr {
t.Errorf("MiniCA.SignSSH() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("CA.SignSSH() error = %v, wantErr %v", err, tt.wantErr)
return
}

if tt.wantErr {
if got != nil {
t.Errorf("MiniCA.SignSSH() = %v, want nil", got)
t.Errorf("CA.SignSSH() = %v, want nil", got)
}
} else {
checker := ssh.CertChecker{
Expand All @@ -369,3 +435,107 @@ func TestMiniCA_SignSSH(t *testing.T) {
})
}
}

func TestCA_SignSSH_mutation(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
}
publicKey, err := ssh.NewPublicKey(signer.Public())
if err != nil {
t.Fatal(err)
}

template := &ssh.Certificate{
Key: publicKey,
CertType: ssh.HostCert,
KeyId: "ssh.test.com",
ValidPrincipals: []string{"ssh.test.com"},
}

ca := mustCA(t)
got, err := ca.SignSSH(template)
if err != nil {
t.Fatal(err)
}

// Validate certificate
checker := ssh.CertChecker{
IsHostAuthority: func(auth ssh.PublicKey, address string) bool {
return reflect.DeepEqual(auth, ca.SSHHostSigner.PublicKey())
},
}
if err := checker.CheckHostKey("ssh.test.com:22", &net.IPAddr{IP: net.IP{1, 2, 3, 4}}, got); err != nil {
t.Errorf("CertChecker.CheckHostKey() error = %v", err)
}

// Validate mutation
if template.ValidAfter != 0 {
t.Errorf("CA.SignSSH() mutated template.ValidAfter")
}
if template.ValidBefore != 0 {
t.Errorf("CA.SignSSH() mutated template.ValidBefore")
}
if template.Nonce != nil {
t.Errorf("CA.SignSSH() mutated template.Nonce")
}
if template.Serial != 0 {
t.Errorf("CA.SignSSH() mutated template.Serial")
}

if got.ValidAfter == 0 {
t.Errorf("CA.SignSSH() got.ValidAfter should not be 0")
}
if got.ValidBefore == 0 {
t.Errorf("CA.SignSSH() got.ValidBefore should not be 0")
}
if len(got.Nonce) == 0 {
t.Errorf("CA.SignSSH() got.Nonce should not be empty")
}
if got.Serial == 0 {
t.Errorf("CA.SignSSH() got.Serial should not be 0")
}
}

func TestCA_SignSSH_infinity(t *testing.T) {
signer, err := keyutil.GenerateDefaultSigner()
if err != nil {
t.Fatal(err)
}
publicKey, err := ssh.NewPublicKey(signer.Public())
if err != nil {
t.Fatal(err)
}

template := &ssh.Certificate{
Key: publicKey,
Serial: 1234,
CertType: ssh.UserCert,
KeyId: "jane@test.com",
ValidPrincipals: []string{"jane"},
ValidBefore: ssh.CertTimeInfinity,
}

ca := mustCA(t)
got, err := ca.SignSSH(template)
if err != nil {
t.Fatal(err)
}

// Validate certificate
checker := ssh.CertChecker{
IsUserAuthority: func(auth ssh.PublicKey) bool {
return reflect.DeepEqual(auth, ca.SSHUserSigner.PublicKey())
},
}
if _, err := checker.Authenticate(mockConnMetadata("jane"), got); err != nil {
t.Errorf("CertChecker.Authenticate() error = %v", err)
}

if got.ValidAfter != 0 {
t.Errorf("CA.SignSSH() got.ValidAfter should be 0")
}
if got.ValidBefore != ssh.CertTimeInfinity {
t.Errorf("CA.SignSSH() got.ValidBefore should not be ssh.CertTimInfinity")
}
}

0 comments on commit cd2e52d

Please sign in to comment.