Skip to content

Commit

Permalink
Merge PR #134
Browse files Browse the repository at this point in the history
  • Loading branch information
@DennisDyallo
DennisDyallo authored Aug 7, 2024
2 parents ffcd85b + 7f1adc6 commit 7fa5480
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 4 deletions.
20 changes: 18 additions & 2 deletions Yubico.YubiKey/src/Yubico/YubiKey/Fido2/MakeCredentialParameters.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,15 +631,21 @@ public void AddHmacSecretExtension(AuthenticatorInfo authenticatorInfo)
/// <param name="authenticatorInfo">
/// The FIDO2 <c>AuthenticatorInfo</c> for the YubiKey being used.
/// </param>
/// <param name="enforceCredProtectPolicy">
/// Determines the behavior taken when the authenticator does not support the
/// requested credProtect extension. Throws NotSupportedException when true, returns
/// silently without adding the extension when false.
/// </param>
/// <exception cref="ArgumentNullException">
/// The <c>authenticatorInfo</c> arg is null.
/// </exception>
/// <exception cref="ArgumentException">
/// <exception cref="NotSupportedException">
/// The YubiKey does not support this extension, or the input values were
/// not correct.
/// </exception>
public void AddCredProtectExtension(
CredProtectPolicy credProtectPolicy,
bool enforceCredProtectPolicy,
AuthenticatorInfo authenticatorInfo)
{
if (credProtectPolicy == CredProtectPolicy.None)
Expand All @@ -652,7 +658,12 @@ public void AddCredProtectExtension(
}
if (!authenticatorInfo.Extensions.Contains<string>(KeyCredProtect))
{
throw new NotSupportedException(ExceptionMessages.NotSupportedByYubiKeyVersion);
if (enforceCredProtectPolicy && credProtectPolicy != CredProtectPolicy.UserVerificationOptional)
{
throw new NotSupportedException(ExceptionMessages.NotSupportedByYubiKeyVersion);
}

return;
}

// The encoding is key/value where the key is "credProtect" and the
Expand All @@ -661,6 +672,11 @@ public void AddCredProtectExtension(
AddExtension(KeyCredProtect, new byte[] { (byte)credProtectPolicy });
}

/// <inheritdoc cref="AddCredProtectExtension(CredProtectPolicy,bool,AuthenticatorInfo)"/>
public void AddCredProtectExtension(CredProtectPolicy credProtectPolicy,
AuthenticatorInfo authenticatorInfo) =>
AddCredProtectExtension(credProtectPolicy, true, authenticatorInfo);

/// <summary>
/// Add an entry to the list of options.
/// </summary>
Expand Down
112 changes: 110 additions & 2 deletions Yubico.YubiKey/tests/integration/Yubico/YubiKey/Fido2/MakeCredentialGetAssertionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,9 @@ public void MakeCredential_MultipleCredentials_GetAssertion_ReturnsMultipleAsser
fido2.KeyCollector = KeyCollector;
int startCount = (int)fido2.AuthenticatorInfo.RemainingDiscoverableCredentials!; //RemainingDiscoverableCredentials is NULL on my two keys I tried with (USBA 5.4.3 Keychain and Nano)

// Verify the PIN
fido2.VerifyPin(); //Never completes on my 5.7
// Fido app was reset above, so set and confirm a pin (hardcoded in KeyCollector)
fido2.SetPin();
fido2.VerifyPin();

// Call MakeCredential
var user1 = new UserEntity(new byte[] { 1, 2, 3, 4 })
Expand Down Expand Up @@ -200,6 +201,112 @@ public void MakeCredential_MultipleCredentials_GetAssertion_ReturnsMultipleAsser
}
}


// This test requires user to touch the device.
[Theory, Trait("Category", "RequiresTouch")]
[InlineData(CredProtectPolicy.UserVerificationOptional, true)]
[InlineData(CredProtectPolicy.UserVerificationOptional, false)]
[InlineData(CredProtectPolicy.UserVerificationRequired, true)]
[InlineData(CredProtectPolicy.UserVerificationRequired, false)]
public void AddCredProtectExtension_KeySupportsCredProtectExtension(
CredProtectPolicy credProtectPolicy,
bool enforceCredProtectPolicy)
{
// Could have combined expectExtensionSupported cases under a single Theory, but
// expectExtensionSupported true vs false requires using YubiKeys with different
// firmware, and using separate Theory methods makes it somewhat easier to run them
// as separate, grouped sets
TestAddCredProtectExtension(expectExtensionSupported: true, credProtectPolicy, enforceCredProtectPolicy);
}

