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
658 stars 55 forks source link

Attestation Statement trustworthiness is not enforced as expected #337

Closed tanelsuurhans closed 3 years ago

tanelsuurhans commented 3 years ago

Preconditions:

When using a Yubikey 5c to register on Chrome OSX, the user is prompted to decide if they want to share the attestation information with the IDP or not. Given this device does not meet the AttCA type, and choosing to share the attestation information, the attestation verification fails with an exception as expected.

However when choosing not to share the information with the IDP, the attestation statement type is set to None, and it passes the verification step with flying colors. This seems to be unintended, as when the acceptable types does not include None, then it should not be allowed to pass.

This seems to be due to the actual verify method only being implemented in the attestation type subclasses, and not in the base class. Given the type is set to None, it invokes the verification method defined in that class, and skips the trustworthy? check completely. This logic should be altered to where it checks the trustworthiness in the base class and delegates the specifics to the subclasses.

Also hi @brauliomartinezlm and @ssuttner :)

brauliomartinezlm commented 3 years ago

Hi @tanelsuurhans 👋 ! Thank you for reporting this :). You are correct on the problem.

I did a small PR to correct the issue. I'll wait for @grzuy or @bdewater for a quick CR.

grzuy commented 3 years ago

Fix released in v2.4.1.

Thanks @tanelsuurhans for helping improve the gem by reporting this issue! Useful to know some are using the attestation: direct also.