diff --git a/acme/api/order_test.go b/acme/api/order_test.go index 38c4c3f0..afb23c3f 100644 --- a/acme/api/order_test.go +++ b/acme/api/order_test.go @@ -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", @@ -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) } diff --git a/acme/challenge.go b/acme/challenge.go index fe643c85..1d5f0ec9 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -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 { diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 423951e8..bb9a2507 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -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", diff --git a/acme/order.go b/acme/order.go index 71884603..2a869e78 100644 --- a/acme/order.go +++ b/acme/order.go @@ -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. diff --git a/acme/order_test.go b/acme/order_test.go index e1d3744b..67d01f6d 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -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) { @@ -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) + } + }) + } +}