Skip to content

Fixed incorrect management key algorithm used for FIPS in TryChangeManagementKey #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 18, 2024
Merged

Fixed incorrect management key algorithm used for FIPS in TryChangeManagementKey #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c323031
Fixes #160. Refactor PivSession and update tests for management key a…
DennisDyallo Nov 14, 2024
315a8e2
Merge branch 'develop' into dennisdyallo/bugfix/160-default-mgmtkey
DennisDyallo Nov 18, 2024
3a65cf1
Merge branch 'develop' into dennisdyallo/bugfix/160-default-mgmtkey
DennisDyallo Nov 18, 2024
4bb5448
Assume default management algorithm
DennisDyallo Nov 18, 2024
001f886
Merge branch 'develop' into dennisdyallo/bugfix/160-default-mgmtkey
DennisDyallo Nov 18, 2024
9de0750
Fixed tests, assume default Tdes version is lower than 5.7
DennisDyallo Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 85 additions & 63 deletions Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.ManagementKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ public sealed partial class PivSession : IDisposable
/// </remarks>
public AuthenticateManagementKeyResult ManagementKeyAuthenticationResult { get; private set; }

private PivAlgorithm DefaultManagementKeyAlgorithm =>
_yubiKeyDevice.HasFeature(YubiKeyFeature.PivAesManagementKey) &&
_yubiKeyDevice.FirmwareVersion >= FirmwareVersion.V5_7_0
? PivAlgorithm.Aes192
: PivAlgorithm.TripleDes;

/// <summary>
/// Try to authenticate the management key.
/// </summary>
Expand Down Expand Up @@ -531,7 +537,7 @@ public bool TryAuthenticateManagementKey(ReadOnlyMemory<byte> managementKey, boo
/// authenticated.
/// </exception>
public bool TryChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.Default) =>
TryChangeManagementKey(touchPolicy, PivAlgorithm.TripleDes);
TryChangeManagementKey(touchPolicy, DefaultManagementKeyAlgorithm);

/// <summary>
/// Try to change the management key. The new key will be the specified
Expand Down Expand Up @@ -651,7 +657,8 @@ public bool TryChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.D
/// </exception>
public bool TryChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyAlgorithm)
{
_log.LogInformation("Try to change the management key, touch policy = {TouchPolicy}, algorithm = {PivALgorithm}.",
_log.LogInformation(
"Try to change the management key, touch policy = {TouchPolicy}, algorithm = {PivALgorithm}.",
touchPolicy.ToString(), newKeyAlgorithm.ToString());

CheckManagementKeyAlgorithm(newKeyAlgorithm, true);
Expand Down Expand Up @@ -726,7 +733,7 @@ public bool TryChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newK
/// authenticated.
/// </exception>
public void ChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.Default) =>
ChangeManagementKey(touchPolicy, PivAlgorithm.TripleDes);
ChangeManagementKey(touchPolicy, DefaultManagementKeyAlgorithm);

/// <summary>
/// Change the management key, throw an exception if the user cancels.
Expand Down Expand Up @@ -761,7 +768,8 @@ public void ChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.Defa
/// </exception>
public void ChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyAlgorithm)
{
_log.LogInformation("Change the management key, touch policy = {TouchPolicy}, algorithm = {PivAlgorithm}.",
_log.LogInformation(
"Change the management key, touch policy = {TouchPolicy}, algorithm = {PivAlgorithm}.",
touchPolicy.ToString(), newKeyAlgorithm.ToString());

if (TryChangeManagementKey(touchPolicy, newKeyAlgorithm) == false)
Expand Down Expand Up @@ -827,7 +835,7 @@ public void ChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyA
public bool TryChangeManagementKey(ReadOnlyMemory<byte> currentKey,
ReadOnlyMemory<byte> newKey,
PivTouchPolicy touchPolicy = PivTouchPolicy.Default) =>
TryChangeManagementKey(currentKey, newKey, touchPolicy, PivAlgorithm.TripleDes);
TryChangeManagementKey(currentKey, newKey, touchPolicy, DefaultManagementKeyAlgorithm);

/// <summary>
/// Try to change the management key. This method will use the
Expand Down Expand Up @@ -915,68 +923,11 @@ private bool TryForcedChangeManagementKey(ReadOnlyMemory<byte> currentKey,
}

_log.LogInformation($"Failed to set management key. Message: {response.StatusMessage}");

}

return false;
}

