content/browser/webauth: allow credProtect level two for non-resident keys.

If `requireResidentKey` is false an authenticator may, at least with
CTAP 2.0, still create a resident key. RPs might want to set credProtect
level two for that case even though it's moot for non-resident keys.

This change allows that stance.

Bug: 941120
Change-Id: If4be7aabbb2d5b002942a0dc033f49b4c71b8538
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1595972
Auto-Submit: Adam Langley <agl@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657458}

content/browser/webauth/authenticator_common.cc

@@ -663,12 +663,12 @@ void AuthenticatorCommon::MakeCredential(
        (options->protection_policy ==
             blink::mojom::ProtectionPolicy::UNSPECIFIED ||
         options->protection_policy == blink::mojom::ProtectionPolicy::NONE)) ||
-      // For non-resident keys the only protection that makes sense is
-      // UV_REQUIRED (or UNSPECIFIED).
+      // For non-resident keys, NONE doesn't make sense. (UV_OR_CRED_ID_REQUIRED
+      // does because, with CTAP 2.0, just because a resident key isn't
+      // _required_ doesn't mean that one won't be created and an RP might want
+      // credProtect to take effect if that happens.)
       (!resident_key &&
-       (options->protection_policy == blink::mojom::ProtectionPolicy::NONE ||
-        options->protection_policy ==
-            blink::mojom::ProtectionPolicy::UV_OR_CRED_ID_REQUIRED)) ||
+       options->protection_policy == blink::mojom::ProtectionPolicy::NONE) ||
       // UV_REQUIRED only makes sense if UV is required overall.
       (options->protection_policy ==
            blink::mojom::ProtectionPolicy::UV_REQUIRED &&

content/browser/webauth/authenticator_impl_unittest.cc

@@ -3572,8 +3572,10 @@ TEST_F(ResidentKeyAuthenticatorImplTest, CredProtectRegistration) {
     {  false,   false,   UNSPECIFIED, true,  false,   kNonsense, UNSPECIFIED},
     {  false,   false,   NONE,        false, false,   kNonsense, UNSPECIFIED},
     {  false,   false,   NONE,        true,  false,   kNonsense, UNSPECIFIED},
-    {  false,   false,   UV_OR_CRED,  false, false,   kNonsense, UNSPECIFIED},
-    {  false,   false,   UV_OR_CRED,  true,  false,   kNonsense, UNSPECIFIED},
+    {  false,   false,   UV_OR_CRED,  false, false,   kOk,       NONE},
+    {  false,   false,   UV_OR_CRED,  true,  false,   kNotAllow, UNSPECIFIED},
+    {  false,   false,   UV_OR_CRED,  false, true,    kOk,       NONE},
+    {  false,   false,   UV_OR_CRED,  true,  true,    kNotAllow, UNSPECIFIED},
     {  false,   false,   UV_REQ,      false, false,   kNonsense, UNSPECIFIED},
     {  false,   false,   UV_REQ,      false, true,    kOk,       NONE},
     {  false,   false,   UV_REQ,      true,  false,   kNonsense, UNSPECIFIED},
@@ -3594,6 +3596,10 @@ TEST_F(ResidentKeyAuthenticatorImplTest, CredProtectRegistration) {
     // authenticator support is irrelevant. Therefore these are just the non-
     // kNonsense cases from the prior block.
     {  true,    false,   UNSPECIFIED, false, false,   kOk,       NONE},
+    {  true,    false,   UV_OR_CRED,  false, false,   kOk,       UV_OR_CRED},
+    {  true,    false,   UV_OR_CRED,  true,  false,   kOk,       UV_OR_CRED},
+    {  true,    false,   UV_OR_CRED,  false, true,    kOk,       UV_OR_CRED},
+    {  true,    false,   UV_OR_CRED,  true,  true,    kOk,       UV_OR_CRED},
     {  true,    false,   UV_REQ,      false, true,    kOk,       UV_REQ},
     {  true,    false,   UV_REQ,      true,  true,    kOk,       UV_REQ},
     {  true,    true,    UNSPECIFIED, false, false,   kOk,       UV_OR_CRED},