Skip to content

Commit

Permalink
Add tests for canonicalize function
Browse files Browse the repository at this point in the history
  • Loading branch information
hslatman committed Jun 25, 2021
1 parent 7843c90 commit 64c15fd
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 7 deletions.
3 changes: 1 addition & 2 deletions acme/api/order_test.go
Expand Up @@ -1678,7 +1678,7 @@ func TestHandler_challengeTypes(t *testing.T) {
Wildcard: false,
},
},
want: []acme.ChallengeType{acme.DNS01, acme.HTTP01, acme.TLSALPN01}, //[]string{"dns-01", "http-01", "tls-alpn-01"},
want: []acme.ChallengeType{acme.DNS01, acme.HTTP01, acme.TLSALPN01},
},
{
name: "ok/wildcard",
Expand All @@ -1703,7 +1703,6 @@ func TestHandler_challengeTypes(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

if got := challengeTypes(tt.args.az); !reflect.DeepEqual(got, tt.want) {
t.Errorf("Handler.challengeTypes() = %v, want %v", got, tt.want)
}
Expand Down
2 changes: 1 addition & 1 deletion acme/challenge.go
Expand Up @@ -261,7 +261,7 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK
}

// serverName determines the SNI HostName to set based on an acme.Challenge
// for TLS-ALPN-01 challenges. RFC8738 states that, if HostName is an IP, it
// for TLS-ALPN-01 challenges RFC8738 states that, if HostName is an IP, it
// should be the ARPA address https://datatracker.ietf.org/doc/html/rfc8738#section-6.
// It also references TLS Extensions [RFC6066].
func serverName(ch *Challenge) string {
Expand Down
2 changes: 1 addition & 1 deletion acme/challenge_test.go
Expand Up @@ -2334,7 +2334,7 @@ func Test_serverName(t *testing.T) {
want: "1.0.0.127.in-addr.arpa.",
},
{
name: "ok/ipv4",
name: "ok/ipv6",
args: args{
ch: &Challenge{
Value: "2001:db8::567:89ab",
Expand Down
3 changes: 0 additions & 3 deletions acme/order.go
Expand Up @@ -221,9 +221,6 @@ func (o *Order) sans(csr *x509.CertificateRequest) ([]x509util.SubjectAlternativ
sans = make([]x509util.SubjectAlternativeName, totalNumberOfSANs)
index := 0

// TODO: limit what IP addresses can be used? Only private? Only certain ranges (i.e. only allow the specific ranges by default, configuration for all?)
// TODO: can DNS already be limited to a certain domain? That would probably be nice to have too, but maybe not as part of this PR
// TODO: if it seems not too big of a change, make consts/enums out of the stringly typed identifiers (challenge types, identifier types)
// TODO: only allow IP based identifier based on configuration? Some additional configuration and validation on the provisioner for this case.

// Validate identifier names against CSR alternative names.
Expand Down
153 changes: 153 additions & 0 deletions acme/order_test.go
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/pkg/errors"
"github.com/smallstep/assert"
"github.com/smallstep/certificates/authority/provisioner"
"go.step.sm/crypto/x509util"
)

func TestOrder_UpdateStatus(t *testing.T) {
Expand Down Expand Up @@ -1187,3 +1188,155 @@ func Test_ipsAreEqual(t *testing.T) {
})
}
}

func Test_canonicalize(t *testing.T) {
type args struct {
csr *x509.CertificateRequest
}
tests := []struct {
name string
args args
wantCanonicalized *x509.CertificateRequest
}{
{
name: "ok/dns",
args: args{
csr: &x509.CertificateRequest{
DNSNames: []string{"www.example.com", "example.com"},
},
},
wantCanonicalized: &x509.CertificateRequest{
DNSNames: []string{"example.com", "www.example.com"},
IPAddresses: []net.IP{},
},
},
{
name: "ok/common-name",
args: args{
csr: &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: "example.com",
},
DNSNames: []string{"www.example.com"},
},
},
wantCanonicalized: &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: "example.com",
},
DNSNames: []string{"example.com", "www.example.com"},
IPAddresses: []net.IP{},
},
},
{
name: "ok/ipv4",
args: args{
csr: &x509.CertificateRequest{
IPAddresses: []net.IP{net.ParseIP("192.168.43.42"), net.ParseIP("192.168.42.42")},
},
},
wantCanonicalized: &x509.CertificateRequest{
DNSNames: []string{},
IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")},
},
},
{
name: "ok/mixed",
args: args{
csr: &x509.CertificateRequest{
DNSNames: []string{"www.example.com", "example.com"},
IPAddresses: []net.IP{net.ParseIP("192.168.43.42"), net.ParseIP("192.168.42.42")},
},
},
wantCanonicalized: &x509.CertificateRequest{
DNSNames: []string{"example.com", "www.example.com"},
IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")},
},
},
{
name: "ok/mixed-common-name",
args: args{
csr: &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: "example.com",
},
DNSNames: []string{"www.example.com"},
IPAddresses: []net.IP{net.ParseIP("192.168.43.42"), net.ParseIP("192.168.42.42")},
},
},
wantCanonicalized: &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: "example.com",
},
DNSNames: []string{"example.com", "www.example.com"},
IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if gotCanonicalized := canonicalize(tt.args.csr); !reflect.DeepEqual(gotCanonicalized, tt.wantCanonicalized) {
t.Errorf("canonicalize() = %v, want %v", gotCanonicalized, tt.wantCanonicalized)
}
})
}
}

func TestOrder_sans(t *testing.T) {
type fields struct {
ID string
AccountID string
ProvisionerID string
Status Status
ExpiresAt time.Time
Identifiers []Identifier
NotBefore time.Time
NotAfter time.Time
Error *Error
AuthorizationIDs []string
AuthorizationURLs []string
FinalizeURL string
CertificateID string
CertificateURL string
}
type args struct {
csr *x509.CertificateRequest
}
tests := []struct {
name string
fields fields
args args
want []x509util.SubjectAlternativeName
wantErr bool
}{
// TODO: Add test cases.
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o := &Order{
ID: tt.fields.ID,
AccountID: tt.fields.AccountID,
ProvisionerID: tt.fields.ProvisionerID,
Status: tt.fields.Status,
ExpiresAt: tt.fields.ExpiresAt,
Identifiers: tt.fields.Identifiers,
NotBefore: tt.fields.NotBefore,
NotAfter: tt.fields.NotAfter,
Error: tt.fields.Error,
AuthorizationIDs: tt.fields.AuthorizationIDs,
AuthorizationURLs: tt.fields.AuthorizationURLs,
FinalizeURL: tt.fields.FinalizeURL,
CertificateID: tt.fields.CertificateID,
CertificateURL: tt.fields.CertificateURL,
}
got, err := o.sans(tt.args.csr)
if (err != nil) != tt.wantErr {
t.Errorf("Order.sans() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Order.sans() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 64c15fd

Please sign in to comment.