Skip to content

Conversation

@dscreve
Copy link
Contributor

@dscreve dscreve commented Nov 7, 2025

This PR contains some accessibility changes to make challenge custom integration working and make integration more useful.


package struct ChallengeGenerator: Sendable {
public struct ChallengeGenerator: Sendable {
public static let challengeSize: Int = 32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this isn't settable, does this need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I need to know the expected size when receiving request...I wouldn't change the behavior, then make it public only as let. I you are okay with make it settable, I can make the change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide a use case for why the challenge size needs to be known ahead of time? ChallengeGenerator is only really intended to be swappable in this target's tests, so from a user's point of view, the size of the challenge should be an implementation detail that can change freely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you can change the size of challenge as you want, because this can be retrieved with this value.
The use case is related to a specific architecture where challenge cannot be stored because the authentication service does not run on system that has access to the user database. I need to perform some additional security checks and for that, I need to append variable sized data to the returned challenge. So, I need to know the expected size to extract the original challenge which has been generated by the WebAuthn API and pass it to the API.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to perform some additional security checks and for that, I need to append variable sized data to the returned challenge

I would suggest ignoring the returned challenge and just using your value then — using it in your scheme provides no additional benefits.

That said, I will once again quote the spec (emphasis in bold mine):

§13.4.3. Cryptographic Challenges
As a cryptographic protocol, Web Authentication is dependent upon randomized challenges to avoid replay attacks. Therefore, the values of both PublicKeyCredentialCreationOptions.challenge and PublicKeyCredentialRequestOptions.challenge MUST be randomly generated by Relying Parties in an environment they trust (e.g., on the server-side), and the returned challenge value in the client’s response MUST match what was generated. This SHOULD be done in a fashion that does not rely upon a client’s behavior, e.g., the Relying Party SHOULD store the challenge temporarily until the operation is complete. Tolerating a mismatch will compromise the security of the protocol.

Challenges SHOULD be valid for a duration similar to the upper limit of the recommended range and default for a WebAuthn ceremony timeout.

In order to prevent replay attacks, the challenges MUST contain enough entropy to make guessing them infeasible. Challenges SHOULD therefore be at least 16 bytes long.

You really should be storing the challenge, whether in a separate DB, redis or valkey instance, in memory, or even with an extra helper service, and doing anything else without a complete understanding of how to validate that no two challenges can ever be re-used opens up your system to unnecessary security vulnerabilities.

//===----------------------------------------------------------------------===//

extension Duration {
public extension Duration {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This library shouldn't be making public extensions on external types, as they'll likely conflict if officially added. If you need this, go ahead and copy this extension, but I would push to have this list in the standard library instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants