Fix IP equality check and add more tests

acme/order.go

@@ -299,20 +299,17 @@ func canonicalize(csr *x509.CertificateRequest) (canonicalized *x509.Certificate
 	return canonicalized
 }
 
-// ipsAreEqual compares IPs to be equal. IPv6 representations of IPv4
-// adresses are considered equal to the IPv4 address in this case.
-// TODO: is this behavior OK to keep?
+// 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 matchAddrFamily(x, y) {
-		return x.Equal(y)
+	if x == nil || y == nil {
+		return false
 	}
-	return false
-}
-
-// matchAddrFamily returns true if two IPs are both IPv4 OR IPv6
-// Implementation taken and adapted from https://golang.org/src/net/ip.go
-func matchAddrFamily(x net.IP, y net.IP) bool {
-	return x.To4() != nil && y.To4() != nil || x.To16() != nil && x.To4() == nil && y.To16() != nil && y.To4() == nil
+	return x.Equal(y)
 }
 
 // uniqueSortedLowerNames returns the set of all unique names in the input after all

acme/order_test.go

@@ -1139,6 +1139,14 @@ func Test_ipsAreEqual(t *testing.T) {
 			},
 			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{
@@ -1148,7 +1156,15 @@ func Test_ipsAreEqual(t *testing.T) {
 			want: true,
 		},
 		{
-			name: "ok/ipv4-and-ipv6",
+			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"),
@@ -1161,10 +1177,10 @@ func Test_ipsAreEqual(t *testing.T) {
 				x: net.ParseIP("192.168.42.42"),
 				y: net.ParseIP("::ffff:192.168.42.42"), // parsed to the same IPv4 by Go
 			},
-			want: true, // TODO: is this behavior OK?
+			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: "ok/invalid-ipv4-and-valid-ipv6",
+			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"),
@@ -1172,7 +1188,15 @@ func Test_ipsAreEqual(t *testing.T) {
 			want: false,
 		},
 		{
-			name: "ok/invalid-ipv4-and-invalid-ipv6",
+			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"),