diff --git a/acme/order.go b/acme/order.go index 2a869e78..9c823f78 100644 --- a/acme/order.go +++ b/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 diff --git a/acme/order_test.go b/acme/order_test.go index c0682fee..b9609cf9 100644 --- a/acme/order_test.go +++ b/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"),