From 1d94cfb268da36cf5fe3d5222cc515c4c11b4a4e Mon Sep 17 00:00:00 2001 From: zugzwang Date: Thu, 7 Nov 2019 18:15:50 -0300 Subject: [PATCH 1/9] Fix ecdh 25519 private key masking --- go.mod | 3 +++ go.sum | 2 ++ openpgp/ecdh/x25519.go | 13 ++++++------- openpgp/packet/private_key.go | 15 ++++----------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index c4af3ec4..667134c8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,9 @@ module golang.org/x/crypto require ( + github.com/davecgh/go-spew v1.1.1 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 golang.org/x/sys v0.0.0-20190412213103-97732733099d ) + +go 1.13 diff --git a/go.sum b/go.sum index 44da4982..6ea8d4f7 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= diff --git a/openpgp/ecdh/x25519.go b/openpgp/ecdh/x25519.go index a7df5004..93561795 100644 --- a/openpgp/ecdh/x25519.go +++ b/openpgp/ecdh/x25519.go @@ -16,7 +16,7 @@ import ( "golang.org/x/crypto/openpgp/internal/ecc" ) -func X25519GenerateParams(rand io.Reader) (priv [32]byte, x [32]byte, err error) { +func X25519GenerateParams(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { var n, helper = new (big.Int), new (big.Int) n.SetUint64(1) n.Lsh(n, 252) @@ -28,16 +28,15 @@ func X25519GenerateParams(rand io.Reader) (priv [32]byte, x [32]byte, err error) if err != nil { return } - // This is because, in tests, rand will return all zeros and we don't - // want to get the point at infinity and loop forever. - priv[1] ^= 0x42 - // If the scalar is out of range, sample another random number. if new(big.Int).SetBytes(priv[:]).Cmp(n) >= 0 { continue } + priv[0] &= 248 + priv[31] &= 127 + priv[31] |= 64 - curve25519.ScalarBaseMult(&x, &priv) + curve25519.ScalarBaseMult(&pub, &priv) return } return @@ -141,4 +140,4 @@ func copyReversed(out []byte, in []byte) { for i := 0; i < l; i++ { out[i] = in[l-i-1] } -} \ No newline at end of file +} diff --git a/openpgp/packet/private_key.go b/openpgp/packet/private_key.go index 12206017..4d53b8af 100644 --- a/openpgp/packet/private_key.go +++ b/openpgp/packet/private_key.go @@ -225,21 +225,14 @@ func (pk *PrivateKey) Serialize(w io.Writer) (err error) { return } +// TODO: Write a description // SerializeUnEncrypted .. func (pk *PrivateKey) SerializeUnEncrypted(w io.Writer) (err error) { buf := bytes.NewBuffer(nil) buf.Write([]byte{uint8(S2KNON)} /* no encryption */) - switch priv := pk.PrivateKey.(type) { - case *rsa.PrivateKey: - err = serializeRSAPrivateKey(buf, priv) - case *dsa.PrivateKey: - err = serializeDSAPrivateKey(buf, priv) - case *elgamal.PrivateKey: - err = serializeElGamalPrivateKey(buf, priv) - case *ecdsa.PrivateKey: - err = serializeECDSAPrivateKey(buf, priv) - default: - err = errors.InvalidArgumentError("unknown private key type") + err = pk.serializePrivateKey(buf) + if err != nil { + return err } privateKeyBytes := buf.Bytes() if pk.sha1Checksum { From 2e04507d9298ff1f831106eccd7be3e427c00d60 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Thu, 7 Nov 2019 18:19:06 -0300 Subject: [PATCH 2/9] Remove debugging package --- go.mod | 1 - go.sum | 2 -- openpgp/ecdh/x25519.go | 6 +++--- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 667134c8..bfb61328 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,6 @@ module golang.org/x/crypto require ( - github.com/davecgh/go-spew v1.1.1 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 golang.org/x/sys v0.0.0-20190412213103-97732733099d ) diff --git a/go.sum b/go.sum index 6ea8d4f7..44da4982 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= diff --git a/openpgp/ecdh/x25519.go b/openpgp/ecdh/x25519.go index 93561795..368e236a 100644 --- a/openpgp/ecdh/x25519.go +++ b/openpgp/ecdh/x25519.go @@ -28,13 +28,13 @@ func X25519GenerateParams(rand io.Reader) (priv [32]byte, pub [32]byte, err erro if err != nil { return } + priv[0] &= 248 + priv[31] &= 127 + priv[31] |= 64 // If the scalar is out of range, sample another random number. if new(big.Int).SetBytes(priv[:]).Cmp(n) >= 0 { continue } - priv[0] &= 248 - priv[31] &= 127 - priv[31] |= 64 curve25519.ScalarBaseMult(&pub, &priv) return From deb143523370df07b96b9f059ba96c7d442515af Mon Sep 17 00:00:00 2001 From: zugzwang Date: Thu, 7 Nov 2019 18:25:35 -0300 Subject: [PATCH 3/9] Remove go version requirement --- go.mod | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.mod b/go.mod index bfb61328..c4af3ec4 100644 --- a/go.mod +++ b/go.mod @@ -4,5 +4,3 @@ require ( golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 golang.org/x/sys v0.0.0-20190412213103-97732733099d ) - -go 1.13 From 9591e2555794f0fa8e602823f6748649e7ea3d3c Mon Sep 17 00:00:00 2001 From: zugzwang Date: Thu, 7 Nov 2019 19:16:37 -0300 Subject: [PATCH 4/9] Add go version req --- go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.mod b/go.mod index c4af3ec4..bfb61328 100644 --- a/go.mod +++ b/go.mod @@ -4,3 +4,5 @@ require ( golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 golang.org/x/sys v0.0.0-20190412213103-97732733099d ) + +go 1.13 From 11f01691f31d978b3374002618db8a7e58912781 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Fri, 8 Nov 2019 16:21:11 -0300 Subject: [PATCH 5/9] Created x25519_test.go and WIP masking test --- openpgp/ecdh/ecdh_test.go | 1 + openpgp/ecdh/x25519.go | 25 +++++++++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/openpgp/ecdh/ecdh_test.go b/openpgp/ecdh/ecdh_test.go index 1e7d148a..83ef551b 100644 --- a/openpgp/ecdh/ecdh_test.go +++ b/openpgp/ecdh/ecdh_test.go @@ -20,6 +20,7 @@ var ( ) +// TODO: Improve this. func TestEncryptDecrypt(t *testing.T) { kdf := KDF{ Hash: algorithm.SHA512, diff --git a/openpgp/ecdh/x25519.go b/openpgp/ecdh/x25519.go index 368e236a..a6dd558d 100644 --- a/openpgp/ecdh/x25519.go +++ b/openpgp/ecdh/x25519.go @@ -16,7 +16,12 @@ import ( "golang.org/x/crypto/openpgp/internal/ecc" ) -func X25519GenerateParams(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { +// Generates a private-public key-pair. +// 'priv' is a private key; a scalar belonging to the set +// 2^{254} + 8 * [0, 2^{251}), in order to avoid the small subgroup of the +// curve. 'pub' is simply 'priv' * G where G is the base point. +// See https://cr.yp.to/ecdh.html and RFC7748, sec 5. +func generateKeyPair(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { var n, helper = new (big.Int), new (big.Int) n.SetUint64(1) n.Lsh(n, 252) @@ -28,9 +33,12 @@ func X25519GenerateParams(rand io.Reader) (priv [32]byte, pub [32]byte, err erro if err != nil { return } + // The following ensures that the private key is a number of the form + // 2^{254} + 8 * [0, 2^{251}), in order to avoid the small subgroup of priv[0] &= 248 priv[31] &= 127 priv[31] |= 64 + // If the scalar is out of range, sample another random number. if new(big.Int).SetBytes(priv[:]).Cmp(n) >= 0 { continue @@ -42,11 +50,14 @@ func X25519GenerateParams(rand io.Reader) (priv [32]byte, pub [32]byte, err erro return } +// X25519GenerateKey samples the key pair according to the correct distribution. +// It also sets the given key-derivation function and returns the *PrivateKey +// object along with an error. func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { ci := ecc.FindByName("Curve25519") priv = new(PrivateKey) priv.PublicKey.Curve = ci.Curve - d, pubKey, err := X25519GenerateParams(rand) + d, pubKey, err := generateKeyPair(rand) if err != nil { return nil, err } @@ -56,9 +67,11 @@ func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { priv.PublicKey.CurveType = ci.CurveType priv.PublicKey.Curve = ci.Curve /* - * Note that ECPoint.point differs from the definition of public keys in [Curve25519] in two ways: - * (1) the byte-ordering is big-endian, which is more uniform with how big integers are represented in TLS, and - * (2) there is an additional length byte (so ECpoint.point is actually 33 bytes), again for uniformity (and extensibility). + * Note that ECPoint.point differs from the definition of public keys in + * [Curve25519] in two ways: (1) the byte-ordering is big-endian, which is + * more uniform with how big integers are represented in TLS, and (2) there + * is an additional length byte (so ECpoint.point is actually 33 bytes), + * again for uniformity (and extensibility). */ var encodedKey = make([]byte, 33) encodedKey[0] = 0x40 @@ -69,7 +82,7 @@ func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { } func X25519Encrypt(random io.Reader, pub *PublicKey, msg, curveOID, fingerprint []byte) (vsG, c []byte, err error) { - d, ephemeralKey, err := X25519GenerateParams(random) + d, ephemeralKey, err := generateKeyPair(random) if err != nil { return nil, nil, err } From 7f2ff46a2db0d1fe00fe61aab7025318e5c2bf95 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Fri, 8 Nov 2019 16:21:48 -0300 Subject: [PATCH 6/9] Created x25519_test.go and WIP masking test --- openpgp/ecdh/x25519_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 openpgp/ecdh/x25519_test.go diff --git a/openpgp/ecdh/x25519_test.go b/openpgp/ecdh/x25519_test.go new file mode 100644 index 00000000..18995b35 --- /dev/null +++ b/openpgp/ecdh/x25519_test.go @@ -0,0 +1,31 @@ +// Copyright 2019 ProtonTech AG. +// + +package ecdh + +import ( + "testing" + "crypto/rand" + "fmt" +) +// Some implementations, such as gpg 2.2.12, do not accept ECDH private keys +// if they're not masked. This is because they're not of the proper form, +// cryptographically, and they don't mask input keys during crypto operations. +// This test checks if the keys that this library stores or outputs are properly +// masked. +func TestGenerateMaskedPrivateKeyX25519(t *testing.T) { + priv, pub, err := generateKeyPair(rand.Reader) + if err != nil { + t.Fatal(err) + } + // Check masking + // 3 lsb + fmt.Println(pub) + fmt.Println(priv) + fmt.Println(priv[0]) + + for _, n := range(priv) { + fmt.Printf("% 08b", n) // prints 00000000 11111101 + } + print("\n") +} From 3c87cac7c4b8c3bb9bcfd6d4a2cc060d97ce8552 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Fri, 8 Nov 2019 17:35:38 -0300 Subject: [PATCH 7/9] ecdh: Go fmt --- openpgp/ecdh/x25519.go | 15 ++++++++------- openpgp/ecdh/x25519_test.go | 34 +++++++++++++++++++--------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/openpgp/ecdh/x25519.go b/openpgp/ecdh/x25519.go index a6dd558d..49085963 100644 --- a/openpgp/ecdh/x25519.go +++ b/openpgp/ecdh/x25519.go @@ -11,8 +11,8 @@ import ( "io" "math/big" - "golang.org/x/crypto/openpgp/aes/keywrap" "golang.org/x/crypto/curve25519" + "golang.org/x/crypto/openpgp/aes/keywrap" "golang.org/x/crypto/openpgp/internal/ecc" ) @@ -22,7 +22,7 @@ import ( // curve. 'pub' is simply 'priv' * G where G is the base point. // See https://cr.yp.to/ecdh.html and RFC7748, sec 5. func generateKeyPair(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { - var n, helper = new (big.Int), new (big.Int) + var n, helper = new(big.Int), new(big.Int) n.SetUint64(1) n.Lsh(n, 252) helper.SetString("27742317777372353535851937790883648493", 10) @@ -35,6 +35,7 @@ func generateKeyPair(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { } // The following ensures that the private key is a number of the form // 2^{254} + 8 * [0, 2^{251}), in order to avoid the small subgroup of + // of the curve. priv[0] &= 248 priv[31] &= 127 priv[31] |= 64 @@ -76,8 +77,8 @@ func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { var encodedKey = make([]byte, 33) encodedKey[0] = 0x40 copy(encodedKey[1:], pubKey[:]) - priv.PublicKey.X = new (big.Int).SetBytes(encodedKey[:]) - priv.PublicKey.Y = new (big.Int) + priv.PublicKey.X = new(big.Int).SetBytes(encodedKey[:]) + priv.PublicKey.Y = new(big.Int) return priv, nil } @@ -88,7 +89,7 @@ func X25519Encrypt(random io.Reader, pub *PublicKey, msg, curveOID, fingerprint } var pubKey [32]byte - if pub.X.BitLen() > 33 * 264 { + if pub.X.BitLen() > 33*264 { return nil, nil, errors.New("ecdh: invalid key") } copy(pubKey[:], pub.X.Bytes()[1:]) @@ -114,9 +115,9 @@ func X25519Encrypt(random io.Reader, pub *PublicKey, msg, curveOID, fingerprint } func X25519Decrypt(priv *PrivateKey, vsG, m, curveOID, fingerprint []byte) (msg []byte, err error) { - var zb, d, ephemeralKey[32]byte + var zb, d, ephemeralKey [32]byte if len(vsG) != 33 || vsG[0] != 0x40 { - return nil, errors.New("ecdh: invalid key") + return nil, errors.New("ecdh: invalid key") } copy(ephemeralKey[:], vsG[1:33]) diff --git a/openpgp/ecdh/x25519_test.go b/openpgp/ecdh/x25519_test.go index 18995b35..bd0b29bd 100644 --- a/openpgp/ecdh/x25519_test.go +++ b/openpgp/ecdh/x25519_test.go @@ -6,26 +6,30 @@ package ecdh import ( "testing" "crypto/rand" - "fmt" ) -// Some implementations, such as gpg 2.2.12, do not accept ECDH private keys -// if they're not masked. This is because they're not of the proper form, + +// Some OpenPGP implementations, such as gpg 2.2.12, do not accept ECDH private +// keys if they're not masked. This is because they're not of the proper form, // cryptographically, and they don't mask input keys during crypto operations. -// This test checks if the keys that this library stores or outputs are properly -// masked. +// This test checks if the keys that this library stores or outputs are +// properly masked. func TestGenerateMaskedPrivateKeyX25519(t *testing.T) { - priv, pub, err := generateKeyPair(rand.Reader) + priv, _, err := generateKeyPair(rand.Reader) if err != nil { t.Fatal(err) } - // Check masking - // 3 lsb - fmt.Println(pub) - fmt.Println(priv) - fmt.Println(priv[0]) - for _, n := range(priv) { - fmt.Printf("% 08b", n) // prints 00000000 11111101 - } - print("\n") + // Check masking + // 3 lsb are 0 + if priv[0]<<5 != 0 { + t.Fatalf("Priv. key is not masked (3 lsb should be unset): %X", priv) + } + // MSB is 0 + if priv[31]>>7 != 0 { + t.Fatalf("Priv. key is not masked (MSB should be unset): %X", priv) + } + // Second-MSB is 1 + if priv[31]>>6 != 1 { + t.Fatalf("Priv. key is not masked (second MSB should be set): %X", priv) + } } From f8b4c818a5afb717e10f575aa49beb88de521a63 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Wed, 20 Nov 2019 15:20:23 -0300 Subject: [PATCH 8/9] Rename and unexport key generation function --- openpgp/ecdh/x25519.go | 6 +++--- openpgp/ecdh/x25519_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openpgp/ecdh/x25519.go b/openpgp/ecdh/x25519.go index 49085963..3db0b2be 100644 --- a/openpgp/ecdh/x25519.go +++ b/openpgp/ecdh/x25519.go @@ -21,7 +21,7 @@ import ( // 2^{254} + 8 * [0, 2^{251}), in order to avoid the small subgroup of the // curve. 'pub' is simply 'priv' * G where G is the base point. // See https://cr.yp.to/ecdh.html and RFC7748, sec 5. -func generateKeyPair(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { +func x25519GenerateKeyPairBytes(rand io.Reader) (priv [32]byte, pub [32]byte, err error) { var n, helper = new(big.Int), new(big.Int) n.SetUint64(1) n.Lsh(n, 252) @@ -58,7 +58,7 @@ func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { ci := ecc.FindByName("Curve25519") priv = new(PrivateKey) priv.PublicKey.Curve = ci.Curve - d, pubKey, err := generateKeyPair(rand) + d, pubKey, err := x25519GenerateKeyPairBytes(rand) if err != nil { return nil, err } @@ -83,7 +83,7 @@ func X25519GenerateKey(rand io.Reader, kdf KDF) (priv *PrivateKey, err error) { } func X25519Encrypt(random io.Reader, pub *PublicKey, msg, curveOID, fingerprint []byte) (vsG, c []byte, err error) { - d, ephemeralKey, err := generateKeyPair(random) + d, ephemeralKey, err := x25519GenerateKeyPairBytes(random) if err != nil { return nil, nil, err } diff --git a/openpgp/ecdh/x25519_test.go b/openpgp/ecdh/x25519_test.go index bd0b29bd..320a43a9 100644 --- a/openpgp/ecdh/x25519_test.go +++ b/openpgp/ecdh/x25519_test.go @@ -14,7 +14,7 @@ import ( // This test checks if the keys that this library stores or outputs are // properly masked. func TestGenerateMaskedPrivateKeyX25519(t *testing.T) { - priv, _, err := generateKeyPair(rand.Reader) + priv, _, err := generateKeyPairBytes(rand.Reader) if err != nil { t.Fatal(err) } From 94ddf2d961cc683fcfef746f64b24265ae9b8b37 Mon Sep 17 00:00:00 2001 From: zugzwang Date: Wed, 20 Nov 2019 15:27:52 -0300 Subject: [PATCH 9/9] Solve testing --- openpgp/ecdh/x25519_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openpgp/ecdh/x25519_test.go b/openpgp/ecdh/x25519_test.go index 320a43a9..de55ce8a 100644 --- a/openpgp/ecdh/x25519_test.go +++ b/openpgp/ecdh/x25519_test.go @@ -14,7 +14,7 @@ import ( // This test checks if the keys that this library stores or outputs are // properly masked. func TestGenerateMaskedPrivateKeyX25519(t *testing.T) { - priv, _, err := generateKeyPairBytes(rand.Reader) + priv, _, err := x25519GenerateKeyPairBytes(rand.Reader) if err != nil { t.Fatal(err) }