From 78b0586b585c086a1ba1f1c3de4d00eed3de1b26 Mon Sep 17 00:00:00 2001 From: ekzyis Date: Wed, 24 Apr 2024 22:35:27 +0200 Subject: [PATCH] Make sure privkey is on curve before using unsafe secp256k1.PrivKeyFromBytes --- nip44.go | 34 ++++++++++++++++++------------ nip44_test.go | 57 ++++++++++++++++++++++++--------------------------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/nip44.go b/nip44.go index b16e4d3..8aebe70 100644 --- a/nip44.go +++ b/nip44.go @@ -7,10 +7,12 @@ import ( "crypto/sha256" "encoding/base64" "encoding/binary" + "encoding/hex" "errors" "fmt" "io" "math" + "math/big" "github.com/decred/dcrd/dcrec/secp256k1/v4" "golang.org/x/crypto/chacha20" @@ -134,19 +136,25 @@ func Decrypt(conversationKey []byte, ciphertext string) (string, error) { return string(unpadded), nil } -func GenerateConversationKey(sendPrivkey *secp256k1.PrivateKey, recvPubkey *secp256k1.PublicKey) []byte { - // TODO: Make sure keys are not invalid or weak since the secp256k1 package does not. - // See documentation of secp256k1.PrivKeyFromBytes: - // ================================================================================ - // | WARNING: This means passing a slice with more than 32 bytes is truncated and | - // | that truncated value is reduced modulo N. Further, 0 is not a valid private | - // | key. It is up to the caller to provide a value in the appropriate range of | - // | [1, N-1]. Failure to do so will either result in an invalid private key or | - // | potentially weak private keys that have bias that could be exploited. | - // ================================================================================ - // -- https://pkg.go.dev/github.com/decred/dcrd/dcrec/secp256k1/v4#PrivKeyFromBytes - shared := secp256k1.GenerateSharedSecret(sendPrivkey, recvPubkey) - return hkdf.Extract(sha256.New, shared, []byte("nip44-v2")) +func GenerateConversationKey(sendPrivkey []byte, recvPubkey []byte) ([]byte, error) { + var ( + N = secp256k1.S256().N + sk *secp256k1.PrivateKey + pk *secp256k1.PublicKey + err error + ) + // make sure that private key is on curve before using unsafe secp256k1.PrivKeyFromBytes + // see https://pkg.go.dev/github.com/decred/dcrd/dcrec/secp256k1/v4#PrivKeyFromBytes + skX := new(big.Int).SetBytes(sendPrivkey) + if skX.Cmp(big.NewInt(0)) == 0 || skX.Cmp(N) >= 0 { + return []byte{}, fmt.Errorf("invalid private key: x coordinate %s is not on the secp256k1 curve", hex.EncodeToString(sendPrivkey)) + } + sk = secp256k1.PrivKeyFromBytes(sendPrivkey) + if pk, err = secp256k1.ParsePubKey(recvPubkey); err != nil { + return []byte{}, err + } + shared := secp256k1.GenerateSharedSecret(sk, pk) + return hkdf.Extract(sha256.New, shared, []byte("nip44-v2")), nil } func chacha20_(key []byte, nonce []byte, message []byte) ([]byte, error) { diff --git a/nip44_test.go b/nip44_test.go index 73704b9..9be7993 100644 --- a/nip44_test.go +++ b/nip44_test.go @@ -94,29 +94,25 @@ func assertDecryptFail(t *testing.T, conversationKey string, plaintext string, c } func assertConversationKeyFail(t *testing.T, sk1 string, pub2 string, msg string) { - // TODO: Update GenerateConversationKey since secp256k1 does accept invalid or weak keys - t.Skip("secp256k1 keys are not validated yet during conversation key generation.") var ( - // sendPrivkey *secp256k1.PrivateKey - // recvPubkey *secp256k1.PublicKey - decoded []byte - ok bool - err error + sk1Decoded []byte + pub2Decoded []byte + ok bool + err error ) - decoded, err = hex.DecodeString(sk1) + sk1Decoded, err = hex.DecodeString(sk1) if ok = assert.NoErrorf(t, err, "hex decode failed for sk1: %v", err); !ok { return } - // sendPrivkey = secp256k1.PrivKeyFromBytes(decoded) - decoded, err = hex.DecodeString("02" + pub2) + pub2Decoded, err = hex.DecodeString("02" + pub2) if ok = assert.NoErrorf(t, err, "hex decode failed for pub2: %v", err); !ok { return } - _, err = secp256k1.ParsePubKey(decoded) + _, err = nip44.GenerateConversationKey(sk1Decoded, pub2Decoded) assert.ErrorContains(t, err, msg) } -func assertConversationKeyGeneration(t *testing.T, sendPrivkey *secp256k1.PrivateKey, recvPubkey *secp256k1.PublicKey, conversationKey string) bool { +func assertConversationKeyGeneration(t *testing.T, sendPrivkey []byte, recvPubkey []byte, conversationKey string) bool { var ( actualConversationKey []byte expectedConversationKey []byte @@ -127,7 +123,10 @@ func assertConversationKeyGeneration(t *testing.T, sendPrivkey *secp256k1.Privat if ok = assert.NoErrorf(t, err, "hex decode failed for conversation key: %v", err); !ok { return false } - actualConversationKey = nip44.GenerateConversationKey(sendPrivkey, recvPubkey) + actualConversationKey, err = nip44.GenerateConversationKey(sendPrivkey, recvPubkey) + if ok = assert.NoErrorf(t, err, "conversation key generation failed: %v", err); !ok { + return false + } if ok = assert.Equalf(t, expectedConversationKey, actualConversationKey, "wrong conversation key"); !ok { return false } @@ -136,41 +135,39 @@ func assertConversationKeyGeneration(t *testing.T, sendPrivkey *secp256k1.Privat func assertConversationKeyGenerationSec(t *testing.T, sk1 string, sk2 string, conversationKey string) bool { var ( - sendPrivkey *secp256k1.PrivateKey - recvPubkey *secp256k1.PublicKey + sk1Decoded []byte + pub2Decoded []byte ok bool err error ) - if decoded, err := hex.DecodeString(sk1); err == nil { - sendPrivkey = secp256k1.PrivKeyFromBytes(decoded) - } + sk1Decoded, err = hex.DecodeString(sk1) if ok = assert.NoErrorf(t, err, "hex decode failed for sk1: %v", err); !ok { return false } if decoded, err := hex.DecodeString(sk2); err == nil { - recvPubkey = secp256k1.PrivKeyFromBytes(decoded).PubKey() + pub2Decoded = secp256k1.PrivKeyFromBytes(decoded).PubKey().SerializeCompressed() } if ok = assert.NoErrorf(t, err, "hex decode failed for sk2: %v", err); !ok { return false } - return assertConversationKeyGeneration(t, sendPrivkey, recvPubkey, conversationKey) + return assertConversationKeyGeneration(t, sk1Decoded, pub2Decoded, conversationKey) } func assertConversationKeyGenerationPub(t *testing.T, sk1 string, pub2 string, conversationKey string) bool { var ( - sendPrivkey *secp256k1.PrivateKey - recvPubkey *secp256k1.PublicKey + sk1Decoded []byte + pub2Decoded []byte ok bool err error ) - if decoded, err := hex.DecodeString(sk1); err == nil { - sendPrivkey = secp256k1.PrivKeyFromBytes(decoded) - } + sk1Decoded, err = hex.DecodeString(sk1) if ok = assert.NoErrorf(t, err, "hex decode failed for sk1: %v", err); !ok { return false } if decoded, err := hex.DecodeString("02" + pub2); err == nil { - recvPubkey, err = secp256k1.ParsePubKey(decoded) + if recvPubkey, err := secp256k1.ParsePubKey(decoded); err == nil { + pub2Decoded = recvPubkey.SerializeCompressed() + } if ok = assert.NoErrorf(t, err, "parse pubkey failed: %v", err); !ok { return false } @@ -178,7 +175,7 @@ func assertConversationKeyGenerationPub(t *testing.T, sk1 string, pub2 string, c if ok = assert.NoErrorf(t, err, "hex decode failed for pub2: %v", err); !ok { return false } - return assertConversationKeyGeneration(t, sendPrivkey, recvPubkey, conversationKey) + return assertConversationKeyGeneration(t, sk1Decoded, pub2Decoded, conversationKey) } func assertMessageKeyGeneration(t *testing.T, conversationKey string, salt string, chachaKey string, chachaSalt string, hmacKey string) bool { @@ -431,7 +428,7 @@ func TestConversationKeyFail001(t *testing.T) { assertConversationKeyFail(t, "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", - "x coordinate 1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef is not on the secp256k1 curve", + "invalid private key: x coordinate ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff is not on the secp256k1 curve", ) } @@ -440,7 +437,7 @@ func TestConversationKeyFail002(t *testing.T) { assertConversationKeyFail(t, "0000000000000000000000000000000000000000000000000000000000000000", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", - "x coordinate 1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef is not on the secp256k1 curve", + "invalid private key: x coordinate 0000000000000000000000000000000000000000000000000000000000000000 is not on the secp256k1 curve", ) } @@ -458,7 +455,7 @@ func TestConversationKeyFail004(t *testing.T) { assertConversationKeyFail(t, "fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", - "x coordinate 1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef is not on the secp256k1 curve", + "invalid private key: x coordinate fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141 is not on the secp256k1 curve", ) }