Ignore IP CNs in CSRs (#8231)

If a finalize CSR contains a SAN which looks like an IP address, don't
actually include that CN in our IssuanceRequest, and don't promote any
other SAN to be the CN either. This is similar to how we ignore the
CSR's CN when it is too long.

Author: aarongable

csr/csr.go

@@ -5,6 +5,7 @@ import (
 	"crypto"
 	"crypto/x509"
 	"errors"
+	"net/netip"
 	"strings"
 
 	"github.com/letsencrypt/boulder/core"
@@ -86,20 +87,27 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
 }
 
 // CNFromCSR returns the lower-cased Subject Common Name from the CSR, if a
-// short enough CN was provided. If it was too long, there will be no CN. If
-// none was provided, the CN will be the first SAN that is short enough, which
-// is done only for backwards compatibility with prior Let's Encrypt behaviour.
+// short enough CN was provided. If it was too long or appears to be an IP,
+// there will be no CN. If none was provided, the CN will be the first SAN that
+// is short enough, which is done only for backwards compatibility with prior
+// Let's Encrypt behaviour.
 func CNFromCSR(csr *x509.CertificateRequest) string {
 	if len(csr.Subject.CommonName) > maxCNLength {
 		return ""
 	}
 
 	if csr.Subject.CommonName != "" {
+		_, err := netip.ParseAddr(csr.Subject.CommonName)
+		if err == nil { // inverted; we're looking for successful parsing here
+			return ""
+		}
+
 		return strings.ToLower(csr.Subject.CommonName)
 	}
 
-	// If there's no CN already, but we want to set one, promote the first SAN
-	// which is shorter than the maximum acceptable CN length (if any).
+	// If there's no CN already, but we want to set one, promote the first dnsName
+	// SAN which is shorter than the maximum acceptable CN length (if any). We
+	// will never promote an ipAddress SAN to the CN.
 	for _, name := range csr.DNSNames {
 		if len(name) <= maxCNLength {
 			return strings.ToLower(name)

csr/csr_test.go

@@ -9,6 +9,7 @@ import (
 	"encoding/asn1"
 	"errors"
 	"net"
+	"net/netip"
 	"net/url"
 	"strings"
 	"testing"
@@ -239,6 +240,22 @@ func TestCNFromCSR(t *testing.T) {
 				}},
 			"",
 		},
+		{
+			"explicit CN that's an IP",
+			&x509.CertificateRequest{
+				Subject: pkix.Name{CommonName: "127.0.0.1"},
+			},
+			"",
+		},
+		{
+			"no CN, only IP SANs",
+			&x509.CertificateRequest{
+				IPAddresses: []net.IP{
+					netip.MustParseAddr("127.0.0.1").AsSlice(),
+				},
+			},
+			"",
+		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {