Update to actix-web 2.0

[HeroicKatora]

Project Improvement

actix-web 2.x has some breaking changes and comes with support for standard futures.

Other context

See prior comments in #37. It also requires an update to ring to support v0.16.

Tracking pull request

• A pull request is bump to actix-web 2.0 #72
• Related: Ring 0.16 and actix 2.0-alpha support #63
• Motivates: Tracking issue for async #70

[gzp79]

The error handling part (ex Resource grant) looks awkward a little bit:

 {
        Ok(Ok(_grant)) => ...  // granted
        Ok(Err(Ok(e))) => ... // denied
        Ok(Err(Err(e))) => ... // "internal" error during grant (ResourceFlow)
        Err(e) => ... // "internal" error in handler ex. mailbox error
    }

edit: fixed using ? operator

[gzp79]

Is there anything that block the related PR to be merged back to main? (#72)
Or is it the ring dependency ?

[thespooler]

Since #37 is closed and dicussion moved here, I'm also wondering about that.
The actix-web 2.0 update has been merged a few months ago (thanks @HeroicKatora).
Without bumping to ring 0.16.9, actix-identity and actix-session can't be used as they force actix-web/actix-http with feature session-cookie, wich requires 0.16.9.

Is there any path forward?

[HeroicKatora]

Yes, bite the bullet and replace ring with RustCrypto/* in the main crate, and create a oxide-auth-ring side crate with the ring-based code (which can then probably also be updated to a much newer version of ring).

[HeroicKatora]

The question is whether it really is sensible (from a security standpoint) to provide all existing implementations of authorizer and issuer or to drop some from the main crate. The map-based implementations are fine without having to rely on any the security of any crypto implementation so one could say that the encrypting ones are only supplementary and could live in another optional crate. The registrar implementation is a slight concern but really, pbkdf2 is not exactly state-of-the-art in any case. (Edit: possibly go for argon2 with argonautica).

[thespooler]

Quite the undertaking!
Why the side crate, is there really a hard dependency on someting from ring explicitly that doesn't have a RustCrypto equivalent? If so, could it really be decoupled from the main crate without introducing the same ring dependency, but one crate further? In other words, is it worth it? If we have an alternative, is keeping a ring impl something desirable?

As for replacing pbkdf2 with argon2, that seems easy enough.
I might try to start with that if it can help.

[HeroicKatora]

Between ring and RustCrypto there is a perceived difference in security. For example, ring puts great focus on making their interface misuse resistant. This has some security implications, despite RustCrypto being more efficient to write code in for new users ¹. It would be disingenuous to interpret this as saying that RustCrypto's implementation being insecure but I tried to limit its use.

If so, could it really be decoupled from the main crate without introducing the same ring dependency, but one crate further?

That's what made me close #58 previously. The argument, however, was that it left some interface entirely without an implementation and the conclusion was to prioritize if the ecosystem were to split further. It certainly has. So the idea would be to ensure that some implementation of all traits exists in the main crate and the ring implementations are in the extra side crate. This structure would at least make it possible to use oxide-auth without ring.

As for replacing pbkdf2 with argon2, that seems easy enough.
I might try to start with that if it can help.

Yep, that would be great. The blocker last time I had looked at it were slightly immature Rust crates for it but I wasn't aware of argonautica back then.

Footnotes

1. https://arxiv.org/pdf/1806.04929.pdf ↩

[thespooler]

Also, as discussed in #55, would you feel confident enough about rand to use its rand::CryptoRng?
Note that by default, it is just ThreadRng, wich is not Send+Sync.

[HeroicKatora]

Not really; given that it is a safe trait to implement, it is not sealed, and that its documentation explicitely states:

Note that this trait is provided for guidance only and cannot guarantee suitability for cryptographic applications.

However directly using a concrete implementor such as OsRng would be okay. Not so sure with ThreadRng.

[thespooler]

It is guidance only yes. OsRng and StdRng implements it. It just means that as far as you can trust the lib author, it is deemed Crypto worthy. From what I can see, StdRnd is chacha20, and OsRng is whatever PRNG is available from the OS from getrandom.

But you're alright with me trying my hand with a PR for using rand as crypto provider?

[thespooler]

With #85 #86 and #87 merged, oxide-auth should be able to be built alongside actix-web with actix-session and actix-identity.
I think this can be closed now.