From 135e912ac8fe118fe1cf22dd49777b2813b9e7ba Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 18 Jun 2021 17:27:35 +0200 Subject: [PATCH] Improve coverage for TLS-ALPN-01 challenge --- acme/challenge.go | 31 ++++++----- acme/challenge_test.go | 117 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 14 deletions(-) diff --git a/acme/challenge.go b/acme/challenge.go index 706d12a1..fe643c85 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -115,25 +115,13 @@ func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWeb } func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebKey, vo *ValidateChallengeOptions) error { - - // RFC8738 states that, if HostName is IP, it should be the ARPA - // address https://datatracker.ietf.org/doc/html/rfc8738#section-6. - // It also references TLS Extensions [RFC6066]. - var serverName string - ip := net.ParseIP(ch.Value) - if ip != nil { - serverName = reverseAddr(ip) - } else { - serverName = ch.Value - } - config := &tls.Config{ NextProtos: []string{"acme-tls/1"}, // https://tools.ietf.org/html/rfc8737#section-4 // ACME servers that implement "acme-tls/1" MUST only negotiate TLS 1.2 // [RFC5246] or higher when connecting to clients for validation. MinVersion: tls.VersionTLS12, - ServerName: serverName, + ServerName: serverName(ch), InsecureSkipVerify: true, // we expect a self-signed challenge certificate } @@ -163,7 +151,7 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON // if no DNS names present, look for IP address and verify that exactly one exists if len(leafCert.DNSNames) == 0 { - if len(leafCert.IPAddresses) != 1 || !leafCert.IPAddresses[0].Equal(ip) { + if len(leafCert.IPAddresses) != 1 || !leafCert.IPAddresses[0].Equal(net.ParseIP(ch.Value)) { return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType, "incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value)) } @@ -272,6 +260,21 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK return nil } +// serverName determines the SNI HostName to set based on an acme.Challenge +// for TLS-ALPN-01 challenges. RFC8738 states that, if HostName is an IP, it +// should be the ARPA address https://datatracker.ietf.org/doc/html/rfc8738#section-6. +// It also references TLS Extensions [RFC6066]. +func serverName(ch *Challenge) string { + var serverName string + ip := net.ParseIP(ch.Value) + if ip != nil { + serverName = reverseAddr(ip) + } else { + serverName = ch.Value + } + return serverName +} + // reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP // address addr suitable for rDNS (PTR) record lookup or an error if it fails // to parse the IP address. diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 8e03a414..727e9ef3 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -2187,6 +2187,43 @@ func TestTLSALPN01Validate(t *testing.T) { srv, tlsDial := newTestTLSALPNServer(cert) srv.Start() + return test{ + ch: ch, + vo: &ValidateChallengeOptions{ + TLSDial: tlsDial, + }, + db: &MockDB{ + MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error { + assert.Equals(t, updch.ID, ch.ID) + assert.Equals(t, updch.Token, ch.Token) + assert.Equals(t, updch.Status, StatusValid) + assert.Equals(t, updch.Type, ch.Type) + assert.Equals(t, updch.Value, ch.Value) + assert.Equals(t, updch.Error, nil) + return nil + }, + }, + srv: srv, + jwk: jwk, + } + }, + "ok/ip": func(t *testing.T) test { + ch := makeTLSCh() + ch.Value = "127.0.0.1" + + jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + assert.FatalError(t, err) + + expKeyAuth, err := KeyAuthorization(ch.Token, jwk) + assert.FatalError(t, err) + expKeyAuthHash := sha256.Sum256([]byte(expKeyAuth)) + + cert, err := newTLSALPNValidationCert(expKeyAuthHash[:], false, true, ch.Value) + assert.FatalError(t, err) + + srv, tlsDial := newTestTLSALPNServer(cert) + srv.Start() + return test{ ch: ch, vo: &ValidateChallengeOptions{ @@ -2234,4 +2271,84 @@ func TestTLSALPN01Validate(t *testing.T) { } }) } + t.Fail() +} + +func Test_reverseAddr(t *testing.T) { + type args struct { + ip net.IP + } + tests := []struct { + name string + args args + wantArpa string + }{ + { + name: "ok/ipv4", + args: args{ + ip: net.ParseIP("127.0.0.1"), + }, + wantArpa: "1.0.0.127.in-addr.arpa.", + }, + { + name: "ok/ipv6", + args: args{ + ip: net.ParseIP("2001:db8::567:89ab"), + }, + wantArpa: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if gotArpa := reverseAddr(tt.args.ip); gotArpa != tt.wantArpa { + t.Errorf("reverseAddr() = %v, want %v", gotArpa, tt.wantArpa) + } + }) + } +} + +func Test_serverName(t *testing.T) { + type args struct { + ch *Challenge + } + tests := []struct { + name string + args args + want string + }{ + { + name: "ok/dns", + args: args{ + ch: &Challenge{ + Value: "example.com", + }, + }, + want: "example.com", + }, + { + name: "ok/ipv4", + args: args{ + ch: &Challenge{ + Value: "127.0.0.1", + }, + }, + want: "1.0.0.127.in-addr.arpa.", + }, + { + name: "ok/ipv4", + args: args{ + ch: &Challenge{ + Value: "2001:db8::567:89ab", + }, + }, + want: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := serverName(tt.args.ch); got != tt.want { + t.Errorf("serverName() = %v, want %v", got, tt.want) + } + }) + } }