Specify if clients are expected to follow redirects for icon URLs #1285

lgarron · 2019-08-23T19:52:01Z

At GitHub, we have "nice" profile icon URLs like https://github.com/lgarron.png

That URL redirects to https://avatars2.githubusercontent.com/u/248078?v=4 , which is moderately stable but could change some day. Could I ask for the spec to specify whether RPs are expected to follow the redirect from the more permanent URL?

I can't find information on this; #1139 is the closest I've found

emlun · 2019-08-25T15:45:00Z

Both user.icon and rp.icon are specified by the RP*, so RPs aren't expected to process them at all after passing them into a WebAuthn function call. #1139 has some discussion on what authenticators and/or clients are expected to do with them.

* Or by the user under the RP's supervision, unless the user modifies the client-side script. In either case, the RP controls the values for the purposes of this discussion.

lgarron · 2019-08-25T22:51:00Z

Ah, I'm sorry, I meant to ask about clients!

#1139 is moderately relevant, but it would be useful to know if we (as an RP) can count on clients to do at least one redirect!

jcjones · 2019-08-28T19:22:52Z

For practical purposes, no user agent is going to actively fetch these icons, as they would be potent correlation mechanisms for resident credentials -- after so much effort is taken elsewhere in the specification to avoid correlation.

Perhaps the right thing to do here is to amend the definition for these icons to specify that these are only valid if they are data: URLs of a valid image type.

lgarron · 2019-08-28T20:29:41Z

Perhaps the right thing to do here is to amend the definition for these icons to specify that these are only valid if they are data: URLs of a valid image type.

In that case, it would be nice to specify a more reasonable expectation than 128 bytes of storage for the icon.

128 bytes is reasonable for a URL, but squeezing icons into 128 data: bytes is basically futile. A recommended target of 4K or 8K is somewhat doable. (Perhaps it could be specified like srcset, offering a large image for computer/phone-resident authenticator storage with lots of space vs.small one for space-constrained physical key storage.)

dwaite · 2019-08-28T23:30:38Z

My understanding of the correlation issue is that a client may cache the icon at the conclusion of authentication and registration, but a client will not resolve the icon before authentication using the network due to correlation concerns.

I agree with @lgarron that very few relying parties will manage to fit a usable icon within the 85 bytes you can hypothetically squeeze out of a 128 character data URL. It is more likely they will use a regular URL and let clients deal with the UX of not having an icon.

emlun · 2019-09-02T14:33:30Z

A recommended target of 4K or 8K is somewhat doable.

Sorry, this is not going to happen for hardware authenticators at least. Resident credentials already require a significant amount of storage space.

ve7jtb · 2019-09-03T15:30:19Z

The browser could cache it during make credential, and after authenticating. I am guessing that browsers may not be able to easily display only cached icons.
Larger storage requirements is not going to fly for roaming authenticators, unless you want them to store only one or two credentials.
On a practical note do we have the size etc of the icons sufficiently defined so that a platform could display them? This may be a bit like the authenticator icon in meta-data, something that seems like a good idea, but is never used in practice.
If browsers are not going to display them I would drop them going forward to make more room.

dwaite · 2019-09-03T18:24:50Z

On a practical note do we have the size etc of the icons sufficiently defined so that a platform could display them? This may be a bit like the authenticator icon in meta-data, something that seems like a good idea, but is never used in practice

Thats my concern. We also have two icons:

The RP entity icon, which likely is the same for all RP registrations. It isn't useful for choosing a credential to use, but may be useful if you are managing the credentials on an authenticator.
The user entity icon, which is likely different for each registration. It might be useful for choosing a credential to use, but will likely only contain a usable value if the RP maintains profile information including a photo or avatar. It also must be user-differentiated, so fundamentally must leak information on loading.

If desired, the RP icon would have a good shot of being changed to be widely usable. For example, we could reference an image source set via .well-known of the RP domain, and have clients fetch that without session state. Of course, a well-known lookup also technically no longer needs the URL field.

We don't have infrastructure to use the user icon ad-hoc (such as an intermediate, anonymizing cache). So the user icon is only really displayable if previously cached by a registration or authentication action, loaded by user action, etc. We still would have the problem that not all RPs have a usable picture for this, or guidance on how it will be displayed.

nadalin · 2019-09-20T02:54:41Z

@jcjones Create PR

jcjones · 2019-09-20T02:56:05Z

I will make a PR to deprecate the image fields and any others that are unlikely to be utilized by platforms or authenticators.

