From b36c3afbaac6334d89f1e3492ad6f9d6d622132d Mon Sep 17 00:00:00 2001 From: Bhargava Shastry Date: Wed, 6 May 2026 10:53:48 +0200 Subject: [PATCH 1/2] sign/dilithium: hold tr by value in pk returned from Public() and NewKeyFromSeed Previously the PublicKey returned by (*PrivateKey).Public() and NewKeyFromSeed cached tr as a pointer aliasing sk.tr. As a result, any later mutation of sk.tr -- for example via a subsequent (*PrivateKey).Unpack on attacker-controlled bytes -- silently propagated into a pk that had already been returned to the caller. Switch both sites to a value copy. The returned pk now holds its own [TRSize]byte and is independent of subsequent changes to sk. Refs: cloudflare/circl#600 Edited in sign/dilithium/mode3/internal (master); regenerated to mode2/5 and sign/mldsa/{mldsa44,mldsa65,mldsa87} via go generate. A regression test (TestPublicTrIsIndependentOfPrivate) is added in the same place and propagates to all six internal packages. --- sign/dilithium/mode2/internal/dilithium.go | 16 ++++++++--- .../mode2/internal/dilithium_test.go | 27 +++++++++++++++++++ sign/dilithium/mode3/internal/dilithium.go | 16 ++++++++--- .../mode3/internal/dilithium_test.go | 27 +++++++++++++++++++ sign/dilithium/mode5/internal/dilithium.go | 16 ++++++++--- .../mode5/internal/dilithium_test.go | 27 +++++++++++++++++++ sign/mldsa/mldsa44/internal/dilithium.go | 16 ++++++++--- sign/mldsa/mldsa44/internal/dilithium_test.go | 27 +++++++++++++++++++ sign/mldsa/mldsa65/internal/dilithium.go | 16 ++++++++--- sign/mldsa/mldsa65/internal/dilithium_test.go | 27 +++++++++++++++++++ sign/mldsa/mldsa87/internal/dilithium.go | 16 ++++++++--- sign/mldsa/mldsa87/internal/dilithium_test.go | 27 +++++++++++++++++++ 12 files changed, 240 insertions(+), 18 deletions(-) diff --git a/sign/dilithium/mode2/internal/dilithium.go b/sign/dilithium/mode2/internal/dilithium.go index 6d4b4d16e..0cd0d9a49 100644 --- a/sign/dilithium/mode2/internal/dilithium.go +++ b/sign/dilithium/mode2/internal/dilithium.go @@ -236,8 +236,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -472,13 +477,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode2/internal/dilithium_test.go b/sign/dilithium/mode2/internal/dilithium_test.go index 47f53d3a0..d97ca3370 100644 --- a/sign/dilithium/mode2/internal/dilithium_test.go +++ b/sign/dilithium/mode2/internal/dilithium_test.go @@ -140,6 +140,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/dilithium/mode3/internal/dilithium.go b/sign/dilithium/mode3/internal/dilithium.go index 9d0a1f196..8625e0464 100644 --- a/sign/dilithium/mode3/internal/dilithium.go +++ b/sign/dilithium/mode3/internal/dilithium.go @@ -234,8 +234,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -470,13 +475,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode3/internal/dilithium_test.go b/sign/dilithium/mode3/internal/dilithium_test.go index a3f1126dd..c9b47aa84 100644 --- a/sign/dilithium/mode3/internal/dilithium_test.go +++ b/sign/dilithium/mode3/internal/dilithium_test.go @@ -138,6 +138,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/dilithium/mode5/internal/dilithium.go b/sign/dilithium/mode5/internal/dilithium.go index 6d4b4d16e..0cd0d9a49 100644 --- a/sign/dilithium/mode5/internal/dilithium.go +++ b/sign/dilithium/mode5/internal/dilithium.go @@ -236,8 +236,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -472,13 +477,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode5/internal/dilithium_test.go b/sign/dilithium/mode5/internal/dilithium_test.go index 47f53d3a0..d97ca3370 100644 --- a/sign/dilithium/mode5/internal/dilithium_test.go +++ b/sign/dilithium/mode5/internal/dilithium_test.go @@ -140,6 +140,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa44/internal/dilithium.go b/sign/mldsa/mldsa44/internal/dilithium.go index 6d4b4d16e..0cd0d9a49 100644 --- a/sign/mldsa/mldsa44/internal/dilithium.go +++ b/sign/mldsa/mldsa44/internal/dilithium.go @@ -236,8 +236,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -472,13 +477,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa44/internal/dilithium_test.go b/sign/mldsa/mldsa44/internal/dilithium_test.go index 47f53d3a0..d97ca3370 100644 --- a/sign/mldsa/mldsa44/internal/dilithium_test.go +++ b/sign/mldsa/mldsa44/internal/dilithium_test.go @@ -140,6 +140,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa65/internal/dilithium.go b/sign/mldsa/mldsa65/internal/dilithium.go index 6d4b4d16e..0cd0d9a49 100644 --- a/sign/mldsa/mldsa65/internal/dilithium.go +++ b/sign/mldsa/mldsa65/internal/dilithium.go @@ -236,8 +236,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -472,13 +477,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa65/internal/dilithium_test.go b/sign/mldsa/mldsa65/internal/dilithium_test.go index 47f53d3a0..d97ca3370 100644 --- a/sign/mldsa/mldsa65/internal/dilithium_test.go +++ b/sign/mldsa/mldsa65/internal/dilithium_test.go @@ -140,6 +140,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa87/internal/dilithium.go b/sign/mldsa/mldsa87/internal/dilithium.go index 6d4b4d16e..0cd0d9a49 100644 --- a/sign/mldsa/mldsa87/internal/dilithium.go +++ b/sign/mldsa/mldsa87/internal/dilithium.go @@ -236,8 +236,13 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key - pk.tr = &sk.tr + // Finish cache of public key. Copy the value rather than aliasing + // sk.tr so that later mutation of sk (for instance through a + // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) + // does not silently propagate into a pk returned earlier from + // NewKeyFromSeed. See cloudflare/circl#600. + pk.tr = new([TRSize]byte) + *pk.tr = sk.tr return &pk, &sk } @@ -472,13 +477,18 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. +// +// The returned pk holds its own copy of tr; later mutation of sk.tr +// does not affect a pk previously returned from this method. +// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: &sk.tr, + tr: new([TRSize]byte), } + *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa87/internal/dilithium_test.go b/sign/mldsa/mldsa87/internal/dilithium_test.go index 47f53d3a0..d97ca3370 100644 --- a/sign/mldsa/mldsa87/internal/dilithium_test.go +++ b/sign/mldsa/mldsa87/internal/dilithium_test.go @@ -140,6 +140,33 @@ func TestPublicFromPrivate(t *testing.T) { } } +// Regression test for cloudflare/circl#600: the pk returned by +// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, +// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for +// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- +// silently propagates into a pk that was returned earlier. +func TestPublicTrIsIndependentOfPrivate(t *testing.T) { + var seed [common.SeedSize]byte + pkSeed, sk := NewKeyFromSeed(&seed) + pkPub := sk.Public() + + pkSeedTr := *pkSeed.tr + pkPubTr := *pkPub.tr + + for i := range sk.tr { + sk.tr[i] ^= 0xff + } + + if *pkSeed.tr != pkSeedTr { + t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", + *pkSeed.tr, pkSeedTr) + } + if *pkPub.tr != pkPubTr { + t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", + *pkPub.tr, pkPubTr) + } +} + func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { From 547aa4275fa4494e04072eb357ac1f2a13304e16 Mon Sep 17 00:00:00 2001 From: Bas Westerbaan Date: Wed, 6 May 2026 12:23:46 +0200 Subject: [PATCH 2/2] sign/dilithium: don't use tr pointer --- sign/dilithium/mode2/internal/dilithium.go | 19 +++---------- .../mode2/internal/dilithium_test.go | 27 ------------------- sign/dilithium/mode3/internal/dilithium.go | 19 +++---------- .../mode3/internal/dilithium_test.go | 27 ------------------- sign/dilithium/mode5/internal/dilithium.go | 19 +++---------- .../mode5/internal/dilithium_test.go | 27 ------------------- sign/mldsa/mldsa44/internal/dilithium.go | 19 +++---------- sign/mldsa/mldsa44/internal/dilithium_test.go | 27 ------------------- sign/mldsa/mldsa65/internal/dilithium.go | 19 +++---------- sign/mldsa/mldsa65/internal/dilithium_test.go | 27 ------------------- sign/mldsa/mldsa87/internal/dilithium.go | 19 +++---------- sign/mldsa/mldsa87/internal/dilithium_test.go | 27 ------------------- 12 files changed, 24 insertions(+), 252 deletions(-) diff --git a/sign/dilithium/mode2/internal/dilithium.go b/sign/dilithium/mode2/internal/dilithium.go index 0cd0d9a49..f4bf00bc3 100644 --- a/sign/dilithium/mode2/internal/dilithium.go +++ b/sign/dilithium/mode2/internal/dilithium.go @@ -49,7 +49,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -121,7 +121,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -236,13 +235,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -477,18 +471,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode2/internal/dilithium_test.go b/sign/dilithium/mode2/internal/dilithium_test.go index d97ca3370..47f53d3a0 100644 --- a/sign/dilithium/mode2/internal/dilithium_test.go +++ b/sign/dilithium/mode2/internal/dilithium_test.go @@ -140,33 +140,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/dilithium/mode3/internal/dilithium.go b/sign/dilithium/mode3/internal/dilithium.go index 8625e0464..08512c210 100644 --- a/sign/dilithium/mode3/internal/dilithium.go +++ b/sign/dilithium/mode3/internal/dilithium.go @@ -47,7 +47,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -119,7 +119,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -234,13 +233,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -475,18 +469,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode3/internal/dilithium_test.go b/sign/dilithium/mode3/internal/dilithium_test.go index c9b47aa84..a3f1126dd 100644 --- a/sign/dilithium/mode3/internal/dilithium_test.go +++ b/sign/dilithium/mode3/internal/dilithium_test.go @@ -138,33 +138,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/dilithium/mode5/internal/dilithium.go b/sign/dilithium/mode5/internal/dilithium.go index 0cd0d9a49..f4bf00bc3 100644 --- a/sign/dilithium/mode5/internal/dilithium.go +++ b/sign/dilithium/mode5/internal/dilithium.go @@ -49,7 +49,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -121,7 +121,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -236,13 +235,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -477,18 +471,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/dilithium/mode5/internal/dilithium_test.go b/sign/dilithium/mode5/internal/dilithium_test.go index d97ca3370..47f53d3a0 100644 --- a/sign/dilithium/mode5/internal/dilithium_test.go +++ b/sign/dilithium/mode5/internal/dilithium_test.go @@ -140,33 +140,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa44/internal/dilithium.go b/sign/mldsa/mldsa44/internal/dilithium.go index 0cd0d9a49..f4bf00bc3 100644 --- a/sign/mldsa/mldsa44/internal/dilithium.go +++ b/sign/mldsa/mldsa44/internal/dilithium.go @@ -49,7 +49,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -121,7 +121,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -236,13 +235,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -477,18 +471,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa44/internal/dilithium_test.go b/sign/mldsa/mldsa44/internal/dilithium_test.go index d97ca3370..47f53d3a0 100644 --- a/sign/mldsa/mldsa44/internal/dilithium_test.go +++ b/sign/mldsa/mldsa44/internal/dilithium_test.go @@ -140,33 +140,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa65/internal/dilithium.go b/sign/mldsa/mldsa65/internal/dilithium.go index 0cd0d9a49..f4bf00bc3 100644 --- a/sign/mldsa/mldsa65/internal/dilithium.go +++ b/sign/mldsa/mldsa65/internal/dilithium.go @@ -49,7 +49,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -121,7 +121,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -236,13 +235,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -477,18 +471,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa65/internal/dilithium_test.go b/sign/mldsa/mldsa65/internal/dilithium_test.go index d97ca3370..47f53d3a0 100644 --- a/sign/mldsa/mldsa65/internal/dilithium_test.go +++ b/sign/mldsa/mldsa65/internal/dilithium_test.go @@ -140,33 +140,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits { diff --git a/sign/mldsa/mldsa87/internal/dilithium.go b/sign/mldsa/mldsa87/internal/dilithium.go index 0cd0d9a49..f4bf00bc3 100644 --- a/sign/mldsa/mldsa87/internal/dilithium.go +++ b/sign/mldsa/mldsa87/internal/dilithium.go @@ -49,7 +49,7 @@ type PublicKey struct { // Cached values t1p [common.PolyT1Size * K]byte A *Mat - tr *[TRSize]byte + tr [TRSize]byte } // PrivateKey is the type of Dilithium private keys. @@ -121,7 +121,6 @@ func (pk *PublicKey) Unpack(buf *[PublicKeySize]byte) { pk.A.Derive(&pk.rho) // tr = CRH(ρ ‖ t1) = CRH(pk) - pk.tr = new([TRSize]byte) h := sha3.NewShake256() _, _ = h.Write(buf[:]) _, _ = h.Read(pk.tr[:]) @@ -236,13 +235,8 @@ func NewKeyFromSeed(seed *[common.SeedSize]byte) (*PublicKey, *PrivateKey) { _, _ = h.Write(packedPk[:]) _, _ = h.Read(sk.tr[:]) - // Finish cache of public key. Copy the value rather than aliasing - // sk.tr so that later mutation of sk (for instance through a - // subsequent (*PrivateKey).Unpack on attacker-controlled bytes) - // does not silently propagate into a pk returned earlier from - // NewKeyFromSeed. See cloudflare/circl#600. - pk.tr = new([TRSize]byte) - *pk.tr = sk.tr + // Finish cache of public key + pk.tr = sk.tr return &pk, &sk } @@ -477,18 +471,13 @@ func SignTo(sk *PrivateKey, msg func(io.Writer), rnd [32]byte, signature []byte) } // Computes the public key corresponding to this private key. -// -// The returned pk holds its own copy of tr; later mutation of sk.tr -// does not affect a pk previously returned from this method. -// See cloudflare/circl#600. func (sk *PrivateKey) Public() *PublicKey { var t0 VecK pk := &PublicKey{ rho: sk.rho, A: &sk.A, - tr: new([TRSize]byte), + tr: sk.tr, } - *pk.tr = sk.tr sk.computeT0andT1(&t0, &pk.t1) pk.t1.PackT1(pk.t1p[:]) return pk diff --git a/sign/mldsa/mldsa87/internal/dilithium_test.go b/sign/mldsa/mldsa87/internal/dilithium_test.go index d97ca3370..47f53d3a0 100644 --- a/sign/mldsa/mldsa87/internal/dilithium_test.go +++ b/sign/mldsa/mldsa87/internal/dilithium_test.go @@ -140,33 +140,6 @@ func TestPublicFromPrivate(t *testing.T) { } } -// Regression test for cloudflare/circl#600: the pk returned by -// NewKeyFromSeed and (*PrivateKey).Public must hold its own copy of tr, -// not a pointer aliasing sk.tr. Otherwise a later mutation of sk -- for -// instance through (*PrivateKey).Unpack on attacker-controlled bytes -- -// silently propagates into a pk that was returned earlier. -func TestPublicTrIsIndependentOfPrivate(t *testing.T) { - var seed [common.SeedSize]byte - pkSeed, sk := NewKeyFromSeed(&seed) - pkPub := sk.Public() - - pkSeedTr := *pkSeed.tr - pkPubTr := *pkPub.tr - - for i := range sk.tr { - sk.tr[i] ^= 0xff - } - - if *pkSeed.tr != pkSeedTr { - t.Fatalf("pk from NewKeyFromSeed aliased sk.tr (got %x want %x)", - *pkSeed.tr, pkSeedTr) - } - if *pkPub.tr != pkPubTr { - t.Fatalf("pk from sk.Public() aliased sk.tr (got %x want %x)", - *pkPub.tr, pkPubTr) - } -} - func TestGamma1Size(t *testing.T) { var expected int switch Gamma1Bits {