[Theory, Trait("Category", "RequiresTouch")]
[InlineData(CredProtectPolicy.UserVerificationOptional, true)]
[InlineData(CredProtectPolicy.UserVerificationOptional, false)]
[InlineData(CredProtectPolicy.UserVerificationRequired, true)]
[InlineData(CredProtectPolicy.UserVerificationRequired, false)]
[InlineData(CredProtectPolicy.UserVerificationOptionalWithCredentialIDList, true)]
public void AddCredProtectExtension_KeyDoesNotSupportCredProtectExtension(
CredProtectPolicy credProtectPolicy,
bool enforceCredProtectPolicy)
{
// Could have combined expectExtensionSupported cases under a single Theory, but
// expectExtensionSupported true vs false requires using YubiKeys with different
// firmware, and using separate Theory methods makes it somewhat easier to run them
// as distinct, grouped sets
TestAddCredProtectExtension(expectExtensionSupported: false, credProtectPolicy, enforceCredProtectPolicy);
}

private void TestAddCredProtectExtension(
bool expectExtensionSupported,
CredProtectPolicy credProtectPolicy,
bool enforceCredProtectPolicy)
{
IYubiKeyDevice yubiKeyDevice = YubiKeyDevice.FindByTransport(Transport.HidFido).First();

bool isValid = Fido2ResetForTest.DoReset(yubiKeyDevice.SerialNumber);
Assert.True(isValid);

using (var fido2Session = new Fido2Session(yubiKeyDevice))
{
// Set up a key collector
fido2Session.KeyCollector = KeyCollector;

// Fido app was reset above, so set and confirm a pin (hardcoded in KeyCollector)
fido2Session.SetPin();
fido2Session.VerifyPin();

// Note that Name is a required value
var user = new UserEntity(new byte[] { 1, 2, 3, 4 })
{
Name = "Name",
DisplayName = "DisplayName",
};

var mcParams = new MakeCredentialParameters(_rp, user)
{
ClientDataHash = _clientDataHash
};

var isExtensionSupported = fido2Session.AuthenticatorInfo.Extensions?.Contains("credProtect") ?? false;

// If this fails, the yubikey used doesn't have the proper support level expected for the test
// For expectExtensionSupported==true, the key must have FW 5.2.0 or later
// For expectExtensionSupported==false, the key must have FW before 5.2.0
Assert.Equal(expectExtensionSupported, isExtensionSupported);

// Act
try
{
mcParams.AddCredProtectExtension(
credProtectPolicy,
enforceCredProtectPolicy,
fido2Session.AuthenticatorInfo!);
}
catch (NotSupportedException)
{
// This shouldn't fail for Optional, even if the key doesn't support the
// extension, so ensure that's not what was set
Assert.NotEqual(CredProtectPolicy.UserVerificationOptional, credProtectPolicy);
Assert.False(isExtensionSupported);
return;
}

// Verify
// The call to set the extension should always succeed under any of these conditions
Assert.True(
!enforceCredProtectPolicy
|| isExtensionSupported
|| credProtectPolicy == CredProtectPolicy.UserVerificationOptional);

Assert.Equal(expectExtensionSupported, mcParams.Extensions?.ContainsKey("credProtect") ?? false);

MakeCredentialData mcData = fido2Session.MakeCredential(mcParams);

CredProtectPolicy cpPolicy = mcData.AuthenticatorData.GetCredProtectExtension();
Assert.Equal(expectExtensionSupported ? credProtectPolicy : CredProtectPolicy.None, cpPolicy);
}
}

private bool KeyCollector(KeyEntryData arg)
{
switch (arg.Request)
Expand All @@ -208,6 +315,7 @@ private bool KeyCollector(KeyEntryData arg)
Console.WriteLine("YubiKey requires touch");
break;
case KeyEntryRequest.VerifyFido2Pin:
case KeyEntryRequest.SetFido2Pin:
arg.SubmitValue(Encoding.UTF8.GetBytes("123456"));
break;
case KeyEntryRequest.VerifyFido2Uv:
Expand Down

0 comments on commit 7fa5480

Please sign in to comment.