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

whisper: sym encryption message padding includes salt #15631

Merged
merged 2 commits into from
Dec 11, 2017

Conversation

gballet
Copy link
Member

@gballet gballet commented Dec 8, 2017

Now that the AES salt has been moved to the payload, padding must be adjusted to hide it, lest an attacker guesses that the packet uses symmetric encryption.

@gballet gballet requested a review from gluk256 December 8, 2017 19:44
@@ -124,6 +124,8 @@ func (msg *sentMessage) appendPadding(params *MessageParams) error {
rawSize := len(params.Payload) + 1
if params.Src != nil {
rawSize += signatureLength
} else {
rawSize += AESNonceLength
Copy link
Contributor

Choose a reason for hiding this comment

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

this is true only for symmetric encryption

Copy link
Member Author

Choose a reason for hiding this comment

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

If Src is nil, then it's using symmetric encryption

Copy link
Contributor

@gluk256 gluk256 Dec 9, 2017

Choose a reason for hiding this comment

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

Src is used to identify the source of the message, i.e. for digital signature. Dst is used for asymmetrc encryption. And KeySym is used for symmetric encryption. So, you can change line 127 like this:
} else if params.KeySym != nil {

Copy link
Member Author

Choose a reason for hiding this comment

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

Well then it's very confusing. Then both sym and asym should be able to sign their messages. So it should be if and not else if

@gballet gballet force-pushed the whisper-v6-sym-padding branch from e8b755a to 3f8f954 Compare December 10, 2017 19:55
@fjl fjl merged commit e7610ea into ethereum:master Dec 11, 2017
@karalabe karalabe added this to the 1.8.0 milestone Dec 14, 2017
b00ris pushed a commit to b00ris/go-ethereum that referenced this pull request Jan 19, 2018
Now that the AES salt has been moved to the payload, padding must
be adjusted to hide it, lest an attacker guesses that the packet
uses symmetric encryption.
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
Now that the AES salt has been moved to the payload, padding must
be adjusted to hide it, lest an attacker guesses that the packet
uses symmetric encryption.
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