Add initial support for ACME IP validation

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 (
+	chTypes := challengeTypes(az)
-		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"}...)
-	}
 
+	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
+}

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)
+			}
+		})
+	}
+}

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
+	// canonicalize the CSR to allow for comparison
-	// identifiers as the initial newOrder request. Identifiers of type "dns"
+	csr = canonicalize(csr)
-	// 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.
+	// retrieve the requested SANs for the Order
-	//
+	sans, err := o.sans(csr)
-	// Note that with certificate templates we are not going to check for the
+	if err != nil {
-	// absence of other SANs as they will only be set if the templates allows
+		return WrapErrorISE(err, "error determining SANs for the CSR")
-	// 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],
-		}
 	}
 
 	// 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
+}

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)
+			}
+		})
+	}
+}