Skip to content

Implement TBS certificate hash pinning and add tests for verification #507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maederm wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement TBS certificate hash pinning and add tests for verification #507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions upstream/upstream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
package upstream

import (
"bytes"
"context"
"crypto/sha256"
"crypto/tls"
"crypto/x509"
"fmt"
Expand Down Expand Up @@ -182,11 +184,11 @@ const (
//
// opts are applied to the u and shouldn't be modified afterwards, nil value is
// valid.
//
// TODO(e.burkov): Clone opts?
func AddressToUpstream(addr string, opts *Options) (u Upstream, err error) {
if opts == nil {
opts = &Options{}
} else {
opts = opts.Clone()
}

if opts.Logger == nil {
Expand Down Expand Up @@ -298,6 +300,22 @@ func parseStamp(upsURL *url.URL, opts *Options) (u Upstream, err error) {
opts.Bootstrap = StaticResolver{ip}
}

if len(stamp.Hashes) > 0 {
existingVerifyConn := opts.VerifyConnection
hashes := stamp.Hashes
opts.VerifyConnection = func(state tls.ConnectionState) error {
if pinErr := verifyTBSPinning(state.PeerCertificates, hashes); pinErr != nil {
return pinErr
}

if existingVerifyConn != nil {
return existingVerifyConn(state)
}

return nil
}
}

switch stamp.Proto {
case dnsstamps.StampProtoTypePlain:
return newPlain(&url.URL{Scheme: "udp", Host: stamp.ServerAddrStr}, opts)
Expand All @@ -314,6 +332,22 @@ func parseStamp(upsURL *url.URL, opts *Options) (u Upstream, err error) {
}
}

// verifyTBSPinning checks that at least one certificate in certs has a
// SHA256(RawTBSCertificate) matching one of the expected hashes.
func verifyTBSPinning(certs []*x509.Certificate, hashes [][]uint8) error {
for _, cert := range certs {
tbsHash := sha256.Sum256(cert.RawTBSCertificate)
for _, expected := range hashes {
if bytes.Equal(tbsHash[:], expected) {
return nil
}
}
}

return errors.Error("tbs certificate hash pinning failed: " +
"no certificate in the chain matches any expected hash")
}

// addPort appends port to u if it's absent.
func addPort(u *url.URL, port uint16) {
if u != nil {
Expand Down
192 changes: 188 additions & 4 deletions upstream/upstream_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/ecdsa"
"crypto/rand"
"crypto/rsa"
"crypto/sha256"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
Expand Down Expand Up @@ -188,9 +189,9 @@ func TestUpstreams(t *testing.T) {
bootstrap: googleBoot,
address: "sdns://AQMAAAAAAAAAETk0LjE0MC4xNC4xNTo1NDQzILgxXdexS27jIKRw3C7Wsao5jMnlhvhdRUXWuMm1AFq6ITIuZG5zY3J5cHQuZmFtaWx5Lm5zMS5hZGd1YXJkLmNvbQ",
}, {
// Cloudflare DNS (DNS-over-HTTPS)
// Cloudflare DNS (DNS-over-HTTPS, no pinned hashes)
bootstrap: googleBoot,
address: "sdns://AgcAAAAAAAAABzEuMC4wLjGgENk8mGSlIfMGXMOlIlCcKvq7AVgcrZxtjon911-ep0cg63Ul-I8NlFj4GplQGb_TTLiczclX57DvMV8Q-JdjgRgSZG5zLmNsb3VkZmxhcmUuY29tCi9kbnMtcXVlcnk",
address: "sdns://AgcAAAAAAAAABzEuMC4wLjEAEmRucy5jbG91ZGZsYXJlLmNvbQovZG5zLXF1ZXJ5",
}, {
// Google (Plain)
bootstrap: nil,
Expand Down Expand Up @@ -420,8 +421,8 @@ func TestUpstreamsInvalidBootstrap(t *testing.T) {
address: "https://1dot1dot1dot1.cloudflare-dns.com/dns-query",
bootstrap: []string{"8.8.8.1", "1.0.0.1"},
}, {
// Cloudflare DNS (DoH)
address: "sdns://AgcAAAAAAAAABzEuMC4wLjGgENk8mGSlIfMGXMOlIlCcKvq7AVgcrZxtjon911-ep0cg63Ul-I8NlFj4GplQGb_TTLiczclX57DvMV8Q-JdjgRgSZG5zLmNsb3VkZmxhcmUuY29tCi9kbnMtcXVlcnk",
// Cloudflare DNS (DoH, no pinned hashes)
address: "sdns://AgcAAAAAAAAABzEuMC4wLjEAEmRucy5jbG91ZGZsYXJlLmNvbQovZG5zLXF1ZXJ5",
bootstrap: []string{"8.8.8.8:53", "8.8.8.1:53"},
}, {
// AdGuard DNS (DNS-over-TLS)
Expand Down Expand Up @@ -873,3 +874,186 @@ func (r *headerRecorder) headersWithLock() (res []qlog.PacketHeader) {
func (*headerRecorder) Close() (err error) {
return nil
}

func TestVerifyTBSPinning(t *testing.T) {
t.Parallel()

tlsConf, _ := createServerTLSConfig(t, "127.0.0.1")
cert, err := x509.ParseCertificate(tlsConf.Certificates[0].Certificate[0])
require.NoError(t, err)

tbsHash := sha256.Sum256(cert.RawTBSCertificate)

t.Run("match", func(t *testing.T) {
t.Parallel()

err := verifyTBSPinning([]*x509.Certificate{cert}, [][]uint8{tbsHash[:]})
assert.NoError(t, err)
})

t.Run("no_match", func(t *testing.T) {
t.Parallel()

wrongHash := make([]byte, 32)
err := verifyTBSPinning([]*x509.Certificate{cert}, [][]uint8{wrongHash})
assert.Error(t, err)
})

t.Run("empty_certs", func(t *testing.T) {
t.Parallel()

err := verifyTBSPinning(nil, [][]uint8{tbsHash[:]})
assert.Error(t, err)
})

t.Run("empty_hashes", func(t *testing.T) {
t.Parallel()

err := verifyTBSPinning([]*x509.Certificate{cert}, nil)
assert.Error(t, err)
})
}

func TestParseStamp_tbsPinning(t *testing.T) {
t.Parallel()

handler := func(w dns.ResponseWriter, req *dns.Msg) {
require.NoError(testutil.PanicT{}, w.WriteMsg(respondToTestMessage(req)))
}

dotSrv := startDoTServer(t, handler)
dohSrv := startDoHServer(t, testDoHServerOptions{})

doqTLSConf, _ := createServerTLSConfig(t, "127.0.0.1")
doqSrv := startDoQServer(t, doqTLSConf, 0)

testCases := []struct {
name string
tlsConfig *tls.Config
addr string
proto dnsstamps.StampProtoType
path string
}{{
name: "dot",
tlsConfig: dotSrv.tlsConfig,
addr: fmt.Sprintf("127.0.0.1:%d", dotSrv.port),
proto: dnsstamps.StampProtoTypeTLS,
}, {
name: "doh",
tlsConfig: dohSrv.tlsConfig,
addr: dohSrv.addr,
proto: dnsstamps.StampProtoTypeDoH,
path: "/dns-query",
}, {
name: "doq",
tlsConfig: doqTLSConf,
addr: doqSrv.addr,
proto: dnsstamps.StampProtoTypeDoQ,
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

serverCert, err := x509.ParseCertificate(tc.tlsConfig.Certificates[0].Certificate[0])
require.NoError(t, err)

tbsHash := sha256.Sum256(serverCert.RawTBSCertificate)
correctHash := tbsHash[:]

wrongHash := make([]byte, 32)
copy(wrongHash, correctHash)
wrongHash[0] ^= 0xFF

t.Run("correct_hash", func(t *testing.T) {
t.Parallel()

stamp := dnsstamps.ServerStamp{
ServerAddrStr: tc.addr,
Proto: tc.proto,
ProviderName: tc.addr,
Hashes: [][]uint8{correctHash},
Path: tc.path,
}
stampStr := stamp.String()
u, err := AddressToUpstream(stampStr, &Options{
Logger: testLogger,
InsecureSkipVerify: true,
Timeout: 5 * time.Second,
})
require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, u.Close)

checkUpstream(t, u, stampStr)
})

t.Run("wrong_hash", func(t *testing.T) {
t.Parallel()

stamp := dnsstamps.ServerStamp{
ServerAddrStr: tc.addr,
Proto: tc.proto,
ProviderName: tc.addr,
Hashes: [][]uint8{wrongHash},
Path: tc.path,
}
stampStr := stamp.String()
u, err := AddressToUpstream(stampStr, &Options{
Logger: testLogger,
InsecureSkipVerify: true,
Timeout: 5 * time.Second,
})
require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, u.Close)

req := createTestMessage()
_, err = u.Exchange(req)
require.Error(t, err)
assert.ErrorContains(t, err, "tbs certificate hash pinning failed")
})

t.Run("multiple_hashes_one_correct", func(t *testing.T) {
t.Parallel()

stamp := dnsstamps.ServerStamp{
ServerAddrStr: tc.addr,
Proto: tc.proto,
ProviderName: tc.addr,
Hashes: [][]uint8{wrongHash, correctHash},
Path: tc.path,
}
stampStr := stamp.String()
u, err := AddressToUpstream(stampStr, &Options{
Logger: testLogger,
InsecureSkipVerify: true,
Timeout: 5 * time.Second,
})
require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, u.Close)

checkUpstream(t, u, stampStr)
})

t.Run("no_hashes", func(t *testing.T) {
t.Parallel()

stamp := dnsstamps.ServerStamp{
ServerAddrStr: tc.addr,
Proto: tc.proto,
ProviderName: tc.addr,
Path: tc.path,
}
stampStr := stamp.String()
u, err := AddressToUpstream(stampStr, &Options{
Logger: testLogger,
InsecureSkipVerify: true,
Timeout: 5 * time.Second,
})
require.NoError(t, err)
testutil.CleanupAndRequireSuccess(t, u.Close)

checkUpstream(t, u, stampStr)
})
})
}
}