New issue
Advanced search Search tips

Issue 863988 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Make RP ID hash check cover non-device authenticators

Project Member Reported by martinkr@google.com, Jul 16

Issue description

The ResponseData::CheckRpIdHash calls should move up into the request handler so they cover platform authenticators
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20

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

commit 8987fd0ef8e33333f9b529b3e62f8893182a56cf
Author: Martin Kreichgauer <martinkr@google.com>
Date: Fri Jul 20 22:42:03 2018

device/fido: move post request checks from FidoTask in RequestHandler

This extends the scope of these checks to cover non-device
authenticators (like Touch ID).

The following checks are moved from MakeAssertionTask into
MakeAssertionRequestHandler:
 - |CheckRpIdHash| to verify the RP ID of the request matches the
returned credential.

The following checks are moved from GetAssertionTask into
MakeAssertionRequestHandler:
 - |CheckRpIdHash| to verify the RP ID of the request matches the
returned credential.
 - |CheckRequirementsOnReturnedUserEntities| to check constraints on the
optional UserEntity response field.
 - |CheckRequirementsOnReturnedCredentialId| to check whether the
returned credential id was in the allow list (except for resident keys).

Also fixes the following bugs in
|CheckRequirementsOnReturnedCredentialId|:
 - Responses with resident key support should still have their response
checked against the allow list if one was provided.
 - For allow lists of size 1, the credential id may be omitted in the
reponse; but if it is not, it must be checked against the allow list.

Corresponding unit tests are moved accordingly.

Bug:  863988 ,  678128 
Change-Id: If7b76e7ecac45d96914a62661da9979c62895a25
Reviewed-on: https://chromium-review.googlesource.com/1144403
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577017}
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/fido_request_handler.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/fido_task.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_request_handler.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_task.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_task.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/get_assertion_task_unittest.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_request_handler.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_task.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_task.h
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/make_credential_task_unittest.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/mock_fido_device.cc
[modify] https://crrev.com/8987fd0ef8e33333f9b529b3e62f8893182a56cf/device/fido/mock_fido_device.h

Status: Fixed (was: Assigned)

Sign in to add a comment