// Verify that and that the given algorithm is allowed.
// If checkMode is true, also check that the PIN-only mode is None.
// This is called by methods that set PIN-only mode or change the mgmt
// key.
// The algorithm can only be 3DES or AES, and it can only be AES if the
// YubiKey is 5.4.2 or later.
// It is not allowed to change the mgmt key if it is PIN-only, so those
// methods that change, will check the mode as well (they will pass true
// as the checkMode arg).
// If setting PIN-only, then the mode is not an issue, so don't check
// (pass false as the checkMode arg).
// If everything is fine, return, otherwise throw an exception.
private void CheckManagementKeyAlgorithm(PivAlgorithm algorithm, bool checkMode)
{
if (checkMode)
{
var pinOnlyMode = GetPinOnlyMode();
if (pinOnlyMode.HasFlag(PivPinOnlyMode.PinProtected) ||
pinOnlyMode.HasFlag(PivPinOnlyMode.PinDerived))
{
throw new InvalidOperationException(
string.Format(
CultureInfo.CurrentCulture,
ExceptionMessages.MgmtKeyCannotBeChanged));
}
}

bool isValid = false;

switch (algorithm)
{
case PivAlgorithm.TripleDes:
isValid = true;

break;

case PivAlgorithm.Aes128:
case PivAlgorithm.Aes192:
case PivAlgorithm.Aes256:
isValid = _yubiKeyDevice.HasFeature(YubiKeyFeature.PivAesManagementKey);

break;

default:
break;
}

if (!isValid)
{
throw new ArgumentException(
string.Format(
CultureInfo.CurrentCulture,
ExceptionMessages.UnsupportedAlgorithm));
}
}

