Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Error when building paseto #40

Closed
anthonyjchriste opened this issue Jun 24, 2021 · 6 comments · Fixed by #41
Closed

Error when building paseto #40

anthonyjchriste opened this issue Jun 24, 2021 · 6 comments · Fixed by #41
Labels
bug Something isn't working

Comments

@anthonyjchriste
Copy link

anthonyjchriste commented Jun 24, 2021

Describe the bug
When I try to build the latest trunk branch, I get the following error.

error[E0308]: mismatched types
  --> src/v2/local.rs:48:48
   |
48 |   if let Ok(mut state) = GenericHashState::new(24, Some(nonce_key)) {
   |                                                ^^
   |                                                |
   |                                                expected enum `Option`, found integer
   |                                                help: try using a variant of the expected enum: `Some(24)`
   |
   = note: expected enum `Option<usize>`
              found type `{integer}`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `paseto`

To Reproduce
Steps to reproduce the behavior:

  1. Check our a fresh copy of the trunk branch or checkout the latest release
  2. Run cargo update to update compatible dependencies
  3. Run cargo check

Expected behavior
The crate should compile without error.

Versions (please complete the following information):

  • OS: Fedora release 34 (Thirty Four) 5.12.11-300.fc34.x86_64
  • Rustc version rustc 1.53.0 (53cb7b09b 2021-06-17)
  • Version 2.0.1+

Additional context

It would appear that sodiumoxide updated their API which caused a breaking change in this library.

I've forked the repo and will look to provide a PR.

@anthonyjchriste anthonyjchriste added the bug Something isn't working label Jun 24, 2021
@Mythra
Copy link
Collaborator

Mythra commented Jun 24, 2021

Yeah looks like they released a breaking API Change as part of a minor update 😭 , looks like sodiumoxide is also moving away to maintenance mode though. So we should probably be migrating away from it altogether. Thanks for catching this though! In the meantime you should be able to workaround without a patch by manually specifying a sodiumoxide dependency explicitly on 0.2.6.

@anthonyjchriste to what extent would you like to be involved here? I'd appreciate any help, but don't want to put a lot on you here either! As I'll probably take an action item here to just move away entirely from sodiumoxide.

@anthonyjchriste
Copy link
Author

I can help out with small patches. We make use of this library at work and I appreciate the effort put into it so far by all of the contributors. We have a vested interest in keeping this library updated.

With that said, I don't have a security background, so I would mainly be interested in contributing in other technical matters (refactoring, library maintenance, etc). I'm happy to hear what you have in mind though. I'm certainly not opposed to moving away from sodiumoxide as long as someone else is able to help verify changes on the security side of things.

@Mythra
Copy link
Collaborator

Mythra commented Jun 24, 2021

Sounds good, I'll start working on switching over. To be clear I'm not suggesting changing the actual cryptography being used, just moving away from sodiumoxide as the one doing those operations! (E.g. moving to a crate that performs the same operations). I'll get working on the patch, and should have something up by EoD tomorrow if all goes well!

@anthonyjchriste
Copy link
Author

That's great. Really appreciate it! Thanks for the absurdly quick response by open source standards!

@Mythra
Copy link
Collaborator

Mythra commented Jun 25, 2021

Hey @anthonyjchriste ,

This should all be good now, and a new patch release has been published too so you don't necessarily have to use trunk😉 . Thanks for pointing this out, and have a great rest of your day!

@anthonyjchriste
Copy link
Author

I just updated and can confirm all is well. Dropping some c dependencies is similarly a big win.

Thank you so much @Mythra! Your time is appreciated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants