Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypt decrypt #2130

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
93 changes: 93 additions & 0 deletions btcec/ciphering.go
Expand Up @@ -5,7 +5,14 @@
package btcec

import (
"bytes"
"crypto/aes"
"crypto/cipher"
"crypto/rand"
"crypto/sha256"
"fmt"
secp "github.com/decred/dcrd/dcrec/secp256k1/v4"
"math/big"
)

// GenerateSharedSecret generates a shared secret based on a private key and a
Expand All @@ -14,3 +21,89 @@ import (
func GenerateSharedSecret(privkey *PrivateKey, pubkey *PublicKey) []byte {
return secp.GenerateSharedSecret(privkey, pubkey)
}

// Encrypt encrypts a message using a public key, returning the encrypted message or an error.
// It generates an ephemeral key, derives a shared secret and an encryption key, then encrypts the message using AES-GCM.
// The ephemeral public key, nonce, tag and encrypted message are then combined and returned as a single byte slice.
func Encrypt(pubKey *PublicKey, msg []byte) ([]byte, error) {
yemmyharry marked this conversation as resolved.
Show resolved Hide resolved
var pt bytes.Buffer

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to copy the cipher text into a different variable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var pt bytes.Buffer is used to accumulate the encrypted data in a buffer, which is then returned as a single byte slice at the end of the function

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not return the cipher text itself?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt return the ciphertext itself because i need some details in the decrypt function. the tag (and other parameters like nonce etc) are appended to the ciphertext in the encrypt function and in the decrypt function the tag is separated from the ciphertext to verify authenticity.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tag is in the cipher text already

Is there a reason that you did not just do this:

nonce := make([]byte, aead.NonceSize())
ciphertext := make([]byte, 4+len(ephemeralPubKey))
binary.LittleEndian.PutUint32(ciphertext, uint32(len(ephemeralPubKey)))
copy(ciphertext[4:], ephemeralPubKey)
ciphertext = aead.Seal(ciphertext, nonce, plaintext, ephemeralPubKey)

Gotten from here: https://pkg.go.dev/github.com/decred/dcrd/dcrec/secp256k1/v4#section-readme

No need for the intermediary pt bytes in this case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both approaches are valid. I used a buffer as it automatically grows and shrinks as needed, reducing manual memory management.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, I do not think the buffer is needed at all when the cipher text can just be returned directly unless there is need to copy the cipher text into a different variable. if not maybe we should not use extra space.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt return the ciphertext itself because i need some details in the decrypt function. the tag (and other parameters like nonce etc) are appended to the ciphertext in the encrypt function and in the decrypt function the tag is separated from the ciphertext to verify authenticity.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nonce := make([]byte, aead.NonceSize())
ciphertext := make([]byte, 4+len(ephemeralPubKey))
binary.LittleEndian.PutUint32(ciphertext, uint32(len(ephemeralPubKey)))
copy(ciphertext[4:], ephemeralPubKey)
ciphertext = aead.Seal(ciphertext, nonce, plaintext, ephemeralPubKey)

If you write it this way would you still need the intermediary buffer?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ephemeral, err := NewPrivateKey()
if err != nil {
return nil, fmt.Errorf("failed to generate private key: %v", err)
}

pt.Write(ephemeral.PubKey().SerializeUncompressed())

ecdhKey := GenerateSharedSecret(ephemeral, pubKey)
hashedSecret := sha256.Sum256(ecdhKey)
encryptionKey := hashedSecret[:16]

block, err := aes.NewCipher(encryptionKey)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you sliced the shared secret to 16 bytes here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes for compatibility with AES-128-GCM encryption

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh thought you said AES-256-GCM here: a2245a6#r1570762508 but looks like the comment has been updated.

if err != nil {
return nil, err
}

nonce := make([]byte, 16)
if _, err := rand.Read(nonce); err != nil {
return nil, err
}

pt.Write(nonce)

gcm, err := cipher.NewGCMWithNonceSize(block, 16)
if err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to understand why it has to be a nonce size of 16 instead of the default 12

return nil, err
}

ciphertext := gcm.Seal(nil, nonce, msg, nil)

tag := ciphertext[len(ciphertext)-gcm.NonceSize():]
pt.Write(tag)
ciphertext = ciphertext[:len(ciphertext)-len(tag)]
pt.Write(ciphertext)

return pt.Bytes(), nil
}

