Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #204: factor makeCred into reg & authn priv considerations #777

Merged
merged 2 commits into from Feb 7, 2018

Conversation

equalsJeffH
Copy link
Contributor

@equalsJeffH equalsJeffH commented Feb 6, 2018

@equalsJeffH equalsJeffH added type:editorial privacy-tracker Group bringing to attention of Privacy, or tracked by the Privacy Group but not needing response. labels Feb 6, 2018
@equalsJeffH equalsJeffH added this to the CR milestone Feb 6, 2018
@equalsJeffH equalsJeffH self-assigned this Feb 6, 2018
Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

Looks good to me!

index.bs Outdated
{{PublicKeyCredential/[[DiscoverFromExternalSource]](origin, options, sameOriginWithAncestors)}} methods need to take care to
not leak information that could enable a malicious [=[RP]=] to distinguish between these cases, where "named" means that the
[=public key credential|credential=] is listed by the [=[RP]=] in either {{MakePublicKeyCredentialOptions/excludeCredentials}}
or {{PublicKeyCredentialRequestOptions/allowCredentials}}, as applicable:

- A named [=public key credential|credential=] is not available.
- A named [=public key credential|credential=] is available, but the user does not [=user consent|consent=] to use it.

If the above cases are distinguishable, information is leaked by which a malicious [=[RP]=] could identify the user by probing for
which [=public key credential|credentials=] are available. For example, one such information leak is if the client returns a
failure response as soon as the user denies [=user consent|consent=] to proceed with the operation. In this case the [=[RP]=]
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps change

as soon as the user denies [=user consent|consent=] to proceed with the operation

to something like this?

as soon as the user denies [=user consent|consent=] to proceed with an [=authentication=] [=ceremony=]

Seeing as the following sentence refers specifically to allowCredentials, I mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

incorp'd a build on this suggestion, pls review

Copy link
Member

Choose a reason for hiding this comment

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

What I was trying to say is that the conclusion "at least one of the credentials listed in the allowCredentials parameter is available" is applicable only to the case of authentication ceremonies, not registration ceremonies. For registration ceremonies the conclusion is instead that the user has an authenticator that is not registered, which probably isn't as big a privacy concern.

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Looks good to me

@nadalin
Copy link
Contributor

nadalin commented Feb 7, 2018

@equalsJeffH can you finish the merge here ?

@equalsJeffH
Copy link
Contributor Author

incorp'd a build on suggestion #777 (comment), pls review

@equalsJeffH equalsJeffH merged commit 1d7b023 into master Feb 7, 2018
@emlun emlun deleted the jeffh-fix-204-priv-acntids branch June 12, 2019 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
privacy-tracker Group bringing to attention of Privacy, or tracked by the Privacy Group but not needing response. type:editorial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Privacy across Account IDs
4 participants