Attestation verification

[m-barthelemy]

This PR aims at implementing support for authenticator attestation verification during registration. It mostly builds upon the great work that was added in https://github.com/swift-server/webauthn-swift/pull/19

• Support for packed attestations used by modern FDO2 devices
• Support for fido-u2f attestations used by older devices (and optionally by some modern devices too)
• Support for tpm and android-key attestations used respectively by Windows Hello and modern Android devices.
• Adds back dependency to swift-certificates.
• I have changed the WebAuthnManager.finishRegistration() method signature so that the optional root CA certs, if any, as passed as Certificate instead of Data(). This introduces a dependency on swift-certificates for the users, and I'm not sure how okay this is.
• The Credential returned WebAuthnManager.finishRegistration() now contains an attestationResult field (see AttestationResult) with a few fields related to the attestation. This allows the relying party to make further decisions based on if and how the authenticator attestation was verified.

Currently, this PR doesn't allow to explicitly enable or disable attestation verification. If we find an attestation object (with a format not being .none), we will try to verify it.
 This can cause issues/surprises for the relying party when receiving such payloads if the relying party never intended to care about attestation and didn't enable it (most people don't need or care about attestation).
 Do we want to add a flag to WebAuthnManager.finishRegistration(), disabled by default, that would only try to validate the attestation if set?
 Or do we want to make it a separate operation? In that case finishRegistration() shouldn't even have a parameter accepting CA certs.

Need general advice and guidance in order to hopefully get this in a mergeable state someday!

[dimitribouniol]

Nice additions! (Sorry, couldn't help myself 😛)

Happy to help review this, but could you perhaps prepare a series of smaller bite-size PRs to make that easier? We can use this PR to track the remaining progress as those go in one by one.

Do we want to add a flag to WebAuthnManager.finishRegistration(), disabled by default, that would only try to validate the attestation if set?
 Or do we want to make it a separate operation? In that case finishRegistration() shouldn't even have a parameter accepting CA certs.

I would think that non-validating attestation data being ignored would be worse, so instead, we may want to communicate the expected attestation mode(s), which should match up with the corresponding setup from beginRegistration().

[dimitribouniol]

(btw, we also have a channel in the open source swift slack if you wanted a more direct discussion path: https://join.slack.com/t/swift-open-source/shared_invite/zt-2ildchnoz-YHLE1Pgx6ifVhAUat3pa7A #webauthn)

[m-barthelemy]

Nice additions! (Sorry, couldn't help myself 😛)

Thanks a lot for your feedback!

Happy to help review this, but could you perhaps prepare a series of smaller bite-size PRs to make that easier? We can use this PR to track the remaining progress as those go in one by one.

Yeah I fully understand that the current PR is massive and hard to review! I think I can easily split it into at least 3 smaller PRs:

• one about high-level and public stuff such as potential changes to finishRegistration() and what could be returned to the user upon verification (AttestationResult)
• one that has the bulk of the internal plumbing and maybe includes support for .packed attestations
• one that contains implementations for the other attestation formats (.tpm, .fidoU2F, .androidKey). If needed this could even be split into separate PRs for each format

I would think that non-validating attestation data being ignored would be worse, so instead, we may want to communicate the expected attestation mode(s), which should match up with the corresponding setup from beginRegistration().

So this could mean changing the current signature:

    public func finishRegistration(
        challenge: [UInt8],
        credentialCreationData: RegistrationCredential,
        requireUserVerification: Bool = false,
        supportedPublicKeyAlgorithms: [PublicKeyCredentialParameters] = .supported,
        pemRootCertificatesByFormat: [AttestationFormat: [Data]] = [:],
        confirmCredentialIDNotRegisteredYet: (String) async throws -> Bool
    ) async throws -> Credential

to something like:

    public func finishRegistration(
        challenge: [UInt8],
        credentialCreationData: RegistrationCredential,
        requireUserVerification: Bool = false,
        supportedPublicKeyAlgorithms: [PublicKeyCredentialParameters] = .supported,
        // Attestation verification will be triggered if set to `.direct`
        attestation: AttestationConveyancePreference = .none,
        pemRootCertificatesByFormat: [AttestationFormat: [Data]] = [:],
        confirmCredentialIDNotRegisteredYet: (String) async throws -> Bool
    ) async throws -> Credential

right? If so, makes sense to me! Except maybe that AttestationConveyancePreference now looks more like a "required attestation verification mode(s)" - does it make sense to defined another enum that would have the same cases as AttestationConveyancePreference ?