kanidm / webauthn-rs

An implementation of webauthn components for Rustlang servers
Mozilla Public License 2.0
483 stars 80 forks source link

webauthn-rs-demo: make tls support optional #357

Closed micolous closed 11 months ago

micolous commented 11 months ago

rustls depends on ring 0.16, which doesn't build on aarch64-pc-windows-msvc, and has lots of other cross-compiling issues. Many of these issues have been fixed, but only on the ring 0.17 branch.

This PR makes webauthn-rs-demo TLS support optional (--features tls).

Longer term, I'd rather switch this over to using OpenSSL like everything else, but this at least unblocks building the entire workspace with default features on a system ring 0.16 doesn't support.

yaleman commented 11 months ago

In keeping with the "do it right by default" can we make TLS a default feature and just disable it in the windows workspace builds?

micolous commented 11 months ago

Can we make TLS a default feature and just disable it in the windows workspace builds?

There's no way to enable/disable this conditionally by platform: https://github.com/rust-lang/cargo/issues/1197

A work-around would be to convert it to a library, and then use it from another package which has a [dependency] declaration by platform. However, it'd be simpler to move this away from rustls entirely so that the problem goes away.

As far as I can tell, the other tide example doesn't build in TLS support at all.

In keeping with the "do it right by default"...

This binary currently generates a self-signed certificate at start-up, and there's no way to provide one. As a result, you either need to:

Because the certificate will change on start-up, there's marginal benefit to using HTTPS over plain HTTP.

I guess the open question is how is https://webauthn.firstyear.id.au configured, and will this break it?

yaleman commented 11 months ago

I more meant add tls to features.default which you can disable on build.

micolous commented 11 months ago

The trouble is that on an affected target, you'd need to apply --no-default-features as a workspace-level option, and then re-add all the other default features (which aren't broken) for every single package in the workspace, which will change over time.

TLS support in webauthn-rs-demo is not documented, nor is it enabled by default anyway. While this shifts the burden from (at minimum) an extremely niche platform (aarch64-pc-windows-gnu) to literally everyone else, the burden is to add --features tls for one package if they need TLS support; and there's no tests of any kind around it.

Once the demo is migrated from rustls to openssl, it could become a default (as openssl works just fine there). That migration would also mean that it doesn't ship two crypto libraries for a tiny demo app. 😄

micolous commented 11 months ago

Dropping this in favour of #360