// Decrypt decrypts data that was encrypted using the Encrypt function.
// The decrypted message is returned if the decryption is successful, or an error is returned if there are any issues.
func Decrypt(privkey *PrivateKey, msg []byte) ([]byte, error) {
// Message cannot be less than length of public key (65) + nonce (16) + tag (16)
if len(msg) <= (1 + 32 + 32 + 16 + 16) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the length of public key compressed 33 bytes? Maybe make comments clearer on how the number came about, what does 1 represent here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An uncompressed public key in Elliptic Curve Cryptography (ECC) consists of:
1 byte for the curve type
32 bytes for the X coordinate
32 bytes for the Y coordinate

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An uncompressed public key in Elliptic Curve Cryptography (ECC) consists of: 1 byte for the curve type 32 bytes for the X coordinate 32 bytes for the Y coordinate

Was actually referring to serialized compressed public key which is what was written into the bytes during the encryption.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its actually an uncompressed public key.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that is true, I can see that an uncompressed public key was used in the encrypt function. Maybe this code would be clearer this way:

Suggested change
if len(msg) <= (1 + 32 + 32 + 16 + 16) {
// Message cannot be less than the size of an
// uncompressed public key (65 bytes) + nonce (16 bytes)
// + tag (16 bytes)
if len(msg) <= (65+ 16 + 16) {

Or something better
If 1+ 32 + 32 is to be included maybe include where the numbers are from in the comment as well?

return nil, fmt.Errorf("invalid length of message")
}

pb := new(big.Int).SetBytes(msg[:65]).Bytes()
pubKey, err := ParsePubKey(pb)
if err != nil {
return nil, err
}

ecdhKey := GenerateSharedSecret(privkey, pubKey)
hashedSecret := sha256.Sum256(ecdhKey)
encryptionKey := hashedSecret[:16]

msg = msg[65:]
nonce := msg[:16]
tag := msg[16:32]

ciphertext := bytes.Join([][]byte{msg[32:], tag}, nil)

block, err := aes.NewCipher(encryptionKey)
if err != nil {
return nil, fmt.Errorf("cannot create new aes block: %w", err)
}

gcm, err := cipher.NewGCMWithNonceSize(block, 16)
if err != nil {
return nil, fmt.Errorf("cannot create gcm cipher: %w", err)
}

plaintext, err := gcm.Open(nil, nonce, ciphertext, nil)
if err != nil {
return nil, fmt.Errorf("cannot decrypt ciphertext: %w", err)
}

return plaintext, nil
}
30 changes: 30 additions & 0 deletions btcec/ciphering_test.go
Expand Up @@ -6,6 +6,9 @@ package btcec

import (
"bytes"
"encoding/hex"
"fmt"
"github.com/stretchr/testify/assert"
"testing"
)

Expand All @@ -29,3 +32,30 @@ func TestGenerateSharedSecret(t *testing.T) {
secret1, secret2)
}
}

func TestEncryptAndDecrypt(t *testing.T) {
privateKey, err := NewPrivateKey()
if err != nil {
t.Errorf("private key generation error: %s", err)
return
}
publicKey := privateKey.PubKey()
message := []byte("Hello, this is a test message.")

encryptedMessage, err := Encrypt(publicKey, message)
if !assert.NoError(t, err) {
return
}

fmt.Println("Encrypted Message:", hex.EncodeToString(encryptedMessage))

decryptedMessage, err := Decrypt(privateKey, encryptedMessage)
if !assert.NoError(t, err) {
return
}

fmt.Println("Decrypted Message:", string(decryptedMessage))

assert.Equal(t, string(message), string(decryptedMessage))

}