New issue
Advanced search Search tips

Issue 814103 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression



Sign in to add a comment

Only serialise CollectedClientData once during webauthn processing

Project Member Reported by agl@chromium.org, Feb 21 2018

Issue description

We currently serialise the CollectedClientData several times during processing. Since I made the serialisation non-deterministic, this breaks things. The correct fix is to only serialise once but, to make things work before the branch, I'll make the serialisation deterministic again first.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb234c8e7356f2239700c604ad7b3eda42aaac33

commit eb234c8e7356f2239700c604ad7b3eda42aaac33
Author: Adam Langley <agl@chromium.org>
Date: Wed Feb 21 16:12:09 2018

webauthn: disable randomisation of CollectedClientData.

The JSON is serialised more than once during processing, thus the
randomisation can break things when the results differ. The correct
solution is to only do the work of serialising once, but this is a
minimal change that fixes the issue before the M66 branch point.

Bug:  814103 
Change-Id: Ia0e6506724fe3512fc7f1f513df4759023d9417c
Reviewed-on: https://chromium-review.googlesource.com/927961
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538121}
[modify] https://crrev.com/eb234c8e7356f2239700c604ad7b3eda42aaac33/content/browser/webauth/collected_client_data.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72d721e571da15fc104d4318dbe7fb04d6dcf661

commit 72d721e571da15fc104d4318dbe7fb04d6dcf661
Author: Adam Langley <agl@chromium.org>
Date: Fri Feb 23 19:37:08 2018

webauthn: only serialize CollectedClientData once.

This change causes |AuthenticatorImpl| to only serialize
|CollectedClientData| once for each request. Since the code then carries
it around as a string, the |CollectedClientData| object itself is never
used except to immediately call |SerializeToJson| on it. Thus
|CollectedClientData| is folded into a function on |AuthenticatorImpl| to
save two indirections.

Along the way, implement the changes to the Token Binding member[1] and
re-add the random hint to not template match[2] now that it doesn't
break things.

[1] https://github.com/w3c/webauthn/commit/a47fe1c4d53b123caa7abc76e9659b95dc1c1a16
[2] https://chromium.googlesource.com/chromium/src/+/eb234c8e7356f2239700c604ad7b3eda42aaac33

Bug:  814103 
Change-Id: Ifa96f3eee11bcbf740c7adff43e092ce5361210a
Reviewed-on: https://chromium-review.googlesource.com/929807
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538860}
[modify] https://crrev.com/72d721e571da15fc104d4318dbe7fb04d6dcf661/content/browser/BUILD.gn
[modify] https://crrev.com/72d721e571da15fc104d4318dbe7fb04d6dcf661/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/72d721e571da15fc104d4318dbe7fb04d6dcf661/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/72d721e571da15fc104d4318dbe7fb04d6dcf661/content/browser/webauth/authenticator_impl_unittest.cc
[delete] https://crrev.com/9aecd396345fd6c629d58ccfbb9c866e3b129ec1/content/browser/webauth/collected_client_data.cc
[delete] https://crrev.com/9aecd396345fd6c629d58ccfbb9c866e3b129ec1/content/browser/webauth/collected_client_data.h

Comment 3 by agl@chromium.org, Mar 7 2018

Status: Fixed (was: Assigned)

Sign in to add a comment