cedarcode / webauthn-ruby

WebAuthn ruby server library ― Make your Ruby/Rails web server become a conformant WebAuthn Relying Party
https://rubygems.org/gems/webauthn
MIT License
649 stars 53 forks source link

Prepare for OpenSSL 3 compatibility #360

Closed ClearlyClaire closed 2 years ago

ClearlyClaire commented 2 years ago

This is not enough for OpenSSL 3 compatibility as various dependencies still need fixing, but as far as I know, it should fix all incompatible code from the webauthn-ruby gem itself without breaking compatibility with the 2.2 version of the openssl gem.

Actual compatibility with OpenSSL 3 would require at least the following to be merged:

lokst commented 2 years ago

Thank you for raising this, I am also keen on OpenSSL 3 support 🙂

itay-grudev commented 2 years ago

@ClearlyClaire I am unsure why the additional changes are required. All tests pass with openssl v3.0.0 with just the gemspec change. I created an MR simply with that unaware of your work:

PR: https://github.com/cedarcode/webauthn-ruby/pull/361 Test results: https://github.com/itay-grudev/webauthn-ruby/runs/6905678939

Have I missed something really important? I also don't see any deprecation notices.

ClearlyClaire commented 2 years ago

@ClearlyClaire I am unsure why the additional changes are required. All tests pass with openssl v3.0.0 with just the gemspec change. I created an MR simply with that unaware of your work:

PR: #361 Test results: https://github.com/itay-grudev/webauthn-ruby/runs/6905678939

Have I missed something really important? I also don't see any deprecation notices.

If I remember correctly, the code will run just fine with no deprecation notice on the openssl gem 3.0.0 built against OpenSSL 1.1, but will fail when built against OpenSSL 3.

itay-grudev commented 2 years ago

I understand. I will close my PR and will upvote yours. Thank you!

itay-grudev commented 2 years ago

Is your work backwards compatible with openssl < v3? If so there is really no reason not to merge it.

ClearlyClaire commented 2 years ago

Is your work backwards compatible with openssl v2? If so there is really no reason not to merge it.

I ran tests with OpenSSL 1.1 and had no issue.

senid231 commented 2 years ago

@ClearlyClaire thanks for your work

any news about this?

brauliomartinezlm commented 2 years ago

@ClearlyClaire sorry for the delay. If you can update the dependencies and add openssl 3 to Appraisal we can get this in shortly. Thanks!

ClearlyClaire commented 2 years ago

This gems does not seem to be using Appraisal, it does not come with a Gemfile.lock, and the webauthn.gemspec seems to not forbid openssl 3 or cose 1.2.1. Whether it will need any more change therefore seems to rely entirely on which version tpm-key_attestation will be released as next.

brauliomartinezlm commented 2 years ago

This gems does not seem to be using Appraisal, it does not come with a Gemfile.lock, and the webauthn.gemspec seems to not forbid openssl 3 or cose 1.2.1. Whether it will need any more change therefore seems to rely entirely on which version tpm-key_attestation will be released as next.

You're right. There's no appraisal here. I was thinking about the conformance tests when I wrote that. I won't block this on that tho, we can fix bump conformance tests to latest version in a separate issue/PR.

I just release tpm-key_attestation v0.11.0 so we need to bump it here in webauthn-ruby

ClearlyClaire commented 2 years ago

Seems like tpm-key_attestation has some other breaking changes introduced by https://github.com/cedarcode/tpm-key_attestation/commit/e3e014d8c74a29b68300a45e2d70ebecd04db549

ClearlyClaire commented 2 years ago

Pushed a commit changing use of TPM::KeyAttestation to accomodate for the aforementioned breaking changes. The tests run but I'm not 100% sure I haven't missed something. Also, I haven't changed webauthn-ruby's terminology so there is a discrepancy between it and tpm-key_attestation wrt. root certificates vs. trusted certificates.

ClearlyClaire commented 2 years ago

Is there anything blocking a merge?