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

RC4 module #20

Merged
merged 11 commits into from May 27, 2011
Merged

RC4 module #20

merged 11 commits into from May 27, 2011

Conversation

thoughtpolice
Copy link

Modulo some small typos and whatever else may be lying around, this module contains my full RC4 implementation, as well as a whole ton of checks against a ton of reference test vectors and cryptocipher, and some yices-proven properties.

As we talked about before, while you can generate code currently for the RC4 example, it's not recommended. That should probably be noted, unless you were going to cajole the code and try and get it to do code generation a little more nicely before you release a new version.

Sorry it took so long (and if you've been waiting on it,) been kinda busy! :)

@LeventErkok
Copy link
Owner

Hi Austin:

I got your pull request and I'll do so when I get a chance. However, the dependence on the crypto package worries me a bit. Do you mind if I trim down/modify your implementation to eliminate that? Of course, you can do the same and send another pull request if you feel like doing it yourself.

-Levent.

On May 23, 2011, at 4:34 AM, thoughtpolice wrote:

Modulo some small typos and whatever else may be lying around, this module contains my full RC4 implementation, as well as a whole ton of checks against a ton of reference test vectors and cryptocipher, and some yices-proven properties.

As we talked about before, while you can generate code currently for the RC4 example, it's not recommended. That should probably be noted, unless you were going to cajole the code and try and get it to do code generation a little more nicely before you release a new version.

Sorry it took so long (and if you've been waiting on it,) been kinda busy! :)

Reply to this email directly or view it on GitHub:
#20

@thoughtpolice
Copy link
Author

I can do it, no problem!

FYI, on Github, you don't need to send another pull request. Instead, I can just push more commits to my RC4 branch, and they will automatically show up in this pull request. So when I've eliminated the cryptocipher dependency (very easy: only used for some quickcheck tests,) I'll push the changes, and ping you on here to let you know you can merge this branch into master.

@LeventErkok
Copy link
Owner

great! looking forward to it..

-Levent.

On May 26, 2011, at 7:12 AM, thoughtpolice wrote:

I can do it, no problem!

FYI, on Github, you don't need to send another pull request. Instead, I can just push more commits to my RC4 branch, and they will automatically show up in this pull request. So when I've eliminated the cryptocipher dependency (very easy: only used for some quickcheck tests,) I'll push the changes, and ping you on here to let you know you can merge this branch into master.

Reply to this email directly or view it on GitHub:
#20 (comment)

@thoughtpolice
Copy link
Author

OK, I merely left all the code, but commented it out and took away the dependencies (in case people want to look at the source.) If you'd like to remove it completely, go ahead!

LeventErkok added a commit that referenced this pull request May 27, 2011
@LeventErkok LeventErkok merged commit 7c5e9ea into LeventErkok:master May 27, 2011
@LeventErkok
Copy link
Owner

Austin:

When I generate C code, I get C files of size 9MB or so.. Not only those are not going to be fast, gcc will be unlikely to compile them.. I was wondering if you knew about that, or checked in an old version?

It might be the case that RC4 is not a good candidate for code generation.. I'll take a closer look.

-Levent.

On May 26, 2011, at 8:43 PM, thoughtpolice wrote:

OK, I merely left all the code, but commented it out and took away the dependencies (in case people want to look at the source.) If you'd like to remove it completely, go ahead!

Reply to this email directly or view it on GitHub:
#20 (comment)

@thoughtpolice
Copy link
Author

No, I left it untouched since the last time we talked about the issue via email (I thought I mentioned that prior to this but apparently not.) I haven't taken time to re-jig it to see if I can get better generated code, although I could try. You may very well be right RC4 isn't a good candidate for code generation, but the call of whether you think it's fixable and whether it should go into the release is probably better left to you than me I suppose.

@LeventErkok
Copy link
Owner

Austin:

I got a chance to study the RC4 algorithm a bit more. It's the text-book "you need mutable arrays for this" example. Unfortunately, there is no current way in SBV to model RC4 to get fast execution and efficient C code. Imagine programming RC4 in Haskell where all you had was immutable lists to work with as your data structure. You'd have similar problems.

However, I think the example is still quite interesting. I cleaned up the code a bit and pushed a simpler variant. See what you think. If it's OK with you, this is what I'd like to include in the next hackage release.

-Levent.

On May 27, 2011, at 12:57 AM, thoughtpolice wrote:

No, I left it untouched since the last time we talked about the issue via email (I thought I mentioned that prior to this but apparently not.) I haven't taken time to re-jig it to see if I can get better generated code, although I could try. You may very well be right RC4 isn't a good candidate for code generation, but the call of whether you think it's fixable and whether it should go into the release is probably better left to you than me I suppose.

Reply to this email directly or view it on GitHub:
#20 (comment)

@thoughtpolice
Copy link
Author

Wow! Very nice, I like the new implementation a lot - mine was more of a straightforward port to the C code. It's still in place (which is a problem,) but very clean and simple.

I'm definitely OK with seeing this in a new release and glad it's finally in, so go ahead!

@LeventErkok
Copy link
Owner

Cool.. I'm not planning any immediate releases, but it'll go out eventually (maybe within the next month or so).

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.

None yet

2 participants