jcjones · 2019-10-23T19:21:01Z

Still working on this!

As discussed in issue w3c#1285, the image URL fields for PublicKeyCredentialEntity, while intended for user interface design, are potent correlation mechanisms if they are downloaded by RPs. RPs would have to take extraordinary care, beyond reasonable measures, to avoid uses by RPs with mal-intent to cross-correlate accounts. It is better for User Agents to use existing origin/icon mechanisms for their UX designs, or to define new such mechanisms as-needed, that are origin-wide rather than provide the possibility to embed detailed tracking information into these URLs.

Fix #1285 - Remove icons from PublicKeyCredentialEntity

As discussed in issue #1285, the image URL fields for PublicKeyCredentialEntity, while intended for user interface design, are potent correlation mechanisms if they are downloaded by RPs. RPs would have to take extraordinary care, beyond reasonable measures, to avoid uses by RPs with mal-intent to cross-correlate accounts. It is better for User Agents to use existing origin/icon mechanisms for their UX designs, or to define new such mechanisms as-needed, that are origin-wide rather than provide the possibility to embed detailed tracking information into these URLs.

Fix #1285 - Remove icons from PublicKeyCredentialEntity

https://bugs.webkit.org/show_bug.cgi?id=203346 <rdar://problem/56558488> Reviewed by Brent Fulgham. Source/WebCore: Covered by new tests within existing test files. * Modules/webauthn/AuthenticatorAssertionResponse.h: (WebCore::AuthenticatorAssertionResponse::setName): (WebCore::AuthenticatorAssertionResponse::name const): (WebCore::AuthenticatorAssertionResponse::setDisplayName): (WebCore::AuthenticatorAssertionResponse::displayName const): (WebCore::AuthenticatorAssertionResponse::setNumberOfCredentials): (WebCore::AuthenticatorAssertionResponse::numberOfCredentials const): Adds new members to store new fields of the response from the authenticator. Field "icon" is omitted given it could be used to track users according to w3c/webauthn#1285. * Modules/webauthn/fido/DeviceResponseConverter.cpp: (fido::readCTAPGetAssertionResponse): Adds new logic to parse above fields from an authenticator response. Source/WebKit: This patch implements authenticatorGetNextAssertion as suggested by the spec: https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticatorGetNextAssertion The work flow is as follow: 1) When a valid assertion response is received, check its numberOfCredentials member; 2) When it is larger then 1, use authenticatorGetNextAssertion to get all remaining responses; 3) Once all responses are gathered, ask UI clients to pick one to return. * UIProcess/API/APIWebAuthenticationPanelClient.h: (API::WebAuthenticationPanelClient::selectAssertionResponses const): * UIProcess/WebAuthentication/Authenticator.h: * UIProcess/WebAuthentication/AuthenticatorManager.cpp: (WebKit::AuthenticatorManager::selectAssertionResponses): * UIProcess/WebAuthentication/AuthenticatorManager.h: * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp: (WebKit::MockHidConnection::parseRequest): * UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp: (WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived): (WebKit::CtapAuthenticator::continueGetNextAssertionAfterResponseReceived): * UIProcess/WebAuthentication/fido/CtapAuthenticator.h: Tools: * TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WebCore/FidoTestData.h: Adds new test case for new logic in DeviceResponseConverter. LayoutTests: * http/wpt/webauthn/public-key-credential-get-failure-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-failure-hid.https.html: * http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-success-hid.https.html: * http/wpt/webauthn/resources/util.js: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254356 268f45cc-cd09-0410-ab3c-d52691b4dbfc

