diff --git a/acme/api/handler_test.go b/acme/api/handler_test.go index 5501479d..f354bbac 100644 --- a/acme/api/handler_test.go +++ b/acme/api/handler_test.go @@ -574,13 +574,13 @@ func TestHandler_GetChallenge(t *testing.T) { assert.Equals(t, azID, "authzID") return &acme.Challenge{ Status: acme.StatusPending, - Type: "http-01", + Type: acme.HTTP01, AccountID: "accID", }, nil }, MockUpdateChallenge: func(ctx context.Context, ch *acme.Challenge) error { assert.Equals(t, ch.Status, acme.StatusPending) - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) assert.Equals(t, ch.AccountID, "accID") assert.Equals(t, ch.AuthorizationID, "authzID") assert.HasSuffix(t, ch.Error.Type, acme.ErrorConnectionType.String()) @@ -616,13 +616,13 @@ func TestHandler_GetChallenge(t *testing.T) { return &acme.Challenge{ ID: "chID", Status: acme.StatusPending, - Type: "http-01", + Type: acme.HTTP01, AccountID: "accID", }, nil }, MockUpdateChallenge: func(ctx context.Context, ch *acme.Challenge) error { assert.Equals(t, ch.Status, acme.StatusPending) - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) assert.Equals(t, ch.AccountID, "accID") assert.Equals(t, ch.AuthorizationID, "authzID") assert.HasSuffix(t, ch.Error.Type, acme.ErrorConnectionType.String()) @@ -633,7 +633,7 @@ func TestHandler_GetChallenge(t *testing.T) { ID: "chID", Status: acme.StatusPending, AuthorizationID: "authzID", - Type: "http-01", + Type: acme.HTTP01, AccountID: "accID", URL: url, Error: acme.NewError(acme.ErrorConnectionType, "force"), diff --git a/acme/api/order.go b/acme/api/order.go index 9d410173..9cf2c1eb 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "encoding/base64" "encoding/json" + "net" "net/http" "strings" "time" @@ -28,9 +29,12 @@ func (n *NewOrderRequest) Validate() error { return acme.NewError(acme.ErrorMalformedType, "identifiers list cannot be empty") } for _, id := range n.Identifiers { - if id.Type != "dns" { + if !(id.Type == acme.DNS || id.Type == acme.IP) { return acme.NewError(acme.ErrorMalformedType, "identifier type unsupported: %s", id.Type) } + if id.Type == acme.IP && net.ParseIP(id.Value) == nil { + return acme.NewError(acme.ErrorMalformedType, "invalid IP address: %s", id.Value) + } } return nil } @@ -85,6 +89,7 @@ func (h *Handler) NewOrder(w http.ResponseWriter, r *http.Request) { "failed to unmarshal new-order request payload")) return } + if err := nor.Validate(); err != nil { api.WriteError(w, err) return @@ -149,15 +154,9 @@ func (h *Handler) newAuthorization(ctx context.Context, az *acme.Authorization) } } - var ( - err error - chTypes = []string{"dns-01"} - ) - // HTTP and TLS challenges can only be used for identifiers without wildcards. - if !az.Wildcard { - chTypes = append(chTypes, []string{"http-01", "tls-alpn-01"}...) - } + chTypes := challengeTypes(az) + var err error az.Token, err = randutil.Alphanumeric(32) if err != nil { return acme.WrapErrorISE(err, "error generating random alphanumeric ID") @@ -275,3 +274,24 @@ func (h *Handler) FinalizeOrder(w http.ResponseWriter, r *http.Request) { w.Header().Set("Location", h.linker.GetLink(ctx, OrderLinkType, o.ID)) api.JSON(w, o) } + +// challengeTypes determines the types of challenges that should be used +// for the ACME authorization request. +func challengeTypes(az *acme.Authorization) []acme.ChallengeType { + var chTypes []acme.ChallengeType + + switch az.Identifier.Type { + case acme.IP: + chTypes = []acme.ChallengeType{acme.HTTP01, acme.TLSALPN01} + case acme.DNS: + chTypes = []acme.ChallengeType{acme.DNS01} + // HTTP and TLS challenges can only be used for identifiers without wildcards. + if !az.Wildcard { + chTypes = append(chTypes, []acme.ChallengeType{acme.HTTP01, acme.TLSALPN01}...) + } + default: + chTypes = []acme.ChallengeType{} + } + + return chTypes +} diff --git a/acme/api/order_test.go b/acme/api/order_test.go index 300aa61b..afb23c3f 100644 --- a/acme/api/order_test.go +++ b/acme/api/order_test.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "net/http/httptest" "net/url" + "reflect" "testing" "time" @@ -44,6 +45,22 @@ func TestNewOrderRequest_Validate(t *testing.T) { err: acme.NewError(acme.ErrorMalformedType, "identifier type unsupported: foo"), } }, + "fail/bad-ip": func(t *testing.T) test { + nbf := time.Now().UTC().Add(time.Minute) + naf := time.Now().UTC().Add(5 * time.Minute) + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "ip", Value: "192.168.42.1000"}, + }, + NotAfter: naf, + NotBefore: nbf, + }, + nbf: nbf, + naf: naf, + err: acme.NewError(acme.ErrorMalformedType, "invalid IP address: %s", "192.168.42.1000"), + } + }, "ok": func(t *testing.T) test { nbf := time.Now().UTC().Add(time.Minute) naf := time.Now().UTC().Add(5 * time.Minute) @@ -60,6 +77,68 @@ func TestNewOrderRequest_Validate(t *testing.T) { naf: naf, } }, + "ok/ipv4": func(t *testing.T) test { + nbf := time.Now().UTC().Add(time.Minute) + naf := time.Now().UTC().Add(5 * time.Minute) + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + }, + NotAfter: naf, + NotBefore: nbf, + }, + nbf: nbf, + naf: naf, + } + }, + "ok/ipv6": func(t *testing.T) test { + nbf := time.Now().UTC().Add(time.Minute) + naf := time.Now().UTC().Add(5 * time.Minute) + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "ip", Value: "2001:db8::1"}, + }, + NotAfter: naf, + NotBefore: nbf, + }, + nbf: nbf, + naf: naf, + } + }, + "ok/mixed-dns-and-ipv4": func(t *testing.T) test { + nbf := time.Now().UTC().Add(time.Minute) + naf := time.Now().UTC().Add(5 * time.Minute) + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "dns", Value: "example.com"}, + {Type: "ip", Value: "192.168.42.42"}, + }, + NotAfter: naf, + NotBefore: nbf, + }, + nbf: nbf, + naf: naf, + } + }, + "ok/mixed-ipv4-and-ipv6": func(t *testing.T) test { + nbf := time.Now().UTC().Add(time.Minute) + naf := time.Now().UTC().Add(5 * time.Minute) + return test{ + nor: &NewOrderRequest{ + Identifiers: []acme.Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "2001:db8::1"}, + }, + NotAfter: naf, + NotBefore: nbf, + }, + nbf: nbf, + naf: naf, + } + }, } for name, run := range tests { tc := run(t) @@ -395,7 +474,7 @@ func TestHandler_newAuthorization(t *testing.T) { db: &acme.MockDB{ MockCreateChallenge: func(ctx context.Context, ch *acme.Challenge) error { assert.Equals(t, ch.AccountID, az.AccountID) - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) assert.Equals(t, ch.Token, az.Token) assert.Equals(t, ch.Status, acme.StatusPending) assert.Equals(t, ch.Value, az.Identifier.Value) @@ -424,15 +503,15 @@ func TestHandler_newAuthorization(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -478,15 +557,15 @@ func TestHandler_newAuthorization(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -528,7 +607,7 @@ func TestHandler_newAuthorization(t *testing.T) { db: &acme.MockDB{ MockCreateChallenge: func(ctx context.Context, ch *acme.Challenge) error { ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) assert.Equals(t, ch.AccountID, az.AccountID) assert.Equals(t, ch.Token, az.Token) assert.Equals(t, ch.Status, acme.StatusPending) @@ -695,7 +774,7 @@ func TestHandler_NewOrder(t *testing.T) { db: &acme.MockDB{ MockCreateChallenge: func(ctx context.Context, ch *acme.Challenge) error { assert.Equals(t, ch.AccountID, "accID") - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) assert.NotEquals(t, ch.Token, "") assert.Equals(t, ch.Status, acme.StatusPending) assert.Equals(t, ch.Value, "zap.internal") @@ -730,15 +809,15 @@ func TestHandler_NewOrder(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -802,22 +881,22 @@ func TestHandler_NewOrder(t *testing.T) { switch chCount { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) assert.Equals(t, ch.Value, "zap.internal") ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) assert.Equals(t, ch.Value, "zap.internal") ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) assert.Equals(t, ch.Value, "zap.internal") ch3 = &ch case 3: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) assert.Equals(t, ch.Value, "zar.internal") ch4 = &ch default: @@ -842,7 +921,7 @@ func TestHandler_NewOrder(t *testing.T) { az.ID = "az2ID" az2ID = &az.ID assert.Equals(t, az.Identifier, acme.Identifier{ - Type: "dns", + Type: acme.DNS, Value: "zar.internal", }) assert.Equals(t, az.Wildcard, true) @@ -917,15 +996,15 @@ func TestHandler_NewOrder(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -1009,15 +1088,15 @@ func TestHandler_NewOrder(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -1100,15 +1179,15 @@ func TestHandler_NewOrder(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -1192,15 +1271,15 @@ func TestHandler_NewOrder(t *testing.T) { switch count { case 0: ch.ID = "dns" - assert.Equals(t, ch.Type, "dns-01") + assert.Equals(t, ch.Type, acme.DNS01) ch1 = &ch case 1: ch.ID = "http" - assert.Equals(t, ch.Type, "http-01") + assert.Equals(t, ch.Type, acme.HTTP01) ch2 = &ch case 2: ch.ID = "tls" - assert.Equals(t, ch.Type, "tls-alpn-01") + assert.Equals(t, ch.Type, acme.TLSALPN01) ch3 = &ch default: assert.FatalError(t, errors.New("test logic error")) @@ -1581,3 +1660,52 @@ func TestHandler_FinalizeOrder(t *testing.T) { }) } } + +func TestHandler_challengeTypes(t *testing.T) { + type args struct { + az *acme.Authorization + } + tests := []struct { + name string + args args + want []acme.ChallengeType + }{ + { + name: "ok/dns", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "dns", Value: "example.com"}, + Wildcard: false, + }, + }, + want: []acme.ChallengeType{acme.DNS01, acme.HTTP01, acme.TLSALPN01}, + }, + { + name: "ok/wildcard", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "dns", Value: "*.example.com"}, + Wildcard: true, + }, + }, + want: []acme.ChallengeType{acme.DNS01}, + }, + { + name: "ok/ip", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "ip", Value: "192.168.42.42"}, + Wildcard: false, + }, + }, + want: []acme.ChallengeType{acme.HTTP01, acme.TLSALPN01}, + }, + } + 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 1059e437..1d5f0ec9 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -21,18 +21,26 @@ import ( "go.step.sm/crypto/jose" ) +type ChallengeType string + +const ( + HTTP01 ChallengeType = "http-01" + DNS01 ChallengeType = "dns-01" + TLSALPN01 ChallengeType = "tls-alpn-01" +) + // Challenge represents an ACME response Challenge type. type Challenge struct { - ID string `json:"-"` - AccountID string `json:"-"` - AuthorizationID string `json:"-"` - Value string `json:"-"` - Type string `json:"type"` - Status Status `json:"status"` - Token string `json:"token"` - ValidatedAt string `json:"validated,omitempty"` - URL string `json:"url"` - Error *Error `json:"error,omitempty"` + ID string `json:"-"` + AccountID string `json:"-"` + AuthorizationID string `json:"-"` + Value string `json:"-"` + Type ChallengeType `json:"type"` + Status Status `json:"status"` + Token string `json:"token"` + ValidatedAt string `json:"validated,omitempty"` + URL string `json:"url"` + Error *Error `json:"error,omitempty"` } // ToLog enables response logging. @@ -54,11 +62,11 @@ func (ch *Challenge) Validate(ctx context.Context, db DB, jwk *jose.JSONWebKey, return nil } switch ch.Type { - case "http-01": + case HTTP01: return http01Validate(ctx, ch, db, jwk, vo) - case "dns-01": + case DNS01: return dns01Validate(ctx, ch, db, jwk, vo) - case "tls-alpn-01": + case TLSALPN01: return tlsalpn01Validate(ctx, ch, db, jwk, vo) default: return NewErrorISE("unexpected challenge type '%s'", ch.Type) @@ -113,7 +121,7 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON // ACME servers that implement "acme-tls/1" MUST only negotiate TLS 1.2 // [RFC5246] or higher when connecting to clients for validation. MinVersion: tls.VersionTLS12, - ServerName: ch.Value, + ServerName: serverName(ch), InsecureSkipVerify: true, // we expect a self-signed challenge certificate } @@ -141,9 +149,17 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON leafCert := certs[0] - if len(leafCert.DNSNames) != 1 || !strings.EqualFold(leafCert.DNSNames[0], ch.Value) { - return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType, - "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single DNS name, %v", ch.Value)) + // if no DNS names present, look for IP address and verify that exactly one exists + if len(leafCert.DNSNames) == 0 { + if len(leafCert.IPAddresses) != 1 || !leafCert.IPAddresses[0].Equal(net.ParseIP(ch.Value)) { + return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType, + "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value)) + } + } else { + if len(leafCert.DNSNames) != 1 || !strings.EqualFold(leafCert.DNSNames[0], ch.Value) { + return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType, + "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value)) + } } idPeAcmeIdentifier := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 31} @@ -244,6 +260,65 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK return nil } +// 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 +// 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 { + var serverName string + ip := net.ParseIP(ch.Value) + if ip != nil { + serverName = reverseAddr(ip) + } else { + serverName = ch.Value + } + return serverName +} + +// reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP +// address addr suitable for rDNS (PTR) record lookup or an error if it fails +// to parse the IP address. +// Implementation taken and adapted from https://golang.org/src/net/dnsclient.go?s=780:834#L20 +func reverseAddr(ip net.IP) (arpa string) { + if ip.To4() != nil { + return uitoa(uint(ip[15])) + "." + uitoa(uint(ip[14])) + "." + uitoa(uint(ip[13])) + "." + uitoa(uint(ip[12])) + ".in-addr.arpa." + } + // Must be IPv6 + buf := make([]byte, 0, len(ip)*4+len("ip6.arpa.")) + // Add it, in reverse, to the buffer + for i := len(ip) - 1; i >= 0; i-- { + v := ip[i] + buf = append(buf, hexit[v&0xF], + '.', + hexit[v>>4], + '.') + } + // Append "ip6.arpa." and return (buf already has the final .) + buf = append(buf, "ip6.arpa."...) + return string(buf) +} + +// Convert unsigned integer to decimal string. +// Implementation taken from https://golang.org/src/net/parse.go +func uitoa(val uint) string { + if val == 0 { // avoid string allocation + return "0" + } + var buf [20]byte // big enough for 64bit value base 10 + i := len(buf) - 1 + for val >= 10 { + q := val / 10 + buf[i] = byte('0' + val - q*10) + i-- + val = q + } + // val < 10 + buf[i] = byte('0' + val) + return string(buf[i:]) +} + +const hexit = "0123456789abcdef" + // KeyAuthorization creates the ACME key authorization value from a token // and a jwk. func KeyAuthorization(token string, jwk *jose.JSONWebKey) (string, error) { diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 14287945..bb9a2507 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -1544,7 +1544,7 @@ func TestTLSALPN01Validate(t *testing.T) { err: NewErrorISE("failure saving error to acme challenge: force"), } }, - "ok/no-names-error": func(t *testing.T) test { + "ok/no-names-nor-ips-error": func(t *testing.T) test { ch := makeTLSCh() jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) @@ -1573,7 +1573,7 @@ func TestTLSALPN01Validate(t *testing.T) { assert.Equals(t, updch.Type, ch.Type) assert.Equals(t, updch.Value, ch.Value) - err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single DNS name, %v", ch.Value) + err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value) assert.HasPrefix(t, updch.Error.Err.Error(), err.Err.Error()) assert.Equals(t, updch.Error.Type, err.Type) @@ -1616,7 +1616,7 @@ func TestTLSALPN01Validate(t *testing.T) { assert.Equals(t, updch.Type, ch.Type) assert.Equals(t, updch.Value, ch.Value) - err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single DNS name, %v", ch.Value) + err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value) assert.HasPrefix(t, updch.Error.Err.Error(), err.Err.Error()) assert.Equals(t, updch.Error.Type, err.Type) @@ -1660,7 +1660,7 @@ func TestTLSALPN01Validate(t *testing.T) { assert.Equals(t, updch.Type, ch.Type) assert.Equals(t, updch.Value, ch.Value) - err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single DNS name, %v", ch.Value) + err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value) assert.HasPrefix(t, updch.Error.Err.Error(), err.Err.Error()) assert.Equals(t, updch.Error.Type, err.Type) @@ -1703,7 +1703,7 @@ func TestTLSALPN01Validate(t *testing.T) { assert.Equals(t, updch.Type, ch.Type) assert.Equals(t, updch.Value, ch.Value) - err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single DNS name, %v", ch.Value) + err := NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value) assert.HasPrefix(t, updch.Error.Err.Error(), err.Err.Error()) assert.Equals(t, updch.Error.Type, err.Type) @@ -2187,6 +2187,43 @@ func TestTLSALPN01Validate(t *testing.T) { srv, tlsDial := newTestTLSALPNServer(cert) srv.Start() + return test{ + ch: ch, + vo: &ValidateChallengeOptions{ + TLSDial: tlsDial, + }, + db: &MockDB{ + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equals(t, updch.ID, ch.ID) + assert.Equals(t, updch.Token, ch.Token) + assert.Equals(t, updch.Status, StatusValid) + assert.Equals(t, updch.Type, ch.Type) + assert.Equals(t, updch.Value, ch.Value) + assert.Equals(t, updch.Error, nil) + return nil + }, + }, + srv: srv, + jwk: jwk, + } + }, + "ok/ip": func(t *testing.T) test { + ch := makeTLSCh() + ch.Value = "127.0.0.1" + + jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + assert.FatalError(t, err) + + expKeyAuth, err := KeyAuthorization(ch.Token, jwk) + assert.FatalError(t, err) + expKeyAuthHash := sha256.Sum256([]byte(expKeyAuth)) + + cert, err := newTLSALPNValidationCert(expKeyAuthHash[:], false, true, ch.Value) + assert.FatalError(t, err) + + srv, tlsDial := newTestTLSALPNServer(cert) + srv.Start() + return test{ ch: ch, vo: &ValidateChallengeOptions{ @@ -2235,3 +2272,82 @@ func TestTLSALPN01Validate(t *testing.T) { }) } } + +func Test_reverseAddr(t *testing.T) { + type args struct { + ip net.IP + } + tests := []struct { + name string + args args + wantArpa string + }{ + { + name: "ok/ipv4", + args: args{ + ip: net.ParseIP("127.0.0.1"), + }, + wantArpa: "1.0.0.127.in-addr.arpa.", + }, + { + name: "ok/ipv6", + args: args{ + ip: net.ParseIP("2001:db8::567:89ab"), + }, + wantArpa: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if gotArpa := reverseAddr(tt.args.ip); gotArpa != tt.wantArpa { + t.Errorf("reverseAddr() = %v, want %v", gotArpa, tt.wantArpa) + } + }) + } +} + +func Test_serverName(t *testing.T) { + type args struct { + ch *Challenge + } + tests := []struct { + name string + args args + want string + }{ + { + name: "ok/dns", + args: args{ + ch: &Challenge{ + Value: "example.com", + }, + }, + want: "example.com", + }, + { + name: "ok/ipv4", + args: args{ + ch: &Challenge{ + Value: "127.0.0.1", + }, + }, + want: "1.0.0.127.in-addr.arpa.", + }, + { + name: "ok/ipv6", + args: args{ + ch: &Challenge{ + Value: "2001:db8::567:89ab", + }, + }, + want: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := serverName(tt.args.ch); got != tt.want { + t.Errorf("serverName() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/acme/db/nosql/challenge.go b/acme/db/nosql/challenge.go index f3a3cfca..f84a6f4e 100644 --- a/acme/db/nosql/challenge.go +++ b/acme/db/nosql/challenge.go @@ -11,15 +11,15 @@ import ( ) type dbChallenge struct { - ID string `json:"id"` - AccountID string `json:"accountID"` - Type string `json:"type"` - Status acme.Status `json:"status"` - Token string `json:"token"` - Value string `json:"value"` - ValidatedAt string `json:"validatedAt"` - CreatedAt time.Time `json:"createdAt"` - Error *acme.Error `json:"error"` + ID string `json:"id"` + AccountID string `json:"accountID"` + Type acme.ChallengeType `json:"type"` + Status acme.Status `json:"status"` + Token string `json:"token"` + Value string `json:"value"` + ValidatedAt string `json:"validatedAt"` + CreatedAt time.Time `json:"createdAt"` + Error *acme.Error `json:"error"` } func (dbc *dbChallenge) clone() *dbChallenge { diff --git a/acme/order.go b/acme/order.go index a003fe9a..fd8956f7 100644 --- a/acme/order.go +++ b/acme/order.go @@ -1,9 +1,11 @@ package acme import ( + "bytes" "context" "crypto/x509" "encoding/json" + "net" "sort" "strings" "time" @@ -12,10 +14,17 @@ import ( "go.step.sm/crypto/x509util" ) +type IdentifierType string + +const ( + IP IdentifierType = "ip" + DNS IdentifierType = "dns" +) + // Identifier encodes the type that an order pertains to. type Identifier struct { - Type string `json:"type"` - Value string `json:"value"` + Type IdentifierType `json:"type"` + Value string `json:"value"` } // Order contains order metadata for the ACME protocol order type. @@ -131,41 +140,13 @@ func (o *Order) Finalize(ctx context.Context, db DB, csr *x509.CertificateReques return NewErrorISE("unexpected status %s for order %s", o.Status, o.ID) } - // RFC8555: The CSR MUST indicate the exact same set of requested - // identifiers as the initial newOrder request. Identifiers of type "dns" - // MUST appear either in the commonName portion of the requested subject - // name or in an extensionRequest attribute [RFC2985] requesting a - // subjectAltName extension, or both. - if csr.Subject.CommonName != "" { - csr.DNSNames = append(csr.DNSNames, csr.Subject.CommonName) - } - csr.DNSNames = uniqueSortedLowerNames(csr.DNSNames) - orderNames := make([]string, len(o.Identifiers)) - for i, n := range o.Identifiers { - orderNames[i] = n.Value - } - orderNames = uniqueSortedLowerNames(orderNames) - - // Validate identifier names against CSR alternative names. - // - // Note that with certificate templates we are not going to check for the - // absence of other SANs as they will only be set if the templates allows - // them. - if len(csr.DNSNames) != len(orderNames) { - return NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ - "CSR names = %v, Order names = %v", csr.DNSNames, orderNames) - } + // canonicalize the CSR to allow for comparison + csr = canonicalize(csr) - sans := make([]x509util.SubjectAlternativeName, len(csr.DNSNames)) - for i := range csr.DNSNames { - if csr.DNSNames[i] != orderNames[i] { - return NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ - "CSR names = %v, Order names = %v", csr.DNSNames, orderNames) - } - sans[i] = x509util.SubjectAlternativeName{ - Type: x509util.DNSType, - Value: csr.DNSNames[i], - } + // retrieve the requested SANs for the Order + sans, err := o.sans(csr) + if err != nil { + return err } // Get authorizations from the ACME provisioner. @@ -213,6 +194,122 @@ func (o *Order) Finalize(ctx context.Context, db DB, csr *x509.CertificateReques return nil } +func (o *Order) sans(csr *x509.CertificateRequest) ([]x509util.SubjectAlternativeName, error) { + + var sans []x509util.SubjectAlternativeName + + // order the DNS names and IP addresses, so that they can be compared against the canonicalized CSR + orderNames := make([]string, numberOfIdentifierType(DNS, o.Identifiers)) + orderIPs := make([]net.IP, numberOfIdentifierType(IP, o.Identifiers)) + indexDNS, indexIP := 0, 0 + for _, n := range o.Identifiers { + switch n.Type { + case DNS: + orderNames[indexDNS] = n.Value + indexDNS++ + case IP: + orderIPs[indexIP] = net.ParseIP(n.Value) // NOTE: this assumes are all valid IPs at this time; or will result in nil entries + indexIP++ + default: + return sans, NewErrorISE("unsupported identifier type in order: %s", n.Type) + } + } + orderNames = uniqueSortedLowerNames(orderNames) + orderIPs = uniqueSortedIPs(orderIPs) + + totalNumberOfSANs := len(csr.DNSNames) + len(csr.IPAddresses) + sans = make([]x509util.SubjectAlternativeName, totalNumberOfSANs) + index := 0 + + // Validate identifier names against CSR alternative names. + // + // Note that with certificate templates we are not going to check for the + // absence of other SANs as they will only be set if the template allows + // them. + if len(csr.DNSNames) != len(orderNames) { + return sans, NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ + "CSR names = %v, Order names = %v", csr.DNSNames, orderNames) + } + + for i := range csr.DNSNames { + if csr.DNSNames[i] != orderNames[i] { + return sans, NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ + "CSR names = %v, Order names = %v", csr.DNSNames, orderNames) + } + sans[index] = x509util.SubjectAlternativeName{ + Type: x509util.DNSType, + Value: csr.DNSNames[i], + } + index++ + } + + if len(csr.IPAddresses) != len(orderIPs) { + return sans, NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", csr.IPAddresses, orderIPs) + } + + for i := range csr.IPAddresses { + if !ipsAreEqual(csr.IPAddresses[i], orderIPs[i]) { + return sans, NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", csr.IPAddresses, orderIPs) + } + sans[index] = x509util.SubjectAlternativeName{ + Type: x509util.IPType, + Value: csr.IPAddresses[i].String(), + } + index++ + } + + return sans, nil +} + +// numberOfIdentifierType returns the number of Identifiers that +// are of type typ. +func numberOfIdentifierType(typ IdentifierType, ids []Identifier) int { + c := 0 + for _, id := range ids { + if id.Type == typ { + c++ + } + } + return c +} + +// canonicalize canonicalizes a CSR so that it can be compared against an Order +// NOTE: this effectively changes the order of SANs in the CSR, which may be OK, +// but may not be expected. +func canonicalize(csr *x509.CertificateRequest) (canonicalized *x509.CertificateRequest) { + + // for clarity only; we're operating on the same object by pointer + canonicalized = csr + + // RFC8555: The CSR MUST indicate the exact same set of requested + // identifiers as the initial newOrder request. Identifiers of type "dns" + // MUST appear either in the commonName portion of the requested subject + // name or in an extensionRequest attribute [RFC2985] requesting a + // subjectAltName extension, or both. + if csr.Subject.CommonName != "" { + canonicalized.DNSNames = append(csr.DNSNames, csr.Subject.CommonName) + } + canonicalized.DNSNames = uniqueSortedLowerNames(csr.DNSNames) + canonicalized.IPAddresses = uniqueSortedIPs(csr.IPAddresses) + + return canonicalized +} + +// ipsAreEqual compares IPs to be equal. Nil values (i.e. invalid IPs) are +// not considered equal. IPv6 representations of IPv4 addresses are +// considered equal to the IPv4 address in this implementation, which is +// standard Go behavior. An example is "::ffff:192.168.42.42", which +// is equal to "192.168.42.42". This is considered a known issue within +// step and is tracked here too: https://github.com/golang/go/issues/37921. +func ipsAreEqual(x, y net.IP) bool { + if x == nil || y == nil { + return false + } + return x.Equal(y) +} + // uniqueSortedLowerNames returns the set of all unique names in the input after all // of them are lowercased. The returned names will be in their lowercased form // and sorted alphabetically. @@ -228,3 +325,23 @@ func uniqueSortedLowerNames(names []string) (unique []string) { sort.Strings(unique) return } + +// uniqueSortedIPs returns the set of all unique net.IPs in the input. They +// are sorted by their bytes (octet) representation. +func uniqueSortedIPs(ips []net.IP) (unique []net.IP) { + type entry struct { + ip net.IP + } + ipEntryMap := make(map[string]entry, len(ips)) + for _, ip := range ips { + ipEntryMap[ip.String()] = entry{ip: ip} + } + unique = make([]net.IP, 0, len(ipEntryMap)) + for _, entry := range ipEntryMap { + unique = append(unique, entry.ip) + } + sort.Slice(unique, func(i, j int) bool { + return bytes.Compare(unique[i], unique[j]) < 0 + }) + return +} diff --git a/acme/order_test.go b/acme/order_test.go index 849f0daa..83488c8c 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -5,12 +5,15 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/json" + "net" + "reflect" "testing" "time" "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) { @@ -364,7 +367,7 @@ func TestOrder_Finalize(t *testing.T) { err: NewErrorISE("unrecognized order status: %s", o.Status), } }, - "fail/error-names-length-mismatch": func(t *testing.T) test { + "fail/error-provisioner-auth": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -377,21 +380,26 @@ func TestOrder_Finalize(t *testing.T) { {Type: "dns", Value: "bar.internal"}, }, } - orderNames := []string{"bar.internal", "foo.internal"} csr := &x509.CertificateRequest{ Subject: pkix.Name{ CommonName: "foo.internal", }, + DNSNames: []string{"bar.internal"}, } return test{ o: o, csr: csr, - err: NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ - "CSR names = %v, Order names = %v", []string{"foo.internal"}, orderNames), + prov: &MockProvisioner{ + MauthorizeSign: func(ctx context.Context, token string) ([]provisioner.SignOption, error) { + assert.Equals(t, token, "") + return nil, errors.New("force") + }, + }, + err: NewErrorISE("error retrieving authorization options from ACME provisioner: force"), } }, - "fail/error-names-mismatch": func(t *testing.T) test { + "fail/error-template-options": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -404,22 +412,33 @@ func TestOrder_Finalize(t *testing.T) { {Type: "dns", Value: "bar.internal"}, }, } - orderNames := []string{"bar.internal", "foo.internal"} csr := &x509.CertificateRequest{ Subject: pkix.Name{ CommonName: "foo.internal", }, - DNSNames: []string{"zap.internal"}, + DNSNames: []string{"bar.internal"}, } return test{ o: o, csr: csr, - err: NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ - "CSR names = %v, Order names = %v", []string{"foo.internal", "zap.internal"}, orderNames), + prov: &MockProvisioner{ + MauthorizeSign: func(ctx context.Context, token string) ([]provisioner.SignOption, error) { + assert.Equals(t, token, "") + return nil, nil + }, + MgetOptions: func() *provisioner.Options { + return &provisioner.Options{ + X509: &provisioner.X509Options{ + TemplateData: json.RawMessage([]byte("fo{o")), + }, + } + }, + }, + err: NewErrorISE("error creating template options from ACME provisioner: error unmarshaling template data: invalid character 'o' in literal false (expecting 'a')"), } }, - "fail/error-provisioner-auth": func(t *testing.T) test { + "fail/error-ca-sign": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -445,13 +464,22 @@ func TestOrder_Finalize(t *testing.T) { prov: &MockProvisioner{ MauthorizeSign: func(ctx context.Context, token string) ([]provisioner.SignOption, error) { assert.Equals(t, token, "") + return nil, nil + }, + MgetOptions: func() *provisioner.Options { + return nil + }, + }, + ca: &mockSignAuth{ + sign: func(_csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, _csr, csr) return nil, errors.New("force") }, }, - err: NewErrorISE("error retrieving authorization options from ACME provisioner: force"), + err: NewErrorISE("error signing certificate for order oID: force"), } }, - "fail/error-template-options": func(t *testing.T) test { + "fail/error-db.CreateCertificate": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -471,6 +499,10 @@ func TestOrder_Finalize(t *testing.T) { DNSNames: []string{"bar.internal"}, } + foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} + bar := &x509.Certificate{Subject: pkix.Name{CommonName: "bar"}} + baz := &x509.Certificate{Subject: pkix.Name{CommonName: "baz"}} + return test{ o: o, csr: csr, @@ -480,17 +512,28 @@ func TestOrder_Finalize(t *testing.T) { return nil, nil }, MgetOptions: func() *provisioner.Options { - return &provisioner.Options{ - X509: &provisioner.X509Options{ - TemplateData: json.RawMessage([]byte("fo{o")), - }, - } + return nil }, }, - err: NewErrorISE("error creating template options from ACME provisioner: error unmarshaling template data: invalid character 'o' in literal false (expecting 'a')"), + ca: &mockSignAuth{ + sign: func(_csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, _csr, csr) + return []*x509.Certificate{foo, bar, baz}, nil + }, + }, + db: &MockDB{ + MockCreateCertificate: func(ctx context.Context, cert *Certificate) error { + assert.Equals(t, cert.AccountID, o.AccountID) + assert.Equals(t, cert.OrderID, o.ID) + assert.Equals(t, cert.Leaf, foo) + assert.Equals(t, cert.Intermediates, []*x509.Certificate{bar, baz}) + return errors.New("force") + }, + }, + err: NewErrorISE("error creating certificate for order oID: force"), } }, - "fail/error-ca-sign": func(t *testing.T) test { + "fail/error-db.UpdateOrder": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -510,6 +553,10 @@ func TestOrder_Finalize(t *testing.T) { DNSNames: []string{"bar.internal"}, } + foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} + bar := &x509.Certificate{Subject: pkix.Name{CommonName: "bar"}} + baz := &x509.Certificate{Subject: pkix.Name{CommonName: "baz"}} + return test{ o: o, csr: csr, @@ -525,13 +572,33 @@ func TestOrder_Finalize(t *testing.T) { ca: &mockSignAuth{ sign: func(_csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) { assert.Equals(t, _csr, csr) - return nil, errors.New("force") + return []*x509.Certificate{foo, bar, baz}, nil }, }, - err: NewErrorISE("error signing certificate for order oID: force"), + db: &MockDB{ + MockCreateCertificate: func(ctx context.Context, cert *Certificate) error { + cert.ID = "certID" + assert.Equals(t, cert.AccountID, o.AccountID) + assert.Equals(t, cert.OrderID, o.ID) + assert.Equals(t, cert.Leaf, foo) + assert.Equals(t, cert.Intermediates, []*x509.Certificate{bar, baz}) + return nil + }, + MockUpdateOrder: func(ctx context.Context, updo *Order) error { + assert.Equals(t, updo.CertificateID, "certID") + assert.Equals(t, updo.Status, StatusValid) + assert.Equals(t, updo.ID, o.ID) + assert.Equals(t, updo.AccountID, o.AccountID) + assert.Equals(t, updo.ExpiresAt, o.ExpiresAt) + assert.Equals(t, updo.AuthorizationIDs, o.AuthorizationIDs) + assert.Equals(t, updo.Identifiers, o.Identifiers) + return errors.New("force") + }, + }, + err: NewErrorISE("error updating order oID: force"), } }, - "fail/error-db.CreateCertificate": func(t *testing.T) test { + "ok/new-cert-dns": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -575,17 +642,27 @@ func TestOrder_Finalize(t *testing.T) { }, db: &MockDB{ MockCreateCertificate: func(ctx context.Context, cert *Certificate) error { + cert.ID = "certID" assert.Equals(t, cert.AccountID, o.AccountID) assert.Equals(t, cert.OrderID, o.ID) assert.Equals(t, cert.Leaf, foo) assert.Equals(t, cert.Intermediates, []*x509.Certificate{bar, baz}) - return errors.New("force") + return nil + }, + MockUpdateOrder: func(ctx context.Context, updo *Order) error { + assert.Equals(t, updo.CertificateID, "certID") + assert.Equals(t, updo.Status, StatusValid) + assert.Equals(t, updo.ID, o.ID) + assert.Equals(t, updo.AccountID, o.AccountID) + assert.Equals(t, updo.ExpiresAt, o.ExpiresAt) + assert.Equals(t, updo.AuthorizationIDs, o.AuthorizationIDs) + assert.Equals(t, updo.Identifiers, o.Identifiers) + return nil }, }, - err: NewErrorISE("error creating certificate for order oID: force"), } }, - "fail/error-db.UpdateOrder": func(t *testing.T) test { + "ok/new-cert-ip": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -594,15 +671,12 @@ func TestOrder_Finalize(t *testing.T) { ExpiresAt: now.Add(5 * time.Minute), AuthorizationIDs: []string{"a", "b"}, Identifiers: []Identifier{ - {Type: "dns", Value: "foo.internal"}, - {Type: "dns", Value: "bar.internal"}, + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, }, } csr := &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "foo.internal", - }, - DNSNames: []string{"bar.internal"}, + IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")}, // in case of IPs, no Common Name } foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} @@ -644,13 +718,12 @@ func TestOrder_Finalize(t *testing.T) { assert.Equals(t, updo.ExpiresAt, o.ExpiresAt) assert.Equals(t, updo.AuthorizationIDs, o.AuthorizationIDs) assert.Equals(t, updo.Identifiers, o.Identifiers) - return errors.New("force") + return nil }, }, - err: NewErrorISE("error updating order oID: force"), } }, - "ok/new-cert": func(t *testing.T) test { + "ok/new-cert-dns-and-ip": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -660,14 +733,14 @@ func TestOrder_Finalize(t *testing.T) { AuthorizationIDs: []string{"a", "b"}, Identifiers: []Identifier{ {Type: "dns", Value: "foo.internal"}, - {Type: "dns", Value: "bar.internal"}, + {Type: "ip", Value: "192.168.42.42"}, }, } csr := &x509.CertificateRequest{ Subject: pkix.Name{ CommonName: "foo.internal", }, - DNSNames: []string{"bar.internal"}, + IPAddresses: []net.IP{net.ParseIP("192.168.42.42")}, } foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} @@ -737,3 +810,592 @@ func TestOrder_Finalize(t *testing.T) { }) } } + +func Test_uniqueSortedIPs(t *testing.T) { + type args struct { + ips []net.IP + } + tests := []struct { + name string + args args + wantUnique []net.IP + }{ + { + name: "ok/empty", + args: args{ + ips: []net.IP{}, + }, + wantUnique: []net.IP{}, + }, + { + name: "ok/single-ipv4", + args: args{ + ips: []net.IP{net.ParseIP("192.168.42.42")}, + }, + wantUnique: []net.IP{net.ParseIP("192.168.42.42")}, + }, + { + name: "ok/multiple-ipv4", + args: args{ + ips: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.42.10"), net.ParseIP("192.168.42.1")}, + }, + wantUnique: []net.IP{net.ParseIP("192.168.42.1"), net.ParseIP("192.168.42.10"), net.ParseIP("192.168.42.42")}, + }, + { + name: "ok/unique-ipv4", + args: args{ + ips: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.42.42")}, + }, + wantUnique: []net.IP{net.ParseIP("192.168.42.42")}, + }, + { + name: "ok/single-ipv6", + args: args{ + ips: []net.IP{net.ParseIP("2001:db8::30")}, + }, + wantUnique: []net.IP{net.ParseIP("2001:db8::30")}, + }, + { + name: "ok/multiple-ipv6", + args: args{ + ips: []net.IP{net.ParseIP("2001:db8::30"), net.ParseIP("2001:db8::20"), net.ParseIP("2001:db8::10")}, + }, + wantUnique: []net.IP{net.ParseIP("2001:db8::10"), net.ParseIP("2001:db8::20"), net.ParseIP("2001:db8::30")}, + }, + { + name: "ok/unique-ipv6", + args: args{ + ips: []net.IP{net.ParseIP("2001:db8::1"), net.ParseIP("2001:db8::1")}, + }, + wantUnique: []net.IP{net.ParseIP("2001:db8::1")}, + }, + { + name: "ok/mixed-ipv4-and-ipv6", + args: args{ + ips: []net.IP{net.ParseIP("2001:db8::1"), net.ParseIP("2001:db8::1"), net.ParseIP("192.168.42.42"), net.ParseIP("192.168.42.42")}, + }, + wantUnique: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("2001:db8::1")}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if gotUnique := uniqueSortedIPs(tt.args.ips); !reflect.DeepEqual(gotUnique, tt.wantUnique) { + t.Errorf("uniqueSortedIPs() = %v, want %v", gotUnique, tt.wantUnique) + } + }) + } +} + +func Test_numberOfIdentifierType(t *testing.T) { + type args struct { + typ IdentifierType + ids []Identifier + } + tests := []struct { + name string + args args + want int + }{ + { + name: "ok/no-identifiers", + args: args{ + typ: DNS, + ids: []Identifier{}, + }, + want: 0, + }, + { + name: "ok/no-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 0, + }, + { + name: "ok/no-ips", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + }, + }, + want: 0, + }, + { + name: "ok/one-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 1, + }, + { + name: "ok/one-ip", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 1, + }, + { + name: "ok/more-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: DNS, + Value: "*.example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 2, + }, + { + name: "ok/more-ips", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + { + Type: IP, + Value: "192.168.42.43", + }, + }, + }, + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := numberOfIdentifierType(tt.args.typ, tt.args.ids); got != tt.want { + t.Errorf("numberOfIdentifierType() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_ipsAreEqual(t *testing.T) { + type args struct { + x net.IP + y net.IP + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "ok/ipv4", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("192.168.42.42"), + }, + want: true, + }, + { + name: "fail/ipv4", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("192.168.42.43"), + }, + want: false, + }, + { + name: "ok/ipv6", + args: args{ + x: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + }, + want: true, + }, + { + name: "fail/ipv6", + args: args{ + x: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7335"), + }, + want: false, + }, + { + name: "fail/ipv4-and-ipv6", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + }, + want: false, + }, + { + name: "ok/ipv4-mapped-to-ipv6", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("::ffff:192.168.42.42"), // parsed to the same IPv4 by Go + }, + want: true, // we expect this to happen; a known issue in which ipv4 mapped ipv6 addresses are considered the same as their ipv4 counterpart + }, + { + name: "fail/invalid-ipv4-and-valid-ipv6", + args: args{ + x: net.ParseIP("192.168.42.1000"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + }, + want: false, + }, + { + name: "fail/valid-ipv4-and-invalid-ipv6", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:733400"), + }, + want: false, + }, + { + name: "fail/invalid-ipv4-and-invalid-ipv6", + args: args{ + x: net.ParseIP("192.168.42.1000"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:1000000"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ipsAreEqual(tt.args.x, tt.args.y); got != tt.want { + t.Errorf("ipsAreEqual() = %v, want %v", got, tt.want) + } + }) + } +} + +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 { + Identifiers []Identifier + } + tests := []struct { + name string + fields fields + csr *x509.CertificateRequest + want []x509util.SubjectAlternativeName + err *Error + }{ + { + name: "ok/dns", + fields: fields{ + Identifiers: []Identifier{ + {Type: "dns", Value: "example.com"}, + }, + }, + csr: &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "example.com", + }, + }, + want: []x509util.SubjectAlternativeName{ + {Type: "dns", Value: "example.com"}, + }, + err: nil, + }, + { + name: "fail/error-names-length-mismatch", + fields: fields{ + Identifiers: []Identifier{ + {Type: "dns", Value: "foo.internal"}, + {Type: "dns", Value: "bar.internal"}, + }, + }, + csr: &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "foo.internal", + }, + }, + want: []x509util.SubjectAlternativeName{}, + err: NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ + "CSR names = %v, Order names = %v", []string{"foo.internal"}, []string{"bar.internal", "foo.internal"}), + }, + { + name: "fail/error-names-mismatch", + fields: fields{ + Identifiers: []Identifier{ + {Type: "dns", Value: "foo.internal"}, + {Type: "dns", Value: "bar.internal"}, + }, + }, + csr: &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "foo.internal", + }, + DNSNames: []string{"zap.internal"}, + }, + want: []x509util.SubjectAlternativeName{}, + err: NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ + "CSR names = %v, Order names = %v", []string{"foo.internal", "zap.internal"}, []string{"bar.internal", "foo.internal"}), + }, + { + name: "ok/ipv4", + fields: fields{ + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.43.42"}, + {Type: "ip", Value: "192.168.42.42"}, + }, + }, + csr: &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.43.42"), net.ParseIP("192.168.42.42")}, + }, + want: []x509util.SubjectAlternativeName{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + err: nil, + }, + { + name: "ok/ipv6", + fields: fields{ + Identifiers: []Identifier{ + {Type: "ip", Value: "2001:0db8:85a3::8a2e:0370:7335"}, + {Type: "ip", Value: "2001:0db8:85a3::8a2e:0370:7334"}, + }, + }, + csr: &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7335"), net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334")}, + }, + want: []x509util.SubjectAlternativeName{ + {Type: "ip", Value: "2001:db8:85a3::8a2e:370:7334"}, + {Type: "ip", Value: "2001:db8:85a3::8a2e:370:7335"}, + }, + err: nil, + }, + { + name: "fail/error-ips-length-mismatch", + fields: fields{ + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + }, + csr: &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42")}, + }, + want: []x509util.SubjectAlternativeName{}, + err: NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", []net.IP{net.ParseIP("192.168.42.42")}, []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")}), + }, + { + name: "fail/error-ips-mismatch", + fields: fields{ + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + }, + csr: &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.42.32")}, + }, + want: []x509util.SubjectAlternativeName{}, + err: NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", []net.IP{net.ParseIP("192.168.42.32"), net.ParseIP("192.168.42.42")}, []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")}), + }, + { + name: "ok/mixed", + fields: fields{ + Identifiers: []Identifier{ + {Type: "dns", Value: "foo.internal"}, + {Type: "dns", Value: "bar.internal"}, + {Type: "ip", Value: "192.168.43.42"}, + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "2001:0db8:85a3:0000:0000:8a2e:0370:7334"}, + }, + }, + csr: &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "bar.internal", + }, + DNSNames: []string{"foo.internal"}, + IPAddresses: []net.IP{net.ParseIP("192.168.43.42"), net.ParseIP("192.168.42.42"), net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334")}, + }, + want: []x509util.SubjectAlternativeName{ + {Type: "dns", Value: "bar.internal"}, + {Type: "dns", Value: "foo.internal"}, + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + {Type: "ip", Value: "2001:db8:85a3::8a2e:370:7334"}, + }, + err: nil, + }, + { + name: "fail/unsupported-identifier-type", + fields: fields{ + Identifiers: []Identifier{ + {Type: "ipv4", Value: "192.168.42.42"}, + }, + }, + csr: &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42")}, + }, + want: []x509util.SubjectAlternativeName{}, + err: NewError(ErrorServerInternalType, "unsupported identifier type in order: ipv4"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &Order{ + Identifiers: tt.fields.Identifiers, + } + canonicalizedCSR := canonicalize(tt.csr) + got, err := o.sans(canonicalizedCSR) + if tt.err != nil { + if err == nil { + t.Errorf("Order.sans() = %v, want error; got none", got) + return + } + switch k := err.(type) { + case *Error: + assert.Equals(t, k.Type, tt.err.Type) + assert.Equals(t, k.Detail, tt.err.Detail) + assert.Equals(t, k.Status, tt.err.Status) + assert.Equals(t, k.Err.Error(), tt.err.Err.Error()) + assert.Equals(t, k.Detail, tt.err.Detail) + default: + assert.FatalError(t, errors.New("unexpected error type")) + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Order.sans() = %v, want %v", got, tt.want) + } + }) + } +}