From 218a2adb9fc5778678fff7b323ee4943e2d4d7a7 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 18 Jun 2021 16:09:48 +0200 Subject: [PATCH] Add tests for IP Order validations --- acme/order.go | 12 +- acme/order_test.go | 375 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 380 insertions(+), 7 deletions(-) diff --git a/acme/order.go b/acme/order.go index 86b3c43a..71884603 100644 --- a/acme/order.go +++ b/acme/order.go @@ -199,15 +199,15 @@ func (o *Order) sans(csr *x509.CertificateRequest) ([]x509util.SubjectAlternativ var sans []x509util.SubjectAlternativeName // order the DNS names and IP addresses, so that they can be compared against the canonicalized CSR - orderNames := make([]string, numberOfIdentifierType("dns", o.Identifiers)) - orderIPs := make([]net.IP, numberOfIdentifierType("ip", o.Identifiers)) + orderNames := make([]string, numberOfIdentifierType(DNS, o.Identifiers)) + orderIPs := make([]net.IP, numberOfIdentifierType(IP, o.Identifiers)) indexDNS, indexIP := 0, 0 for _, n := range o.Identifiers { switch n.Type { - case "dns": + case DNS: orderNames[indexDNS] = n.Value indexDNS++ - case "ip": + case IP: orderIPs[indexIP] = net.ParseIP(n.Value) // NOTE: this assumes are all valid IPs at this time; or will result in nil entries indexIP++ default: @@ -303,8 +303,8 @@ func canonicalize(csr *x509.CertificateRequest) (canonicalized *x509.Certificate } // ipsAreEqual compares IPs to be equal. IPv6 representations of IPv4 -// adresses are NOT considered equal to the IPv4 address in this case. -// Both IPs should be the same version AND equal to each other. +// adresses are considered equal to the IPv4 address in this case. +// TODO: is this behavior OK to keep? func ipsAreEqual(x, y net.IP) bool { if matchAddrFamily(x, y) { return x.Equal(y) diff --git a/acme/order_test.go b/acme/order_test.go index c0461dc6..e1d3744b 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -393,6 +393,31 @@ func TestOrder_Finalize(t *testing.T) { "CSR names = %v, Order names = %v", []string{"foo.internal"}, orderNames), } }, + "fail/error-ips-length-mismatch": func(t *testing.T) test { + now := clock.Now() + o := &Order{ + ID: "oID", + AccountID: "accID", + Status: StatusReady, + ExpiresAt: now.Add(5 * time.Minute), + AuthorizationIDs: []string{"a", "b"}, + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + } + orderIPs := []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")} + csr := &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42")}, + } + + return test{ + o: o, + csr: csr, + err: NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", []net.IP{net.ParseIP("192.168.42.42")}, orderIPs), + } + }, "fail/error-names-mismatch": func(t *testing.T) test { now := clock.Now() o := &Order{ @@ -421,6 +446,31 @@ func TestOrder_Finalize(t *testing.T) { "CSR names = %v, Order names = %v", []string{"foo.internal", "zap.internal"}, orderNames), } }, + "fail/error-ips-mismatch": func(t *testing.T) test { + now := clock.Now() + o := &Order{ + ID: "oID", + AccountID: "accID", + Status: StatusReady, + ExpiresAt: now.Add(5 * time.Minute), + AuthorizationIDs: []string{"a", "b"}, + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + } + orderIPs := []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")} + csr := &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.42.32")}, + } + + return test{ + o: o, + csr: csr, + err: NewError(ErrorBadCSRType, "CSR IPs do not match identifiers exactly: "+ + "CSR IPs = %v, Order IPs = %v", []net.IP{net.ParseIP("192.168.42.32"), net.ParseIP("192.168.42.42")}, orderIPs), + } + }, "fail/error-provisioner-auth": func(t *testing.T) test { now := clock.Now() o := &Order{ @@ -652,7 +702,7 @@ func TestOrder_Finalize(t *testing.T) { err: NewErrorISE("error updating order oID: force"), } }, - "ok/new-cert": func(t *testing.T) test { + "ok/new-cert-dns": func(t *testing.T) test { now := clock.Now() o := &Order{ ID: "oID", @@ -676,6 +726,131 @@ func TestOrder_Finalize(t *testing.T) { bar := &x509.Certificate{Subject: pkix.Name{CommonName: "bar"}} baz := &x509.Certificate{Subject: pkix.Name{CommonName: "baz"}} + return test{ + o: o, + csr: csr, + prov: &MockProvisioner{ + MauthorizeSign: func(ctx context.Context, token string) ([]provisioner.SignOption, error) { + assert.Equals(t, token, "") + return nil, nil + }, + MgetOptions: func() *provisioner.Options { + return nil + }, + }, + ca: &mockSignAuth{ + sign: func(_csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, _csr, csr) + return []*x509.Certificate{foo, bar, baz}, nil + }, + }, + db: &MockDB{ + MockCreateCertificate: func(ctx context.Context, cert *Certificate) error { + cert.ID = "certID" + assert.Equals(t, cert.AccountID, o.AccountID) + assert.Equals(t, cert.OrderID, o.ID) + assert.Equals(t, cert.Leaf, foo) + assert.Equals(t, cert.Intermediates, []*x509.Certificate{bar, baz}) + return nil + }, + MockUpdateOrder: func(ctx context.Context, updo *Order) error { + assert.Equals(t, updo.CertificateID, "certID") + assert.Equals(t, updo.Status, StatusValid) + assert.Equals(t, updo.ID, o.ID) + assert.Equals(t, updo.AccountID, o.AccountID) + assert.Equals(t, updo.ExpiresAt, o.ExpiresAt) + assert.Equals(t, updo.AuthorizationIDs, o.AuthorizationIDs) + assert.Equals(t, updo.Identifiers, o.Identifiers) + return nil + }, + }, + } + }, + "ok/new-cert-ip": func(t *testing.T) test { + now := clock.Now() + o := &Order{ + ID: "oID", + AccountID: "accID", + Status: StatusReady, + ExpiresAt: now.Add(5 * time.Minute), + AuthorizationIDs: []string{"a", "b"}, + Identifiers: []Identifier{ + {Type: "ip", Value: "192.168.42.42"}, + {Type: "ip", Value: "192.168.43.42"}, + }, + } + csr := &x509.CertificateRequest{ + IPAddresses: []net.IP{net.ParseIP("192.168.42.42"), net.ParseIP("192.168.43.42")}, // in case of IPs, no Common Name + } + + foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} + bar := &x509.Certificate{Subject: pkix.Name{CommonName: "bar"}} + baz := &x509.Certificate{Subject: pkix.Name{CommonName: "baz"}} + + return test{ + o: o, + csr: csr, + prov: &MockProvisioner{ + MauthorizeSign: func(ctx context.Context, token string) ([]provisioner.SignOption, error) { + assert.Equals(t, token, "") + return nil, nil + }, + MgetOptions: func() *provisioner.Options { + return nil + }, + }, + ca: &mockSignAuth{ + sign: func(_csr *x509.CertificateRequest, signOpts provisioner.SignOptions, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, _csr, csr) + return []*x509.Certificate{foo, bar, baz}, nil + }, + }, + db: &MockDB{ + MockCreateCertificate: func(ctx context.Context, cert *Certificate) error { + cert.ID = "certID" + assert.Equals(t, cert.AccountID, o.AccountID) + assert.Equals(t, cert.OrderID, o.ID) + assert.Equals(t, cert.Leaf, foo) + assert.Equals(t, cert.Intermediates, []*x509.Certificate{bar, baz}) + return nil + }, + MockUpdateOrder: func(ctx context.Context, updo *Order) error { + assert.Equals(t, updo.CertificateID, "certID") + assert.Equals(t, updo.Status, StatusValid) + assert.Equals(t, updo.ID, o.ID) + assert.Equals(t, updo.AccountID, o.AccountID) + assert.Equals(t, updo.ExpiresAt, o.ExpiresAt) + assert.Equals(t, updo.AuthorizationIDs, o.AuthorizationIDs) + assert.Equals(t, updo.Identifiers, o.Identifiers) + return nil + }, + }, + } + }, + "ok/new-cert-dns-and-ip": func(t *testing.T) test { + now := clock.Now() + o := &Order{ + ID: "oID", + AccountID: "accID", + Status: StatusReady, + ExpiresAt: now.Add(5 * time.Minute), + AuthorizationIDs: []string{"a", "b"}, + Identifiers: []Identifier{ + {Type: "dns", Value: "foo.internal"}, + {Type: "ip", Value: "192.168.42.42"}, + }, + } + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "foo.internal", + }, + IPAddresses: []net.IP{net.ParseIP("192.168.42.42")}, + } + + foo := &x509.Certificate{Subject: pkix.Name{CommonName: "foo"}} + bar := &x509.Certificate{Subject: pkix.Name{CommonName: "bar"}} + baz := &x509.Certificate{Subject: pkix.Name{CommonName: "baz"}} + return test{ o: o, csr: csr, @@ -814,3 +989,201 @@ func Test_uniqueSortedIPs(t *testing.T) { }) } } + +func Test_numberOfIdentifierType(t *testing.T) { + type args struct { + typ IdentifierType + ids []Identifier + } + tests := []struct { + name string + args args + want int + }{ + { + name: "ok/no-identifiers", + args: args{ + typ: DNS, + ids: []Identifier{}, + }, + want: 0, + }, + { + name: "ok/no-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 0, + }, + { + name: "ok/no-ips", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + }, + }, + want: 0, + }, + { + name: "ok/one-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 1, + }, + { + name: "ok/one-ip", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 1, + }, + { + name: "ok/more-dns", + args: args{ + typ: DNS, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: DNS, + Value: "*.example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + }, + }, + want: 2, + }, + { + name: "ok/more-ips", + args: args{ + typ: IP, + ids: []Identifier{ + { + Type: DNS, + Value: "example.com", + }, + { + Type: IP, + Value: "192.168.42.42", + }, + { + Type: IP, + Value: "192.168.42.43", + }, + }, + }, + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := numberOfIdentifierType(tt.args.typ, tt.args.ids); got != tt.want { + t.Errorf("numberOfIdentifierType() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_ipsAreEqual(t *testing.T) { + type args struct { + x net.IP + y net.IP + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "ok/ipv4", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("192.168.42.42"), + }, + want: true, + }, + { + name: "ok/ipv6", + args: args{ + x: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + }, + want: true, + }, + { + name: "ok/ipv4-and-ipv6", + args: args{ + x: net.ParseIP("192.168.42.42"), + y: net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334"), + }, + want: false, + }, + { + name: "ok/ipv4-mapped-to-ipv6", + args: args{ + 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? + }, + { + name: "ok/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"), + }, + want: false, + }, + { + name: "ok/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"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ipsAreEqual(tt.args.x, tt.args.y); got != tt.want { + t.Errorf("ipsAreEqual() = %v, want %v", got, tt.want) + } + }) + } +}