Skip to content

Conversation

blackbeam
Copy link

No description provided.

@mdxs
Copy link
Contributor

mdxs commented Nov 9, 2015

Is this PR correct and is it going to be accepted into ecc/PyECC here?

@niccokunzmann
Copy link
Collaborator

@blackbeam could you provide more information and context for your change, so we can review it?

@blackbeam
Copy link
Author

Output of this implementation is correct and tests proved it.
Problem is in encryption scheme (pt. 2.8 of RFC). And it is sad that RFC does not provide test vectors for it.
RFC states that E = M ^ S, or in other words:

 E[15..0] = M[15..0] ^ S[15..0]
 E[31..16] = M[31..16] ^ S[31..16]
 E[47..32] = M[47..32] ^ S[47..32]
...

But without this PR this implementation will generate:

 E[15..0] = M[15..0] ^ S[15..0]
 E[31..16] = M[31..16] ^ S[16..1]
 E[47..32] = M[47..32] ^ S[17..2]
...

It is both incompatible and insecure.

But as i can see now this PR is incomplete. Same issue is in encrypt function which is duplicate of keystream function and also there should be a test for this case.

I will fix this as soon as i get some spare time.

@blackbeam
Copy link
Author

Done.

@niccokunzmann
Copy link
Collaborator

I am not qualified enough to see through this change. @amintos would be, I guess. I also found this implementation in Java with some tests and this Javascript implementation which has a file called verified test vectors. That should be part of tests so we can see which implementation is correct.

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