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

Added Serpent #60

Merged
merged 10 commits into from Jan 25, 2020
Merged

Added Serpent #60

merged 10 commits into from Jan 25, 2020

Conversation

Slals
Copy link
Contributor

@Slals Slals commented Dec 2, 2019

Serpent1 first implementation. Some work to do for optimization.

Bitslicing implemented.
No benchmark.

Tested on 128bits, 192bits and 256bits key official vectors.

There is one panic in the code (in round and round_inverse), should it returns an Err instead?

EDIT : Bitslicing is now implemented. No more panic in the code.

@Slals Slals changed the title Serpent Added Serpent Dec 2, 2019
@imclint21
Copy link

Awesome, it will be really useful for Lucid!

@tarcieri
Copy link
Member

tarcieri commented Dec 2, 2019

Bitslicing not implemented.

I'd be wary of merging this until that's implemented as generally all implementations we currently provide at least attempt to be constant-time.

Curious what @newpavlov thinks.

@Slals
Copy link
Contributor Author

Slals commented Dec 4, 2019

Hello @newpavlov @tarcieri

I'm currently implementing bitslicing (this code includes bitslicing in key schedule already).

I have a question about it though, do we let users to choose to be in bitslice mode?

@tarcieri
Copy link
Member

tarcieri commented Dec 4, 2019

IMO it should always use the bitsliced version to ensure consant time(ish) operation

@imclint21
Copy link

imclint21 commented Dec 6, 2019

When it will be merged? I'll need it in the next week! 🎉

Thanks

@tarcieri
Copy link
Member

tarcieri commented Dec 6, 2019

I'll try to review it this weekend. I'm not terribly familiar with Serpent for anything other than its historical place in the AES competition, but I'm wary to merge any cipher implementations that haven't been thoroughly reviewed.

Hopefully @newpavlov can take a look as well, and we'll need him to release the serpent crate for use.

@clintnetwork in the meantime can you use a git dependency?

@imclint21
Copy link

Oh very nice! Don't worry @tarcieri do as you can :)

@imclint21
Copy link

Hey @tarcieri, any news about Serpent? 🐍

@Slals
Copy link
Contributor Author

Slals commented Jan 13, 2020

@newpavlov Sorry to bother you, you might be needed for this : Serpent Implementation.

Are you guys busy? How can I help you in order to make it merge?

@tarcieri
Copy link
Member

I'm flying home from the RealWorldCrypto conference today, and @newpavlov is working on his PhD defense, so yes we're both busy.

I should hopefully have some time this weekend to review (I know that's what I said over a month ago, mea culpa), but I'm not familiar with Serpent internals so I would need to study up first.

If you're looking for something to do in the interim to improve confidence in the implementation, you could use e.g. proptest or quickcheck to compare it against a well-known reference implementation.

You could also run cargo rustc --release -- --emit asm (or put the code in Godbolt with -C opt-level=3) and inspect the generated assembly for branch instructions.

@imclint21
Copy link

Hi @tarcieri, I think it's time to merge!

@Slals All works fine? I tried but I've some issues.

@tarcieri
Copy link
Member

Alright, I did a few passes on it and I don't see anything obviously wrong with it, so I can go ahead and merge.

Would appreciate if @newpavlov can do a second pass. He's also the only one with access to publish the crate.

@tarcieri tarcieri merged commit 4ccad52 into RustCrypto:master Jan 25, 2020
imclint21 added a commit to lucid-kv/lucid that referenced this pull request Jan 25, 2020
@tarcieri
Copy link
Member

I just released serpent v0.0.1: #75

@imclint21
Copy link

Thank you!

@newpavlov newpavlov mentioned this pull request Jun 8, 2020
19 tasks
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

3 participants