From 49af35bf5e8476a717557fcd296a7d8a5e379a74 Mon Sep 17 00:00:00 2001 From: Tom Thorogood Date: Mon, 6 Mar 2023 21:14:29 +1030 Subject: [PATCH 1/2] tkn20,kyber,x25519,x448: plug constant-time leaks In particular leaking z in kyber could be quite damaging: https://groups.google.com/a/list.nist.gov/g/pqc-forum/c/SJ31w0QSmIM/m/XgdBgh3wAwAJ The changes to x25519 and x448 are unlikely to be needed, but it's more idiomatic at least. --- abe/cpabe/tkn20/internal/tkn/tk.go | 5 +++-- dh/x25519/key.go | 6 +++--- dh/x448/key.go | 6 +++--- kem/kyber/kyber1024/kyber.go | 2 +- kem/kyber/kyber512/kyber.go | 2 +- kem/kyber/kyber768/kyber.go | 2 +- kem/kyber/templates/pkg.templ.go | 2 +- pke/kyber/kyber1024/internal/cpapke.go | 4 +++- pke/kyber/kyber512/internal/cpapke.go | 4 +++- pke/kyber/kyber768/internal/cpapke.go | 4 +++- 10 files changed, 22 insertions(+), 15 deletions(-) diff --git a/abe/cpabe/tkn20/internal/tkn/tk.go b/abe/cpabe/tkn20/internal/tkn/tk.go index 606c80f5..60df30ad 100644 --- a/abe/cpabe/tkn20/internal/tkn/tk.go +++ b/abe/cpabe/tkn20/internal/tkn/tk.go @@ -3,7 +3,7 @@ package tkn import ( - "bytes" + "crypto/subtle" "encoding/binary" "fmt" "io" @@ -181,7 +181,8 @@ func (s *SecretParams) UnmarshalBinary(data []byte) error { func (s *SecretParams) Equal(s2 *SecretParams) bool { return s.a.Equal(s2.a) && s.wtA.Equal(s2.wtA) && s.bstar.Equal(s2.bstar) && - s.bstar12.Equal(s2.bstar12) && s.k.Equal(s2.k) && bytes.Equal(s.prfKey, s2.prfKey) + s.bstar12.Equal(s2.bstar12) && s.k.Equal(s2.k) && + subtle.ConstantTimeCompare(s.prfKey, s2.prfKey) == 1 } type AttributesKey struct { diff --git a/dh/x25519/key.go b/dh/x25519/key.go index bbd37ddd..c76f72ac 100644 --- a/dh/x25519/key.go +++ b/dh/x25519/key.go @@ -22,11 +22,11 @@ func (k *Key) clamp(in *Key) *Key { // isValidPubKey verifies if the public key is not a low-order point. func (k *Key) isValidPubKey() bool { fp.Modp((*fp.Elt)(k)) - isLowOrder := false + var isLowOrder int for _, P := range lowOrderPoints { - isLowOrder = isLowOrder || subtle.ConstantTimeCompare(P[:], k[:]) != 0 + isLowOrder |= subtle.ConstantTimeCompare(P[:], k[:]) } - return !isLowOrder + return isLowOrder == 0 } // KeyGen obtains a public key given a secret key. diff --git a/dh/x448/key.go b/dh/x448/key.go index 82a07a2b..2fdde511 100644 --- a/dh/x448/key.go +++ b/dh/x448/key.go @@ -22,11 +22,11 @@ func (k *Key) clamp(in *Key) *Key { // isValidPubKey verifies if the public key is not a low-order point. func (k *Key) isValidPubKey() bool { fp.Modp((*fp.Elt)(k)) - isLowOrder := false + var isLowOrder int for _, P := range lowOrderPoints { - isLowOrder = isLowOrder || subtle.ConstantTimeCompare(P[:], k[:]) != 0 + isLowOrder |= subtle.ConstantTimeCompare(P[:], k[:]) } - return !isLowOrder + return isLowOrder == 0 } // KeyGen obtains a public key given a secret key. diff --git a/kem/kyber/kyber1024/kyber.go b/kem/kyber/kyber1024/kyber.go index 223c842e..082c0e64 100644 --- a/kem/kyber/kyber1024/kyber.go +++ b/kem/kyber/kyber1024/kyber.go @@ -296,7 +296,7 @@ func (sk *PrivateKey) Equal(other kem.PrivateKey) bool { return false } if !bytes.Equal(sk.hpk[:], oth.hpk[:]) || - !bytes.Equal(sk.z[:], oth.z[:]) { + subtle.ConstantTimeCompare(sk.z[:], oth.z[:]) != 1 { return false } return sk.sk.Equal(oth.sk) diff --git a/kem/kyber/kyber512/kyber.go b/kem/kyber/kyber512/kyber.go index 8cc1ec76..2e5b9fd0 100644 --- a/kem/kyber/kyber512/kyber.go +++ b/kem/kyber/kyber512/kyber.go @@ -296,7 +296,7 @@ func (sk *PrivateKey) Equal(other kem.PrivateKey) bool { return false } if !bytes.Equal(sk.hpk[:], oth.hpk[:]) || - !bytes.Equal(sk.z[:], oth.z[:]) { + subtle.ConstantTimeCompare(sk.z[:], oth.z[:]) != 1 { return false } return sk.sk.Equal(oth.sk) diff --git a/kem/kyber/kyber768/kyber.go b/kem/kyber/kyber768/kyber.go index 98c40279..e9b025d4 100644 --- a/kem/kyber/kyber768/kyber.go +++ b/kem/kyber/kyber768/kyber.go @@ -296,7 +296,7 @@ func (sk *PrivateKey) Equal(other kem.PrivateKey) bool { return false } if !bytes.Equal(sk.hpk[:], oth.hpk[:]) || - !bytes.Equal(sk.z[:], oth.z[:]) { + subtle.ConstantTimeCompare(sk.z[:], oth.z[:]) != 1 { return false } return sk.sk.Equal(oth.sk) diff --git a/kem/kyber/templates/pkg.templ.go b/kem/kyber/templates/pkg.templ.go index 8fc5b3d9..4e56e0f5 100644 --- a/kem/kyber/templates/pkg.templ.go +++ b/kem/kyber/templates/pkg.templ.go @@ -300,7 +300,7 @@ func (sk *PrivateKey) Equal(other kem.PrivateKey) bool { return false } if !bytes.Equal(sk.hpk[:], oth.hpk[:]) || - !bytes.Equal(sk.z[:], oth.z[:]) { + subtle.ConstantTimeCompare(sk.z[:], oth.z[:]) != 1 { return false } return sk.sk.Equal(oth.sk) diff --git a/pke/kyber/kyber1024/internal/cpapke.go b/pke/kyber/kyber1024/internal/cpapke.go index 01ef88b2..51e677b1 100644 --- a/pke/kyber/kyber1024/internal/cpapke.go +++ b/pke/kyber/kyber1024/internal/cpapke.go @@ -3,6 +3,8 @@ package internal import ( + "crypto/subtle" + "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -172,5 +174,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return ret == 0 + return subtle.ConstantTimeEq(int32(ret), 0) == 1 } diff --git a/pke/kyber/kyber512/internal/cpapke.go b/pke/kyber/kyber512/internal/cpapke.go index 80ab2501..9124add5 100644 --- a/pke/kyber/kyber512/internal/cpapke.go +++ b/pke/kyber/kyber512/internal/cpapke.go @@ -1,6 +1,8 @@ package internal import ( + "crypto/subtle" + "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -170,5 +172,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return ret == 0 + return subtle.ConstantTimeEq(int32(ret), 0) == 1 } diff --git a/pke/kyber/kyber768/internal/cpapke.go b/pke/kyber/kyber768/internal/cpapke.go index 01ef88b2..51e677b1 100644 --- a/pke/kyber/kyber768/internal/cpapke.go +++ b/pke/kyber/kyber768/internal/cpapke.go @@ -3,6 +3,8 @@ package internal import ( + "crypto/subtle" + "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -172,5 +174,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return ret == 0 + return subtle.ConstantTimeEq(int32(ret), 0) == 1 } From a553dc384e15c95e6828d4b729aea4ae6d342437 Mon Sep 17 00:00:00 2001 From: Tom Thorogood Date: Fri, 10 Mar 2023 20:19:38 +1030 Subject: [PATCH 2/2] revert pke/kyber changes --- pke/kyber/kyber1024/internal/cpapke.go | 4 +--- pke/kyber/kyber512/internal/cpapke.go | 4 +--- pke/kyber/kyber768/internal/cpapke.go | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/pke/kyber/kyber1024/internal/cpapke.go b/pke/kyber/kyber1024/internal/cpapke.go index 51e677b1..01ef88b2 100644 --- a/pke/kyber/kyber1024/internal/cpapke.go +++ b/pke/kyber/kyber1024/internal/cpapke.go @@ -3,8 +3,6 @@ package internal import ( - "crypto/subtle" - "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -174,5 +172,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return subtle.ConstantTimeEq(int32(ret), 0) == 1 + return ret == 0 } diff --git a/pke/kyber/kyber512/internal/cpapke.go b/pke/kyber/kyber512/internal/cpapke.go index 9124add5..80ab2501 100644 --- a/pke/kyber/kyber512/internal/cpapke.go +++ b/pke/kyber/kyber512/internal/cpapke.go @@ -1,8 +1,6 @@ package internal import ( - "crypto/subtle" - "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -172,5 +170,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return subtle.ConstantTimeEq(int32(ret), 0) == 1 + return ret == 0 } diff --git a/pke/kyber/kyber768/internal/cpapke.go b/pke/kyber/kyber768/internal/cpapke.go index 51e677b1..01ef88b2 100644 --- a/pke/kyber/kyber768/internal/cpapke.go +++ b/pke/kyber/kyber768/internal/cpapke.go @@ -3,8 +3,6 @@ package internal import ( - "crypto/subtle" - "github.com/cloudflare/circl/internal/sha3" "github.com/cloudflare/circl/pke/kyber/internal/common" ) @@ -174,5 +172,5 @@ func (sk *PrivateKey) Equal(other *PrivateKey) bool { ret |= sk.sh[i][j] ^ other.sh[i][j] } } - return subtle.ConstantTimeEq(int32(ret), 0) == 1 + return ret == 0 }