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 a Rust library #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

burgerdev
Copy link
Collaborator

@burgerdev burgerdev commented Feb 29, 2024

More or less the first thing I did when I started this branch was researching the appropriate X25519 library to use. Now, almost 3 years later, I still find it rather hard to decide. My top picks were x25519_dalek, ed25519_compact and openssl bindings, in order of preference. But when I started with the Dalek crate, I encountered strange problems when using its getrandom feature, so I rewrote the thing for ed25519_compact. Quite a lot seems to have happened since then - I did not see any of the issues I had back then when I re-rewrote the thing just now.

This train of rewrites is the main motivation behind me implementing two versions of the API. I really wanted to have an interface that does not leak the underlying X25519 library - this is how we ended up with the untyped module. Happy to hear your thoughts, but I, for one, consider

fn tag(ours: &[u8; 32], theirs: &[u8; 32], ctr: u8, msg: &[u8]) -> [u8; 32]

... not as an improvement over C, so I wrote the typed module, too, which uses dalek types.

There is probably a third option, with an API that accepts traits and next to it a few impls for x25519 choices, but I don't think my Rust is up to that yet.

EDIT - June 2024: I implemented tagging and verification in terms of traits and provided trait impls for openssl and dalek. This is probably the way to go.

@vvidic
Copy link
Collaborator

vvidic commented Mar 7, 2024

Looks interesting, but my Rust is at a hello world level, so probably can't do a good review here :)

@vvidic vvidic removed their request for review March 7, 2024 20:58
@pkern pkern requested a review from isomer March 19, 2024 21:52
Copy link
Member

@pkern pkern left a comment

Choose a reason for hiding this comment

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

So I agree that the untyped variant is not great and neither is the typed variant. Is there a way where we could offer our own wrapper types? Or could we export these types as type aliases somehow? In general the underlying types are very simple, the traits are where it's at...

rust/examples/glome-cli.rs Outdated Show resolved Hide resolved
@pkern
Copy link
Member

pkern commented Apr 12, 2024

This looks straightforward and simple now. Thank you! :D

I had one question on chat if you can now implement these traits from another crate. And two things I stumbled upon was the dalek import from lib and lib being no_std. But I'd be happy to merge this as-is. The PR is still technically in draft mode, though. (And I understand that there are the TODOs in the code, so maybe we still want to extend TagArgs a little bit before merging?

@burgerdev burgerdev marked this pull request as ready for review June 1, 2024 10:13
@burgerdev
Copy link
Collaborator Author

This should be good to review now, I'll add a Github Action later on.

@burgerdev
Copy link
Collaborator Author

@nicmue could you please take a look at this PR, with particular attention to module structure, Rust best practices and the test implementation? I wonder if there's a better way to execute all test vectors for a given PublicKey/PrivateKey implementation.

@burgerdev burgerdev changed the title Add a Rust library and an example Add a Rust library Jun 1, 2024
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