feat(webauthn)!: refuse synced passkeys by default and restrict models - #207
Merged
Conversation
authenticator_policy gains syncedPasskeys, defaulting to block, plus aaguidAllowList and aaguidDenyList. Breaking: a multi-device credential is synced by a platform password manager, so its private key exists outside the authenticator that made it, and every iCloud Keychain and Google Password Manager passkey is one. Deployments relying on platform passkeys stop enrolling them until they set syncedPasskeys to allow. This closes the gap between what the product did and the design position it was documented as holding. The public sector README recorded blocked by default, agency may enable, and separately recorded that it was not implemented. It is now implemented, and that README and security-posture.md say so. Judged on backup eligibility rather than current backup state, because a credential that can leave the device is the exposure whether or not it already has, and judging on current state would let one register while unsynced and sync afterwards. The AAGUID lists need attestation direct to mean anything. An allow list without it refuses everything, so that combination is reported at startup rather than discovered one failed enrolment at a time. An authenticator that declines to identify itself is refused against an allow list rather than admitted, since admitting it would make the list advisory. Refusals are distinguishable and each is recorded as a failed registration. Closes #170
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #170. Requires
@seamless-auth/types0.15.0, already released. FinishesSession 5 of #155.
Breaking, and worth reading before merging
authenticator_policy.syncedPasskeysdefaults toblock. Every iCloudKeychain and Google Password Manager passkey is a multi-device credential, so
on upgrade a deployment relying on platform passkeys stops enrolling them and
gets
403 { "error": "synced_passkey_not_allowed" }.AUTHENTICATOR_POLICY={"syncedPasskeys":"allow", ...}restores the previous behaviour. Existing credentials are unaffected; this
governs new registrations.
The default was chosen deliberately rather than inherited. The public sector
README recorded the design position as "blocked by default, agency may enable"
and separately recorded that it was not implemented. This closes that gap in
the direction the position states.
Eligibility, not current state
The decision is made on backup eligibility, surfaced as
credentialDeviceType: 'multiDevice', not on whether the credential is currentlybacked up. A credential that can leave the device is the exposure whether or
not it already has, and judging on current state would let one register while
unsynced and sync a minute later. There is a test named for that distinction.
Two decisions inside the evaluator
An authenticator that declines to identify itself is refused against an allow
list, not admitted. An all-zero AAGUID means "I will not say what I am".
Admitting it would make the allow list advisory, since anything could opt out of
being checked by staying anonymous.
The deny list is applied before the allow list, so a specific model can be
excluded even when a broad allow list would otherwise admit it.
The footgun, reported at startup
An allow list only means something under
attestation: 'direct'. Set one whileattestation is
noneand nothing can identify itself, so every registration isrefused. Rather than let that be discovered one failed enrolment at a time, the
server logs it once at boot naming the fix.
Documentation, which is an acceptance criterion here
docs/public-sector/README.mdsaid "Confirmed not implemented." It now saysimplemented, and how
docs/security-posture.mdgains a section on the posture, the eligibilityreasoning, and the attestation dependency, which is what that README asked for
configuration.mdand.env.examplecover the three new fieldsVerification
94 test files, 1043 passed, 1 skipped. Coverage 98.8% lines.
typecheck,lint,format:checkclean.12 unit tests on the evaluator and 5 integration tests through the real endpoint.
Mutation checked: disabling the synced-passkey branch fails 4 of them and nothing
else.
Note on the commit history
The pre-commit hook blocked this twice on unrelated tests, an admin anomalies
assertion and then a registration one. Both pass repeatedly in isolation, the
full sequential suite is green, and neither is in a path this change touches.
That is #190.