From 3e36522329aeef0009547c50d30e0f54e0081d89 Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Sat, 29 May 2021 00:19:14 +0200
Subject: [PATCH] Add preliminary support for TLS-ALPN-01 challenge for IP
 identifiers

---
 acme/api/order.go |  7 +++----
 acme/challenge.go | 24 +++++++++++++++++++++---
 acme/order.go     |  9 ++-------
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/acme/api/order.go b/acme/api/order.go
index b0b9cb43..78a4f9c6 100644
--- a/acme/api/order.go
+++ b/acme/api/order.go
@@ -275,15 +275,14 @@ func (h *Handler) FinalizeOrder(w http.ResponseWriter, r *http.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" {
+	// DNS challenge can only be used for identifiers with type dns
+	if az.Identifier.Type == "dns" {
 		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
+		chTypes = append(chTypes, []string{"http-01", "tls-alpn-01"}...)
 	}
 
 	return chTypes
diff --git a/acme/challenge.go b/acme/challenge.go
index 1059e437..90960fc4 100644
--- a/acme/challenge.go
+++ b/acme/challenge.go
@@ -119,8 +119,14 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 
 	hostPort := net.JoinHostPort(ch.Value, "443")
 
+	fmt.Println(hostPort)
+	fmt.Println(fmt.Sprintf("%#+v", config))
+
+	time.Sleep(5 * time.Second) // TODO: remove this; client seems to take a while to start serving; the server does not seem to retry the conn
+
 	conn, err := vo.TLSDial("tcp", hostPort, config)
 	if err != nil {
+		fmt.Println(err)
 		return storeError(ctx, db, ch, false, WrapError(ErrorConnectionType, err,
 			"error doing TLS dial for %s", hostPort))
 	}
@@ -129,6 +135,9 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 	cs := conn.ConnectionState()
 	certs := cs.PeerCertificates
 
+	fmt.Println(fmt.Sprintf("%#+v", cs))
+	fmt.Println(fmt.Sprintf("%#+v", certs))
+
 	if len(certs) == 0 {
 		return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType,
 			"%s challenge for %s resulted in no certificates", ch.Type, ch.Value))
@@ -140,10 +149,19 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 	}
 
 	leafCert := certs[0]
+	fmt.Println(fmt.Sprintf("%#+v", leafCert))
 
-	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 || !strings.EqualFold(leafCert.IPAddresses[0].String(), 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, %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 DNS name, %v", ch.Value))
+		}
 	}
 
 	idPeAcmeIdentifier := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 31}
diff --git a/acme/order.go b/acme/order.go
index fdab182f..e9e161f9 100644
--- a/acme/order.go
+++ b/acme/order.go
@@ -207,17 +207,12 @@ func (o *Order) sans(csr *x509.CertificateRequest) ([]x509util.SubjectAlternativ
 	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)
+	// TODO: validation of the input (if IP; should be valid IPv4/v6; Incoming request should have Host header set / ALPN IN-ADDR.ARPA)
+	// TODO: limit the IP address identifier to a single IP address? RFC _can_ be read like that, but there can be multiple identifiers, of course
 
 	// 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