https://bugs.webkit.org/show_bug.cgi?id=203346 <rdar://problem/56558488> Reviewed by Brent Fulgham. Source/WebCore: Covered by new tests within existing test files. * Modules/webauthn/AuthenticatorAssertionResponse.h: (WebCore::AuthenticatorAssertionResponse::setName): (WebCore::AuthenticatorAssertionResponse::name const): (WebCore::AuthenticatorAssertionResponse::setDisplayName): (WebCore::AuthenticatorAssertionResponse::displayName const): (WebCore::AuthenticatorAssertionResponse::setNumberOfCredentials): (WebCore::AuthenticatorAssertionResponse::numberOfCredentials const): Adds new members to store new fields of the response from the authenticator. Field "icon" is omitted given it could be used to track users according to w3c/webauthn#1285. * Modules/webauthn/fido/DeviceResponseConverter.cpp: (fido::readCTAPGetAssertionResponse): Adds new logic to parse above fields from an authenticator response. Source/WebKit: This patch implements authenticatorGetNextAssertion as suggested by the spec: https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#authenticatorGetNextAssertion The work flow is as follow: 1) When a valid assertion response is received, check its numberOfCredentials member; 2) When it is larger then 1, use authenticatorGetNextAssertion to get all remaining responses; 3) Once all responses are gathered, ask UI clients to pick one to return. * UIProcess/API/APIWebAuthenticationPanelClient.h: (API::WebAuthenticationPanelClient::selectAssertionResponses const): * UIProcess/WebAuthentication/Authenticator.h: * UIProcess/WebAuthentication/AuthenticatorManager.cpp: (WebKit::AuthenticatorManager::selectAssertionResponses): * UIProcess/WebAuthentication/AuthenticatorManager.h: * UIProcess/WebAuthentication/Mock/MockHidConnection.cpp: (WebKit::MockHidConnection::parseRequest): * UIProcess/WebAuthentication/fido/CtapAuthenticator.cpp: (WebKit::CtapAuthenticator::continueGetAssertionAfterResponseReceived): (WebKit::CtapAuthenticator::continueGetNextAssertionAfterResponseReceived): * UIProcess/WebAuthentication/fido/CtapAuthenticator.h: Tools: * TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp: (TestWebKitAPI::TEST): * TestWebKitAPI/Tests/WebCore/FidoTestData.h: Adds new test case for new logic in DeviceResponseConverter. LayoutTests: * http/wpt/webauthn/public-key-credential-get-failure-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-failure-hid.https.html: * http/wpt/webauthn/public-key-credential-get-success-hid.https-expected.txt: * http/wpt/webauthn/public-key-credential-get-success-hid.https.html: * http/wpt/webauthn/resources/util.js: Canonical link: https://commits.webkit.org/219195@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254356 268f45cc-cd09-0410-ab3c-d52691b4dbfc

lgarron changed the title ~~Are RPs expected to follow redirects for icon URLs?~~ Are clients expected to follow redirects for icon URLs? Aug 25, 2019

lgarron changed the title ~~Are clients expected to follow redirects for icon URLs?~~ Specify if clients are expected to follow redirects for icon URLs Aug 26, 2019

nadalin assigned jcjones Aug 28, 2019

nadalin modified the milestones: L3-WD-01, L2-WD-02 Aug 28, 2019

equalsJeffH added the type:technical label Sep 9, 2019

jcjones mentioned this issue Oct 29, 2019

Fix #1285 - Remove icons from PublicKeyCredentialEntity #1337

Merged

equalsJeffH added the stat:pr-open label Oct 30, 2019

jcjones closed this as completed in dbcf596 Oct 30, 2019

jcjones added a commit that referenced this issue Oct 30, 2019

Merge pull request #1337 from jcjones/1285-image_deprecation

28e8d9d

Fix #1285 - Remove icons from PublicKeyCredentialEntity

emlun pushed a commit that referenced this issue Nov 6, 2019

Merge pull request #1337 from jcjones/1285-image_deprecation

2e0fe7a

Fix #1285 - Remove icons from PublicKeyCredentialEntity

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify if clients are expected to follow redirects for icon URLs #1285

Specify if clients are expected to follow redirects for icon URLs #1285

lgarron commented Aug 23, 2019

emlun commented Aug 25, 2019

lgarron commented Aug 25, 2019 •

edited

jcjones commented Aug 28, 2019

lgarron commented Aug 28, 2019 •

edited

dwaite commented Aug 28, 2019

emlun commented Sep 2, 2019

ve7jtb commented Sep 3, 2019

dwaite commented Sep 3, 2019

nadalin commented Sep 20, 2019

jcjones commented Sep 20, 2019

jcjones commented Oct 23, 2019

Specify if clients are expected to follow redirects for icon URLs #1285

Specify if clients are expected to follow redirects for icon URLs #1285

Comments

lgarron commented Aug 23, 2019

emlun commented Aug 25, 2019

lgarron commented Aug 25, 2019 • edited

jcjones commented Aug 28, 2019

lgarron commented Aug 28, 2019 • edited

dwaite commented Aug 28, 2019

emlun commented Sep 2, 2019

ve7jtb commented Sep 3, 2019

dwaite commented Sep 3, 2019

nadalin commented Sep 20, 2019

jcjones commented Sep 20, 2019

jcjones commented Oct 23, 2019

lgarron commented Aug 25, 2019 •

edited

lgarron commented Aug 28, 2019 •

edited