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

Add check for Key Degradation Attack #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tuxmike
Copy link

@tuxmike tuxmike commented Jul 19, 2018

Found out, that the SPAKE2-implementation is missing an important check for key degradation, where an attacker is able to degrade the keys to identitiy (zero element), when he gets to know the password. (see https://moderncrypto.org/mail-archive/curves/2015/000427.html)
This is critical, because, it would destroy PFS on the channel, and makes MiM much more easier.

@todo: just added a small check for zero element, but had no time to really test it.
@todo: didn't check the relevance for SPAKE2+

Regards, Mike

@tuxmike tuxmike force-pushed the keydegradationcheck branch from 5ca5603 to cdedf88 Compare July 19, 2018 11:17
@coveralls
Copy link

coveralls commented Jul 19, 2018

Coverage Status

Coverage increased (+0.2%) to 96.441% when pulling 60b00f4 on tuxmike:keydegradationcheck into 43d0b84 on warner:master.

@tuxmike tuxmike closed this Jul 19, 2018
@tuxmike tuxmike reopened this Jul 19, 2018
@tuxmike
Copy link
Author

tuxmike commented Jul 19, 2018

Strange, the python 3.3 build always fails... guess it has nothing to do with my PR...

@warner
Copy link
Owner

warner commented Jul 15, 2019

Hey, thanks for looking into this.. sorry it's taken me so long to follow up.

It sounds like a reasonable patch to make. The py3.3 build was problematic for other reasons.. we've removed it (and py34 too), so if you were to rebase your patch to current trunk, I think it would pass.

I'd like to understand the attack properly. If I understand it correctly:

  • the attacker has control over the network, and mounts a standard MitM attack
  • the attacker also knows the shared secret password being used by Alice and Bob
  • the attacker examines Alice's message going to Bob, modifies it by multiplying, then sends the result to Bob as if it were from Alice
  • the attacker does the same thing with Bob's message to Alice
  • Alice and Bob now both wind up with a K value of the identity element, which is predictable to the attacker

This implementation follows the SPAKE2 advice/spec and hashes the entire transcript (including the received message) to build the session key (self._finalize(K_bytes) calls finalize_SPAKE2(idA, idB, X_msg, Y_msg, K_bytes, pw)). So I believe Alice will see one key (basically hash(K=0, real_X_msg, fake_Y_msg)) and Bob will see a different key (hash(K=0, fake_X_msg, real_Y_msg)). The attacker will know both keys, and they can decrypt/manipulate/reencrypt in the usual MitM style. But if Alice and Bob had some out-of-band way to compare their session keys, they could discover the discrepancy.

If we weren't including the messages in the transcript, then both session keys would be the same (and known to the attacker), which means out-of-band comparison would not reveal the attack. This patch would then prevent the password-knowing MitM attacker from pulling off an undiscoverable attack, leaving them the expected ability to pull off a discoverable attack.

Any attacker who knows the secret password and can get in the middle of the conversation can pull off a discoverable attack without any special math, they just execute the protocol normally with Alice (producing an Alice-Attacker key), and again with Bob (producing a Bob-Attacker key), and decrypt/reencrypt all the messages going through. But since the keys are different, an out-of-band comparison could reveal it. Being able to force the key to a known value would thwart this comparison check, but only if the key depends just on K and not also on the protocol transcript.

Does that seem right?

@tuxmike
Copy link
Author

tuxmike commented Nov 9, 2019

Hi warner,
did not see your reply until know. Yes, you described that totally right. If the session key is built from both K and transcript, it should be fine / reduce to classic MitM.

Beside you considerations, without the patch, it is also possible to force K to 0 just from a malicious Alice: If she sets her ephemeral key to 0, this will also force K to 0. But it (luckily) does not harm PFS, as the transcript will add Bobs ephemeral key in form of his sent Y key.

But there may exist other attacks using this method that we did not consider, who knows...

I think it regardless makes sense, to prevent manipulated X/Y values to remove the ephemeral/PFS part of the key exchange (producing K).

@tuxmike tuxmike force-pushed the keydegradationcheck branch from cb5a1c1 to 60b00f4 Compare November 9, 2019 11:57
@tuxmike
Copy link
Author

tuxmike commented Nov 9, 2019

CI still fails...

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.

3 participants