// This is the actual Try code, shared by both TryAuth and TryChange.
// The caller provides a KeyEntryData object set with the appropriate
// request:
Expand Down Expand Up @@ -1046,7 +997,8 @@ private bool TryAuthenticateManagementKey(bool mutualAuthentication,
// off-card app authenticated, but the YubiKey itself did
// not.
// If case (3), throw an exception.
if (ManagementKeyAuthenticationResult == AuthenticateManagementKeyResult.MutualYubiKeyAuthenticationFailed)
if (ManagementKeyAuthenticationResult ==
AuthenticateManagementKeyResult.MutualYubiKeyAuthenticationFailed)
{
throw new SecurityException(
string.Format(
Expand All @@ -1061,5 +1013,75 @@ private bool TryAuthenticateManagementKey(bool mutualAuthentication,

return ManagementKeyAuthenticated;
}

private void RefreshManagementKeyAlgorithm() => ManagementKeyAlgorithm = GetManagementKeyAlgorithm();

private PivAlgorithm GetManagementKeyAlgorithm()
{
if (!_yubiKeyDevice.HasFeature(YubiKeyFeature.PivMetadata))
{
// Assume default for version
return DefaultManagementKeyAlgorithm;
}

// Get current ManagementKeyAlgorithm from Yubikey metadata
var response = Connection.SendCommand(new GetMetadataCommand(PivSlot.Management));
if (response.Status != ResponseStatus.Success)
{
throw new InvalidOperationException(response.StatusMessage);
}

var metadata = response.GetData();
return metadata.Algorithm;
}

// Verify that and that the given algorithm is allowed.
// If checkMode is true, also check that the PIN-only mode is None.
// This is called by methods that set PIN-only mode or change the mgmt
// key.
// The algorithm can only be 3DES or AES, and it can only be AES if the
// YubiKey is 5.4.2 or later.
// It is not allowed to change the mgmt key if it is PIN-only, so those
// methods that change, will check the mode as well (they will pass true
// as the checkMode arg).
// If setting PIN-only, then the mode is not an issue, so don't check
// (pass false as the checkMode arg).
// If everything is fine, return, otherwise throw an exception.
private void CheckManagementKeyAlgorithm(PivAlgorithm algorithm, bool checkMode)
{
if (checkMode)
{
var pinOnlyMode = GetPinOnlyMode();
if (pinOnlyMode.HasFlag(PivPinOnlyMode.PinProtected) ||
pinOnlyMode.HasFlag(PivPinOnlyMode.PinDerived))
{
throw new InvalidOperationException(
string.Format(
CultureInfo.CurrentCulture,
ExceptionMessages.MgmtKeyCannotBeChanged));
}
}

bool isValid = IsValid(algorithm);
if (!isValid)
{
throw new ArgumentException(
string.Format(
CultureInfo.CurrentCulture,
ExceptionMessages.UnsupportedAlgorithm));
}

return;

bool IsValid(PivAlgorithm pa) =>
pa switch
{
PivAlgorithm.TripleDes => true, // Default for keys below fw version 5.7
PivAlgorithm.Aes128 => _yubiKeyDevice.HasFeature(YubiKeyFeature.PivAesManagementKey),
PivAlgorithm.Aes192 => _yubiKeyDevice.HasFeature(YubiKeyFeature.PivAesManagementKey),
PivAlgorithm.Aes256 => _yubiKeyDevice.HasFeature(YubiKeyFeature.PivAesManagementKey),
_ => false
};
}
}
}
36 changes: 9 additions & 27 deletions Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,14 @@ private PivSession(StaticKeys? scp03Keys, IYubiKeyDevice yubiKey)
throw new ArgumentNullException(nameof(yubiKey));
}

_yubiKeyDevice = yubiKey;

Connection = scp03Keys is null
? yubiKey.Connect(YubiKeyApplication.Piv)
: yubiKey.ConnectScp03(YubiKeyApplication.Piv, scp03Keys);
? _yubiKeyDevice.Connect(YubiKeyApplication.Piv)
: _yubiKeyDevice.ConnectScp03(YubiKeyApplication.Piv, scp03Keys);

ResetAuthenticationStatus();
UpdateManagementKey(yubiKey);

_yubiKeyDevice = yubiKey;
_disposed = false;
RefreshManagementKeyAlgorithm();
}

/// <summary>
Expand Down Expand Up @@ -313,14 +312,14 @@ public void Dispose()
{
_ = Connection.SendCommand(new SelectApplicationCommand(YubiKeyApplication.Management));
}
#pragma warning disable CA1031
#pragma warning disable CA1031
catch (Exception e)
#pragma warning restore CA1031
#pragma warning restore CA1031
{
string message = string.Format(
CultureInfo.CurrentCulture,
ExceptionMessages.PivSessionDisposeUnknownError, e.GetType(), e.Message);

// Example:
// Exception caught when disposing PivSession: Yubico.PlatformInterop.SCardException,
// Unable to begin a transaction with the given smart card. SCARD_E_SERVICE_STOPPED: The smart card resource manager has shut down.
Expand Down Expand Up @@ -510,7 +509,7 @@ public void ResetApplication()
// As resetting the PIV application resets the management key,
// the management key must be updated to account for the case when the previous management key type
// was not the default key type.
UpdateManagementKey(_yubiKeyDevice);
RefreshManagementKeyAlgorithm();
}

/// <summary>
Expand Down Expand Up @@ -671,22 +670,5 @@ private void TryBlock(byte slot)
CultureInfo.CurrentCulture,
ExceptionMessages.ApplicationResetFailure));
}

private void UpdateManagementKey(IYubiKeyDevice yubiKey) =>
ManagementKeyAlgorithm = yubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey)
? GetManagementKeyAlgorithm()
: PivAlgorithm.TripleDes; // Default for keys with firmware version < 5.7

private PivAlgorithm GetManagementKeyAlgorithm()
{
var response = Connection.SendCommand(new GetMetadataCommand(PivSlot.Management));
if (response.Status != ResponseStatus.Success)
{
throw new InvalidOperationException(response.StatusMessage);
}

var metadata = response.GetData();
return metadata.Algorithm;
}
}
}
Loading
Loading