weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix invalid handling of ip addresses in comparisons

Browse Source 
Signed-off-by: Ashley Davis <ashley.davis@venafi.com>
This commit is contained in:
Ashley Davis 2023-08-24 13:58:14 +01:00
parent e81cbfdca6
commit bbbc758ccd
No known key found for this signature in database
6 changed files with 149 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cmd/cainjector/go.mod
View File

@ -57,6 +57,7 @@ require (
go.uber.org/atomic v1.9.0 // indirect
go.uber.org/multierr v1.6.0 // indirect
go.uber.org/zap v1.24.0 // indirect
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect
golang.org/x/net v0.10.0 // indirect
golang.org/x/oauth2 v0.5.0 // indirect
golang.org/x/sys v0.8.0 // indirect

2
cmd/cainjector/go.sum
View File

@ -176,6 +176,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=

2
pkg/util/pki/csr.go
View File

@ -85,6 +85,8 @@ func URLsFromStrings(urlStrs []string) ([]*url.URL, error) {
return urls, nil
}
// IPAddressesToString converts a slice of IP addresses to strings, which can be useful for
// printing a list of addresses but MUST NOT be used for comparing two slices of IP addresses.
func IPAddressesToString(ipAddresses []net.IP) []string {
var ipNames []string
for _, ip := range ipAddresses {

20
pkg/util/pki/match.go
View File

@ -21,6 +21,7 @@ import (
"crypto/ecdsa"
"crypto/ed25519"
"crypto/rsa"
"net"
"fmt"
"reflect"
@ -103,6 +104,16 @@ func ed25519PrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSp
return nil, nil
}
func ipSlicesMatch(parsedIPs []net.IP, stringIPs []string) bool {
parsedStringIPs := make([]net.IP, len(stringIPs))
for i, s := range stringIPs {
parsedStringIPs[i] = net.ParseIP(s)
}
return util.EqualIPsUnsorted(parsedStringIPs, parsedIPs)
}
// RequestMatchesSpec compares a CertificateRequest with a CertificateSpec
// and returns a list of field names on the Certificate that do not match their
// counterpart fields on the CertificateRequest.
@ -121,15 +132,18 @@ func RequestMatchesSpec(req *cmapi.CertificateRequest, spec cmapi.CertificateSpe
var violations []string
if !util.EqualUnsorted(IPAddressesToString(x509req.IPAddresses), spec.IPAddresses) {
if !ipSlicesMatch(x509req.IPAddresses, spec.IPAddresses) {
violations = append(violations, "spec.ipAddresses")
}
if !util.EqualUnsorted(URLsToString(x509req.URIs), spec.URIs) {
violations = append(violations, "spec.uris")
}
if !util.EqualUnsorted(x509req.EmailAddresses, spec.EmailAddresses) {
violations = append(violations, "spec.emailAddresses")
}
if !util.EqualUnsorted(x509req.DNSNames, spec.DNSNames) {
violations = append(violations, "spec.dnsNames")
}
@ -239,12 +253,14 @@ func SecretDataAltNamesMatchSpec(secret *corev1.Secret, spec cmapi.CertificateSp
}
}
if !util.EqualUnsorted(IPAddressesToString(x509cert.IPAddresses), spec.IPAddresses) {
if !ipSlicesMatch(x509cert.IPAddresses, spec.IPAddresses) {
violations = append(violations, "spec.ipAddresses")
}
if !util.EqualUnsorted(URLsToString(x509cert.URIs), spec.URIs) {
violations = append(violations, "spec.uris")
}
if !util.EqualUnsorted(x509cert.EmailAddresses, spec.EmailAddresses) {
violations = append(violations, "spec.emailAddresses")
}

45
pkg/util/util.go
View File

@ -28,6 +28,7 @@ import (
"time"
cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
"golang.org/x/exp/slices"
)
func OnlyOneNotNil(items ...interface{}) (any bool, one bool) {
@ -98,31 +99,41 @@ func EqualURLsUnsorted(s1, s2 []*url.URL) bool {
return true
}
// Test for equal IP slices even if unsorted
// EqualIPsUnsorted checks if the given slices of IP addresses contain the same elements, even if in a different order
func EqualIPsUnsorted(s1, s2 []net.IP) bool {
if len(s1) != len(s2) {
return false
}
s1_2, s2_2 := make([]string, len(s1)), make([]string, len(s2))
// we may want to implement a sort interface here instead of []byte conversion
for i := range s1 {
s1_2[i] = string(s1[i])
s2_2[i] = string(s2[i])
// Two IPv4 addresses can compare unequal with bytes.Equal which is why net.IP.Equal exists.
// We still want to sort the lists, though, and we don't want different representations of IPv4 addresses
// to be sorted differently. That can happen if one is stored as a 4-byte address while
// the other is stored as a 16-byte representation
// To avoid ambiguity, we ensure that only the 16-byte form is used for all addresses we work with.
s1_2, s2_2 := make([]net.IP, len(s1)), make([]net.IP, len(s2))
for i := 0; i < len(s1); i++ {
s1_2[i] = s1[i].To16()
s2_2[i] = s2[i].To16()
}
sort.SliceStable(s1_2, func(i, j int) bool {
return s1_2[i] < s1_2[j]
})
sort.SliceStable(s2_2, func(i, j int) bool {
return s2_2[i] < s2_2[j]
// TODO: the function signature will change to func(a net.IP, b net.IP) int when we upgrade to go 1.21
slices.SortFunc(s1_2, func(a net.IP, b net.IP) bool {
// TODO: this will just change to bytes.Compare (without the <= 0) after we upgrade to go 1.21
return bytes.Compare([]byte(a), []byte(b)) <= 0
})
for i, s := range s1_2 {
if s != s2_2[i] {
return false
}
}
return true
// TODO: the function signature will change to func(a net.IP, b net.IP) int when we upgrade to go 1.21
slices.SortFunc(s2_2, func(a net.IP, b net.IP) bool {
// TODO: this will just change to bytes.Compare (without the <= 0) after we upgrade to go 1.21
return bytes.Compare([]byte(a), []byte(b)) <= 0
})
return slices.EqualFunc(s1_2, s2_2, func(a net.IP, b net.IP) bool {
return a.Equal(b)
})
}
// Test for equal KeyUsage slices even if unsorted

107
pkg/util/util_test.go
View File

@ -88,15 +88,104 @@ func TestEqualURLsUnsorted(t *testing.T) {
}
func TestEqualIPsUnsorted(t *testing.T) {
for _, test := range stringSliceTestData {
s1, s2 := parseIPs(t, test.s1), parseIPs(t, test.s2)
t.Run(test.desc, func(test testT) func(*testing.T) {
return func(t *testing.T) {
if actual := EqualIPsUnsorted(s1, s2); actual != test.equal {
t.Errorf("equalIpsUnsorted(%+v, %+v) = %t, but expected %t", s1, s2, actual, test.equal)
}
// This test uses string representations of IP addresses because it's much more convenient to
// represent different types of IPv6 address as strings. This implicitly relies on the behavior
// of net.ParseIP under the hood when it comes to parsing IPv4 addresses, though - it could return
// either a 4 or 16 byte slice to represent an IPv4 address. As such, we have a separate test
// which uses raw net.IP byte slices below, which checks that we're not relying on underlying
// behavior of ParseIP when comparing.
specs := map[string]struct {
s1 []string
s2 []string
expEqual bool
}{
"simple ipv4 comparison": {
s1: []string{"8.8.8.8", "1.1.1.1"},
s2: []string{"1.1.1.1", "8.8.8.8"},
expEqual: true,
},
"simple ipv6 comparison": {
s1: []string{"2a00:1450:4009:822::200e", "2a03:2880:f166:81:face:b00c:0:25de"},
s2: []string{"2a03:2880:f166:81:face:b00c:0:25de", "2a00:1450:4009:822::200e"},
expEqual: true,
},
"mixed ipv4 and ipv6": {
s1: []string{"2a00:1450:4009:822::200e", "2a03:2880:f166:81:face:b00c:0:25de", "1.1.1.1"},
s2: []string{"2a03:2880:f166:81:face:b00c:0:25de", "1.1.1.1", "2a00:1450:4009:822::200e"},
expEqual: true,
},
"mixed ipv6 specificity": {
s1: []string{"2a03:2880:f166:0081:face:b00c:0000:25de"},
s2: []string{"2a03:2880:f166:81:face:b00c:0:25de"},
expEqual: true,
},
"unequal addresses ipv6": {
s1: []string{"2a03:2880:f166:0081:face::25de"},
s2: []string{"2a03:2880:f166:81:face:b00c:1:25de"},
expEqual: false,
},
}
for name, spec := range specs {
s1 := parseIPs(t, spec.s1)
s2 := parseIPs(t, spec.s2)
t.Run(name, func(t *testing.T) {
got := EqualIPsUnsorted(s1, s2)
if got != spec.expEqual {
t.Errorf("EqualIPsUnsorted(%+v, %+v) = %t, but expected %t", s1, s2, got, spec.expEqual)
}
}(test))
})
}
}
func TestEqualIPsUnsorted_RawIPs(t *testing.T) {
// See description in TestEqualIPsUnsorted for motivation here
specs := map[string]struct {
s1 []net.IP
s2 []net.IP
expEqual bool
}{
"simple ipv4 comparison": {
s1: []net.IP{net.IP([]byte{0x1, 0x1, 0x1, 0x1}), net.IP([]byte{0x8, 0x8, 0x8, 0x8})},
s2: []net.IP{net.IP([]byte{0x8, 0x8, 0x8, 0x8}), net.IP([]byte{0x1, 0x1, 0x1, 0x1})},
expEqual: true,
},
"simple ipv6 comparison": {
s1: []net.IP{
net.IP([]byte{0x2a, 0xe, 0x23, 0x45, 0x67, 0x89, 0x0, 0x1, 0x0, 0x1, 0x0, 0x2, 0x0, 0x0, 0x0, 0x6}),
net.IP([]byte{0x2a, 0x03, 0x28, 0x80, 0xf1, 0x66, 0x00, 0x81, 0xfa, 0xce, 0xb0, 0x0c, 0x00, 0x00, 0x25, 0xde}),
},
s2: []net.IP{
net.IP([]byte{0x2a, 0x03, 0x28, 0x80, 0xf1, 0x66, 0x00, 0x81, 0xfa, 0xce, 0xb0, 0x0c, 0x00, 0x00, 0x25, 0xde}),
net.IP([]byte{0x2a, 0xe, 0x23, 0x45, 0x67, 0x89, 0x0, 0x1, 0x0, 0x1, 0x0, 0x2, 0x0, 0x0, 0x0, 0x6}),
},
expEqual: true,
},
"mixed ipv4 lengths": {
// This is the most important test in this test function!
// IPv4 addresses have two valid representations as `net.IP`s and we shouldn't miss the case where they're equal
s1: []net.IP{
net.IP([]byte{0xa, 0x0, 0x0, 0xce}),
},
s2: []net.IP{
net.IP([]byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xa, 0x0, 0x0, 0xce}),
},
expEqual: true,
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
got := EqualIPsUnsorted(spec.s1, spec.s2)
if got != spec.expEqual {
t.Errorf("EqualIPsUnsorted(%+v, %+v) = %t, but expected %t", spec.s1, spec.s2, got, spec.expEqual)
}
})
}
}
@ -158,7 +247,7 @@ func parseIPs(t *testing.T, ipStrs []string) []net.IP {
var ips []net.IP
for _, i := range ipStrs {
ips = append(ips, []byte(i))
ips = append(ips, net.ParseIP(i))
}
return ips