Skip to content

Conversation

@gballet
Copy link
Member

@gballet gballet commented Nov 29, 2017

As per EIP-627, the Salt for symmetric encryption is now part of the payload. This commit does that.

@gballet gballet requested a review from gluk256 November 29, 2017 08:31
@gballet
Copy link
Member Author

gballet commented Nov 29, 2017

@gluk256 Unit tests aren't finished yet, they are part of an upcoming commit so don't press the 'accept' button before they are there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you make this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the EIP-627 packet codes specifies that:

The message codes reserved for Whisper protocol: 0 - 127. Messages with unknown codes must be ignored, for forward compatibility of future versions.

Support for messages with codes 126 and 127 will be added in subsequent PRs.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change to EIP was made after discussion with Robert (Parity) several months ago.

@fjl
Copy link
Contributor

fjl commented Dec 6, 2017

@gballet please run gofmt -w -s

@fjl
Copy link
Contributor

fjl commented Dec 7, 2017

Travis says:

$ gofmt -s -d whisper/whisperv6/envelope.go
diff -u whisper/whisperv6/envelope.go.orig whisper/whisperv6/envelope.go
--- whisper/whisperv6/envelope.go.orig	2017-12-07 13:53:39.317628694 +0000
+++ whisper/whisperv6/envelope.go	2017-12-07 13:53:39.317628694 +0000
@@ -36,12 +36,12 @@
 // Envelope represents a clear-text data packet to transmit through the Whisper
 // network. Its contents may or may not be encrypted and signed.
 type Envelope struct {
-	Version  []byte
-	Expiry   uint32
-	TTL      uint32
-	Topic    TopicType
-	Data     []byte
-	Nonce    uint64
+	Version []byte
+	Expiry  uint32
+	TTL     uint32
+	Topic   TopicType
+	Data    []byte
+	Nonce   uint64
 
 	pow  float64     // Message-specific PoW as described in the Whisper specification.
 	hash common.Hash // Cached hash of the envelope to avoid rehashing every time.
@@ -63,13 +63,13 @@
 // included into an envelope for network forwarding.
 func NewEnvelope(ttl uint32, topic TopicType, msg *sentMessage) *Envelope {
 	env := Envelope{
-		Version:  make([]byte, 1),
-		Expiry:   uint32(time.Now().Add(time.Second * time.Duration(ttl)).Unix()),
-		TTL:      ttl,
-		Topic:    topic,
-		Data:     msg.Raw,
-		Nonce:    0,
+		Version: make([]byte, 1),
+		Expiry:  uint32(time.Now().Add(time.Second * time.Duration(ttl)).Unix()),
+		TTL:     ttl,
+		Topic:   topic,
+		Data:    msg.Raw,
+		Nonce:   0,
 	}

 	if EnvelopeVersion < 256 {

@fjl
Copy link
Contributor

fjl commented Dec 7, 2017

This is weird because the diff doesn't actually apply to envelope.go

gballet and others added 10 commits December 7, 2017 16:30
The aes nonce is now part of the payload of a symmetric
message. Since there is no futher available indication
whether a message is symmetric or not, functions
isSymmetric and isAsymmetric have been removed and
a symmetric topic match will be attempted if the filter
is symmetric. Accordingly, other checks have been
removed at the top level, because they can no longer
be performed until it is known for a fact that the
message uses symmetric encryption.
That was left out in the last commit.
There is an ongoing discussion whether this is the proper
way to go, as filters implementing both sym and asym keys
will see an unsuccessful attempt at an asymmetric
decryption be followed by an attempt at a symmetric
decription. This is inefficient in that case, which should
not happen too often. The alternative is to forbid using
filters with both options enabled, and will require
updating unit tests.
It has been decided not to allow filters to be both for
symmetric and asymmetric keys, because that's already
what the API enforces.
@fjl fjl force-pushed the whisper-v6-remove-aesnonce branch from a74db4c to 35297a8 Compare December 7, 2017 15:32
@fjl
Copy link
Contributor

fjl commented Dec 7, 2017

Rebased. This should be OK now.

@fjl fjl merged commit d95962c into ethereum:master Dec 8, 2017
@karalabe karalabe added this to the 1.8.0 milestone Dec 14, 2017
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
As per EIP-627, the salt for symmetric encryption is now
part of the payload. This commit does that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants