diff --git a/acme/api/order.go b/acme/api/order.go index 9d410173..b0b9cb43 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -28,7 +28,7 @@ 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 == "dns" || id.Type == "ip") { return acme.NewError(acme.ErrorMalformedType, "identifier type unsupported: %s", id.Type) } } @@ -149,15 +149,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 +269,22 @@ 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) []string { + chTypes := []string{} + + // DNS challenge can not be used for identifiers with type IP + if az.Identifier.Type != "ip" { + chTypes = append(chTypes, "dns-01") // TODO: make these types consts/enum? + } + + // 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 = append(chTypes, []string{"http-01"}...) // TODO: fix tls-alpn-01 + } + + return chTypes +} diff --git a/acme/api/order_test.go b/acme/api/order_test.go index 300aa61b..6782de75 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" @@ -60,6 +61,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 { // TODO: verify that this is allowed and what we want to be possible (in Validate()) + 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) @@ -1581,3 +1644,53 @@ 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 []string + }{ + { + name: "ok/dns", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "dns", Value: "example.com"}, + Wildcard: false, + }, + }, + want: []string{"dns-01", "http-01", "tls-alpn-01"}, + }, + { + name: "ok/wildcard", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "dns", Value: "*.example.com"}, + Wildcard: true, + }, + }, + want: []string{"dns-01"}, + }, + { + name: "ok/ip", + args: args{ + az: &acme.Authorization{ + Identifier: acme.Identifier{Type: "ip", Value: "192.168.42.42"}, + Wildcard: false, + }, + }, + want: []string{"http-01", "tls-alpn-01"}, + }, + } + 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/order.go b/acme/order.go index a003fe9a..fdab182f 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" @@ -131,41 +133,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) + // canonicalize the CSR to allow for comparison + csr = canonicalize(csr) - // 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) - } - - 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 WrapErrorISE(err, "error determining SANs for the CSR") } // Get authorizations from the ACME provisioner. @@ -213,6 +187,120 @@ 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, len(o.Identifiers)) + orderIPs := make([]net.IP, len(o.Identifiers)) + for i, n := range o.Identifiers { + switch n.Type { + case "dns": + orderNames[i] = n.Value + case "ip": + orderIPs[i] = net.ParseIP(n.Value) // NOTE: this assumes are all valid IPs or will result in nil entries + default: + return sans, NewErrorISE("unsupported identifier type in order: %s", n.Type) + } + } + orderNames = uniqueSortedLowerNames(orderNames) + orderIPs = uniqueSortedIPs(orderIPs) + + // TODO: check whether this order was requested with identifier-type IP, + // if so, handle it as an IP order; not as a DNSName order, so the logic + // for verifying the contents MAY not be necessary. + // TODO: limit what IP addresses can be used? Only private? Only certain ranges + // based on configuration? Public vs. private range? That logic should be configurable somewhere. + // TODO: how to handler orders that have DNSNames AND IPs? I guess it could + // happen in cases where there are multiple "identifiers" to order a cert for + // and http or tls-alpn-1 is used (NOT DNS, because that can't be used for IPs). + // TODO: ensure that DNSNames indeed MUST NEVER have an IP + // TODO: only allow IP based identifier based on configuration? + // TODO: validation of the input (if IP; should be valid IPv4/v6) + + // Determine if DNS names or IPs should be processed. + // At this time, orders in which DNS names and IPs are mixed are not supported. // TODO: ensure that's OK and/or should we support more, RFC-wise + shouldProcessIPAddresses := len(csr.DNSNames) == 0 && len(orderIPs) != 0 // TODO: verify that this logic is OK and sufficient + if shouldProcessIPAddresses { + // Validate identifier IPs against CSR alternative names (IPs). + 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) + } + + sans = make([]x509util.SubjectAlternativeName, len(csr.IPAddresses)) + 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[i] = x509util.SubjectAlternativeName{ + Type: x509util.IPType, + Value: csr.IPAddresses[i].String(), + } + } + } else { + // 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 sans, NewError(ErrorBadCSRType, "CSR names do not match identifiers exactly: "+ + "CSR names = %v, Order names = %v", csr.DNSNames, orderNames) + } + + sans = make([]x509util.SubjectAlternativeName, len(csr.DNSNames)) + 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[i] = x509util.SubjectAlternativeName{ + Type: x509util.DNSType, + Value: csr.DNSNames[i], + } + } + } + + return sans, nil +} + +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) // TODO: sorting and setting this value MAY result in different values in CSR (and probably also ending up in cert); is that behavior wanted? + + return canonicalized +} + +// ipsAreEqual compares IPs to be equal. IPv6 representations of IPv4 +// adresses are NOT considered equal to the IPv4 address in this case. +// Both IPs should be the same version AND equal to each other. +func ipsAreEqual(x, y net.IP) bool { + if isIPv4(x) && isIPv4(y) { + return x.Equal(y) + } + return x.Equal(y) +} + +// isIPv4 returns if an IP is IPv4 or not. +func isIPv4(ip net.IP) bool { + return ip.To4() != nil +} + // 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 +316,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 993a92f2..c0461dc6 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -5,6 +5,8 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/json" + "net" + "reflect" "testing" "time" @@ -737,3 +739,78 @@ 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) + } + }) + } +}