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.

Sign up for GitHub

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

Jump to bottom

Fix bug in second hash calculation #141

Merged
merged 14 commits into from
Mar 3, 2023
Merged

Fix bug in second hash calculation #141

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc6b7af
Fix bug in second hash calculation
ischasny Mar 2, 2023
211c6ea
Review feedback
ischasny Mar 2, 2023
d52a261
Add dhash test
ischasny Mar 2, 2023
97b536b
Fix go.mod
ischasny Mar 2, 2023
f71ac58
Update store/dhash/dhash.go
ischasny Mar 3, 2023
8397bed
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
3c83f4e
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
b6de4a3
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
bf7b779
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
f46ab32
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
db78586
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
b96a4f7
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
3318d7d
Update store/dhash/dhash_test.go
ischasny Mar 3, 2023
5fbbb28
Clean up
ischasny Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/libp2p/go-libp2p v0.23.2
github.com/multiformats/go-multihash v0.2.1
github.com/multiformats/go-varint v0.0.7
github.com/stretchr/testify v1.8.1
go.opencensus.io v0.23.0
golang.org/x/crypto v0.3.0
lukechampine.com/blake3 v1.1.7
Expand All @@ -30,6 +31,7 @@ require (
github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f // indirect
github.com/cockroachdb/redact v1.0.8 // indirect
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 // indirect
github.com/gammazero/deque v0.2.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
Expand All @@ -50,11 +52,13 @@ require (
github.com/multiformats/go-multibase v0.1.1 // indirect
github.com/multiformats/go-multicodec v0.7.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572 // indirect
github.com/spaolacci/murmur3 v1.1.0 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.8.0 // indirect
go.uber.org/zap v1.24.0 // indirect
golang.org/x/exp v0.0.0-20221205204356-47842c84f3db // indirect
golang.org/x/sys v0.3.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
7 changes: 7 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,17 @@ github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb6
github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4=
Expand Down Expand Up @@ -408,6 +413,7 @@ google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpAD
google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE=
Expand All @@ -421,6 +427,7 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
lukechampine.com/blake3 v1.1.7 h1:GgRMhmdsuK8+ii6UZFDL8Nb+VyMwadAgcJyfYHxG6n0=
Expand Down
54 changes: 33 additions & 21 deletions store/dhash/dhash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/aes"
"crypto/cipher"
"crypto/sha256"
"encoding/binary"

"github.com/libp2p/go-libp2p/core/peer"
"github.com/multiformats/go-multihash"
Expand All @@ -13,30 +14,39 @@ import (
const (
// nonceLen defines length of the nonce to use for AESGCM encryption
nonceLen = 12
// keysize defines the size of multihash key
keysize = 32
)

var (
// secondHashPrefix is a prefix that a mulithash is prepended with when calculating a second hash
secondHashPrefix = []byte("CR_DOUBLEHASH\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
// deriveKeyPrefix is a prefix that a multihash is prepended with when deriving an encryption key
deriveKeyPrefix = []byte("CR_ENCRYPTIONKEY\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
// noncePrefix is a prefix that a multihash is prepended with when calculating a nonce
noncePrefix = []byte("CR_NONCE\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")
)

// SecondSHA returns SHA256 over the payload
func SHA256(payload, dest []byte) []byte {
return sha256Multiple(dest, payload)
}

func sha256Multiple(dest []byte, payloads ...[]byte) []byte {
h := sha256.New()
h.Write(payload)
for _, payload := range payloads {
h.Write(payload)
}
return h.Sum(dest)
}

// SecondMultihash calculates SHA256 over the multihash and wraps it into another multihash with DBL_SHA256 codec
func SecondMultihash(mh multihash.Multihash) (multihash.Multihash, error) {
prefix := []byte("CR_DOUBLEHASH")
mh, err := multihash.Sum(append(prefix, mh...), multihash.DBL_SHA2_256, keysize)
if err != nil {
return nil, err
}
return mh, nil
digest := SHA256(append(secondHashPrefix, mh...), nil)
return multihash.Encode(digest, multihash.DBL_SHA2_256)
}

// deriveKey derives encryptioin key from the passphrase using SHA256
func deriveKey(passphrase []byte) []byte {
return SHA256(append([]byte("AESGCM"), passphrase...), nil)
return SHA256(append(deriveKeyPrefix, passphrase...), nil)
}

// DecryptAES decrypts AES payload using the nonce and the passphrase
Expand All @@ -62,9 +72,11 @@ func EncryptAES(payload, passphrase []byte) ([]byte, []byte, error) {
derivedKey := deriveKey([]byte(passphrase))

// Create initialization vector (nonse) to be used during encryption
ischasny marked this conversation as resolved.
Show resolved Hide resolved
// Nonce is derived from the mulithash (passpharase) so that encrypted payloads
// for the same multihash can be compared to each other without having to decrypt
nonce := SHA256(passphrase, nil)[:nonceLen]
// Nonce is derived from the passphrase concatenated with the payload so that the encrypted payloads
// for the same multihash can be compared to each other without having to decrypt them, as it's not possible.
payloadLen := make([]byte, 8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Concerning the payloadLen, the exact format isn't defined yet in ipfs/specs#373.

Would it make more sense to use a constant byte array (e.g make([]byte, 8)) or to use an unsigned varint. As we mostly encrypt peerids, that are quite short, we don't always need a big number for the payload len. A varint allows to use less bytes to describe the payload len, and the maximal payload len can also be very large if needed.

I think that I am more in favor of an unsigned varint format for the payload len, as it gives more agility. WDYT?

cc: @ischasny @Jorropo @masih

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't always need a big number for the payload len

These values are hashed over, so payloadLen isn't going to be a part of the encrypted value, i.e. see line 79: nonce := sha256Multiple(nil, noncePrefix, payloadLen, payload, passphrase)[:nonceLen].

Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, all good! 👍🏻

binary.LittleEndian.PutUint64(payloadLen, uint64(len(payload)))
nonce := sha256Multiple(nil, noncePrefix, payloadLen, payload, passphrase)[:nonceLen]

// Create cypher and seal the data
block, err := aes.NewCipher(derivedKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

The format for Encrypted PeerID/Metadata was recently updated in the DHT Spec. The format should be [encryption_varint, payload_len, nonce, encrypted_payload]. See ipfs/specs#373

The encryption varint for aes-gcm-256 is 0x8040 and the multicodec is 0x2000 (See multiformats/multicodec#314 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point! As IPNI doesn't have anything stored after EncPeerID the encryption_varint and payload_len can be appended on retrieval.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect, just wanted to make sure we are on the same page 😄

Expand All @@ -83,13 +95,13 @@ func EncryptAES(payload, passphrase []byte) ([]byte, []byte, error) {
}

// DecryptValueKey decrypts the value key using the passphrase
func DecryptValueKey(valKey, passphrase []byte) ([]byte, error) {
return DecryptAES(valKey[:nonceLen], valKey[nonceLen:], passphrase)
func DecryptValueKey(valKey, mh multihash.Multihash) ([]byte, error) {
return DecryptAES(valKey[:nonceLen], valKey[nonceLen:], mh)
}

// EncryptValueKey encrypts raw value key using the passpharse
func EncryptValueKey(valKey, passphrase []byte) ([]byte, error) {
nonce, encValKey, err := EncryptAES(valKey, passphrase)
func EncryptValueKey(valKey, mh multihash.Multihash) ([]byte, error) {
nonce, encValKey, err := EncryptAES(valKey, mh)
if err != nil {
return nil, err
}
Expand All @@ -98,13 +110,13 @@ func EncryptValueKey(valKey, passphrase []byte) ([]byte, error) {
}

// DecryptMetadata decrypts metdata using the provided passphrase
func DecryptMetadata(encMetadata, passphrase []byte) ([]byte, error) {
return DecryptAES(encMetadata[:nonceLen], encMetadata[nonceLen:], passphrase)
func DecryptMetadata(encMetadata, valueKey []byte) ([]byte, error) {
return DecryptAES(encMetadata[:nonceLen], encMetadata[nonceLen:], valueKey)
}

// EncryptMetadata encrypts metadata using the provided passphrase
func EncryptMetadata(metadata, passphrase []byte) ([]byte, error) {
nonce, encValKey, err := EncryptAES(metadata, passphrase)
func EncryptMetadata(metadata, valueKey []byte) ([]byte, error) {
nonce, encValKey, err := EncryptAES(metadata, valueKey)
if err != nil {
return nil, err
}
Expand Down
72 changes: 72 additions & 0 deletions store/dhash/dhash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package dhash_test
ischasny marked this conversation as resolved.
Show resolved Hide resolved

import (
"bytes"
"crypto/sha256"
"math/rand"
"testing"

"github.com/ipni/go-indexer-core/store/dhash"
ischasny marked this conversation as resolved.
Show resolved Hide resolved
"github.com/ipni/go-indexer-core/store/test"
"github.com/multiformats/go-multihash"
"github.com/stretchr/testify/require"
)

ischasny marked this conversation as resolved.
Show resolved Hide resolved
func TestEncryptSameValueWithTheSameMultihashShouldProduceTheSameOutput(t *testing.T) {
rng := rand.New(rand.NewSource(1413))
payload := make([]byte, 256)
_, err := rng.Read(payload)
if err != nil {
panic(err)
}
passphrase := make([]byte, 32)
_, err = rng.Read(passphrase)
require.NoError(t, err)

nonce1, encrypted1, err := dhash.EncryptAES(payload, passphrase)
ischasny marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

nonce2, encrypted2, err := dhash.EncryptAES(payload, passphrase)
ischasny marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

require.True(t, bytes.Equal(nonce1, nonce2))
require.True(t, bytes.Equal(encrypted1, encrypted2))
}

func TestCanDecryptEncryptedValue(t *testing.T) {
rng := rand.New(rand.NewSource(1413))
payload := make([]byte, 256)
_, err := rng.Read(payload)
if err != nil {
panic(err)
}
passphrase := make([]byte, 32)
_, err = rng.Read(passphrase)
require.NoError(t, err)

nonce, encrypted, err := dhash.EncryptAES(payload, passphrase)
ischasny marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

decrypted, err := dhash.DecryptAES(nonce, encrypted, passphrase)
ischasny marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

require.True(t, bytes.Equal(payload, decrypted))
}

func TestSecondMultihash(t *testing.T) {
secondHashPrefix := []byte("CR_DOUBLEHASH\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")

mh := test.RandomMultihashes(1)[0]
smh, err := dhash.SecondMultihash(mh)
ischasny marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

h := sha256.New()
h.Write(append(secondHashPrefix, mh...))
digest := h.Sum(nil)

decoded, err := multihash.Decode(smh)
require.NoError(t, err)

require.Equal(t, uint64(multihash.DBL_SHA2_256), decoded.Code)
require.Equal(t, digest, decoded.Digest)
}