From 4b43bbd34a6f71c36d44fea0a41b1d0b2ef7778b Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Fri, 20 Jun 2025 08:34:52 -0700 Subject: [PATCH] refactor(webtransport): Use keygen package for deterministic ecdsa key generation. --- go.mod | 4 +- go.sum | 8 ++++ p2p/transport/webtransport/crypto.go | 49 +++++++++++++---------- p2p/transport/webtransport/crypto_test.go | 38 ------------------ 4 files changed, 38 insertions(+), 61 deletions(-) diff --git a/go.mod b/go.mod index e2826b9ec1..b347bd4209 100644 --- a/go.mod +++ b/go.mod @@ -1,12 +1,13 @@ module github.com/libp2p/go-libp2p -go 1.23.8 +go 1.24.4 retract v0.26.1 // Tag was applied incorrectly due to a bug in the release workflow. retract v0.36.0 // Accidentally modified the tag. require ( + filippo.io/keygen v0.0.0-20240718133620-7f162efbbd87 github.com/benbjohnson/clock v1.3.5 github.com/davidlazar/go-crypto v0.0.0-20200604182044-b73af7476f6c github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 @@ -69,6 +70,7 @@ require ( ) require ( + filippo.io/bigmod v0.0.3 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect diff --git a/go.sum b/go.sum index 5d032afff3..d7d22a3a98 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,10 @@ dmitri.shuralyov.com/app/changes v0.0.0-20180602232624-0a106ad413e3/go.mod h1:Yl dmitri.shuralyov.com/html/belt v0.0.0-20180602232347-f7d459c86be0/go.mod h1:JLBrvjyP0v+ecvNYvCpyZgu5/xkfAUhi6wJj28eUfSU= dmitri.shuralyov.com/service/change v0.0.0-20181023043359-a85b471d5412/go.mod h1:a1inKt/atXimZ4Mv927x+r7UpyzRUf4emIoiiSC2TN4= dmitri.shuralyov.com/state v0.0.0-20180228185332-28bcc343414c/go.mod h1:0PRwlb0D6DFvNNtx+9ybjezNCa8XF0xaYcETyp6rHWU= +filippo.io/bigmod v0.0.3 h1:qmdCFHmEMS+PRwzrW6eUrgA4Q3T8D6bRcjsypDMtWHM= +filippo.io/bigmod v0.0.3/go.mod h1:WxGvOYE0OUaBC2N112Dflb3CjOnMBuNRA2UWZc2UbPE= +filippo.io/keygen v0.0.0-20240718133620-7f162efbbd87 h1:HlcHAMbI9Xvw3aWnhPngghMl5AKE2GOvjmvSGOKzCcI= +filippo.io/keygen v0.0.0-20240718133620-7f162efbbd87/go.mod h1:nAs0+DyACEQGudhkTwlPC9atyqDYC7ZotgZR7D8OwXM= git.apache.org/thrift.git v0.0.0-20180902110319-2566ecd5d999/go.mod h1:fPE2ZNJGynbRyZ4dJvy6G277gSllfV2HJqblrnkyeyg= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= @@ -16,6 +20,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625/go.mod h1:HYsPBTaaSFSlLx/70C2HPIMNZpVV8+vt/A+FMnYP11g= github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s= +github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 h1:oe6fCvaEpkhyW3qAicT0TnGtyht/UrgvOwMcEgLb7Aw= +github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3/go.mod h1:qdP0gaj0QtgX2RUZhnlVrceJ+Qln8aSlDyJwelLLFeM= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= @@ -432,6 +438,8 @@ golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20181030000543-1d582fd0359e/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.1.0/go.mod h1:UGEZY7KEX120AnNLIHFMKIo4obdJhkp2tPbaPlQx13Y= diff --git a/p2p/transport/webtransport/crypto.go b/p2p/transport/webtransport/crypto.go index 90504ead01..a2daae6cd8 100644 --- a/p2p/transport/webtransport/crypto.go +++ b/p2p/transport/webtransport/crypto.go @@ -2,6 +2,7 @@ package libp2pwebtransport import ( "bytes" + "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/sha256" @@ -17,6 +18,8 @@ import ( "golang.org/x/crypto/hkdf" + "filippo.io/keygen" + ic "github.com/libp2p/go-libp2p/core/crypto" "github.com/multiformats/go-multihash" @@ -71,11 +74,16 @@ func generateCert(key ic.PrivKey, start, end time.Time) (*x509.Certificate, *ecd BasicConstraintsValid: true, } - caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), deterministicHKDFReader) + ecdsaSeed := make([]byte, 192/8) // 192 bits of entropy required for P256 + if _, err := io.ReadFull(deterministicHKDFReader, ecdsaSeed); err != nil { + return nil, nil, err + } + + caPrivateKey, err := keygen.ECDSA(elliptic.P256(), ecdsaSeed) if err != nil { return nil, nil, err } - caBytes, err := x509.CreateCertificate(deterministicHKDFReader, certTempl, certTempl, caPrivateKey.Public(), caPrivateKey) + caBytes, err := x509.CreateCertificate(deterministicHKDFReader, certTempl, certTempl, caPrivateKey.Public(), deterministicSigner{caPrivateKey}) if err != nil { return nil, nil, err } @@ -136,29 +144,26 @@ func verifyRawCerts(rawCerts [][]byte, certHashes []multihash.DecodedMultihash) return nil } -// deterministicReader is a hack. It counter-acts the Go library's attempt at -// making ECDSA signatures non-deterministic. Go adds non-determinism by -// randomly dropping a singly byte from the reader stream. This counteracts this -// by detecting when a read is a single byte and using a different reader -// instead. -type deterministicReader struct { - reader io.Reader - singleByteReader io.Reader +func newDeterministicReader(seed []byte, salt []byte, info string) io.Reader { + return hkdf.New(sha256.New, seed, salt, []byte(info)) +} + +// deterministicSigner wraps an ecdsa.PrivateKey and exposes a `Sign` method +// that will produce deterministic signatures by ignoring the rand reader. +// Go 1.24 produces deterministic ecdsa signatures when passed a nil random source. +// See: https://go.dev/doc/go1.24#cryptoecdsapkgcryptoecdsa +type deterministicSigner struct { + priv *ecdsa.PrivateKey } -func newDeterministicReader(seed []byte, salt []byte, info string) io.Reader { - reader := hkdf.New(sha256.New, seed, salt, []byte(info)) - singleByteReader := hkdf.New(sha256.New, seed, salt, []byte(info+" single byte")) +var _ crypto.Signer = deterministicSigner{} - return &deterministicReader{ - reader: reader, - singleByteReader: singleByteReader, - } +func (ds deterministicSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + // Ignore the rand reader to produce deterministic signatures. + _ = rand + return ds.priv.Sign(nil, digest, opts) } -func (r *deterministicReader) Read(p []byte) (n int, err error) { - if len(p) == 1 { - return r.singleByteReader.Read(p) - } - return r.reader.Read(p) +func (ds deterministicSigner) Public() crypto.PublicKey { + return ds.priv.Public() } diff --git a/p2p/transport/webtransport/crypto_test.go b/p2p/transport/webtransport/crypto_test.go index ba439c28af..4656e8f912 100644 --- a/p2p/transport/webtransport/crypto_test.go +++ b/p2p/transport/webtransport/crypto_test.go @@ -11,7 +11,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" - "io" "math/big" mrand "math/rand" "testing" @@ -157,40 +156,3 @@ func TestDeterministicCertHashes(t *testing.T) { require.Equal(t, keyBytes, keyBytes2) } } - -// TestDeterministicSig tests that our hack around making ECDSA signatures -// deterministic works. If this fails, this means we need to try another -// strategy to make deterministic signatures or try something else entirely. -// See deterministicReader for more context. -func TestDeterministicSig(t *testing.T) { - // Run this test 1000 times since we want to make sure the signatures are deterministic - runs := 1000 - for i := 0; i < runs; i++ { - zeroSeed := [32]byte{} - deterministicHKDFReader := newDeterministicReader(zeroSeed[:], nil, deterministicCertInfo) - b := [1024]byte{} - io.ReadFull(deterministicHKDFReader, b[:]) - caPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), deterministicHKDFReader) - require.NoError(t, err) - - sig, err := caPrivateKey.Sign(deterministicHKDFReader, b[:], crypto.SHA256) - require.NoError(t, err) - - deterministicHKDFReader = newDeterministicReader(zeroSeed[:], nil, deterministicCertInfo) - b2 := [1024]byte{} - io.ReadFull(deterministicHKDFReader, b2[:]) - caPrivateKey2, err := ecdsa.GenerateKey(elliptic.P256(), deterministicHKDFReader) - require.NoError(t, err) - - sig2, err := caPrivateKey2.Sign(deterministicHKDFReader, b2[:], crypto.SHA256) - require.NoError(t, err) - - keyBytes, err := x509.MarshalECPrivateKey(caPrivateKey) - require.NoError(t, err) - keyBytes2, err := x509.MarshalECPrivateKey(caPrivateKey2) - require.NoError(t, err) - - require.Equal(t, sig, sig2) - require.Equal(t, keyBytes, keyBytes2) - } -}