Skip to content

Commit

Permalink
removed assumption in commands and response objects as well
Browse files Browse the repository at this point in the history
  • Loading branch information
@DennisDyallo
DennisDyallo committed Dec 17, 2024
1 parent f02207c commit 8994a2f
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 81 deletions.
17 changes: 13 additions & 4 deletions Yubico.YubiKey/src/Yubico/YubiKey/Piv/Commands/InitializeAuthenticateManagementKeyCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using Yubico.Core.Iso7816;

namespace Yubico.YubiKey.Piv.Commands
Expand Down Expand Up @@ -306,18 +307,25 @@ public sealed class InitializeAuthenticateManagementKeyCommand : IYubiKeyCommand
/// </value>
public YubiKeyApplication Application => YubiKeyApplication.Piv;


[Obsolete("This constructor is deprecated. Users must specify management key algorithm type, as it cannot be assumed.")]
public InitializeAuthenticateManagementKeyCommand()
: this(true)
{
}

/// <summary>
/// Initializes a new instance of the InitializeAuthenticateManagementKeyCommand class for
/// Mutual Authentication, and a Triple-DES management key.
/// Mutual Authentication.
/// </summary>
/// <remarks>
/// Using this constructor is equivalent to
/// <code language="csharp">
/// new InitializeAuthenticateManagementKeyCommand(true);
/// new InitializeAuthenticateManagementKeyCommand(true, PivAlgorithm.AES192);
/// </code>
/// </remarks>
public InitializeAuthenticateManagementKeyCommand()
: this(true)
public InitializeAuthenticateManagementKeyCommand(PivAlgorithm algorithm)
: this(true, algorithm)
{
}

Expand All @@ -335,6 +343,7 @@ public InitializeAuthenticateManagementKeyCommand()
/// <param name="mutualAuthentication">
/// <c>True</c> for mutual authentication, <c>false</c> for single.
/// </param>
[Obsolete("This constructor is deprecated. Users must specify management key algorithm type, as it cannot be assumed.")]
public InitializeAuthenticateManagementKeyCommand(bool mutualAuthentication)
: this(mutualAuthentication, PivAlgorithm.TripleDes)
{
Expand Down
15 changes: 3 additions & 12 deletions ...co.YubiKey/src/Yubico/YubiKey/Piv/Commands/InitializeAuthenticateManagementKeyResponse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,7 @@ public sealed class InitializeAuthenticateManagementKeyResponse : PivResponse, I
// ResponseApdu.Data. It will be 8 bytes.
private readonly byte[]? _clientAuthenticationChallenge;

/// <summary>
/// Constructs an InitializeAuthenticateManagementKeyResponse based on a ResponseApdu
/// received from the YubiKey for the Triple-DES algorithm.
/// </summary>
/// <param name="responseApdu">
/// The object containing the Response APDU<br/>returned by the YubiKey.
/// </param>
/// <exception cref="MalformedYubiKeyResponseException">
/// Thrown when the data provided does not meet the expectations, and cannot be parsed.
/// </exception>
[Obsolete("This constructor is deprecated. Users must specify management key algorithm type, as it cannot be assumed.")]
public InitializeAuthenticateManagementKeyResponse(ResponseApdu responseApdu)
: this(responseApdu, PivAlgorithm.TripleDes)
{
Expand All @@ -90,8 +81,8 @@ public InitializeAuthenticateManagementKeyResponse(ResponseApdu responseApdu)
/// <exception cref="MalformedYubiKeyResponseException">
/// Thrown when the data provided does not meet the expectations, and cannot be parsed.
/// </exception>
public InitializeAuthenticateManagementKeyResponse(ResponseApdu responseApdu, PivAlgorithm algorithm) :
base(responseApdu)
public InitializeAuthenticateManagementKeyResponse(ResponseApdu responseApdu, PivAlgorithm algorithm)
: base(responseApdu)
{
Algorithm = algorithm;

Expand Down
49 changes: 16 additions & 33 deletions Yubico.YubiKey/src/Yubico/YubiKey/Piv/Commands/SetManagementKeyCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ private SetManagementKeyCommand()
throw new NotImplementedException();
}

[Obsolete("This constructor is deprecated. Users must specify management key algorithm type, as it cannot be assumed.")]
public SetManagementKeyCommand(ReadOnlyMemory<byte> newKey)
: this(newKey, PivTouchPolicy.Default, PivAlgorithm.TripleDes)
{
}

/// <summary>
/// Initializes a new instance of the <c>SetManagementKeyCommand</c> class.
/// This command takes the new management key as input and will set the
Expand All @@ -176,11 +182,9 @@ private SetManagementKeyCommand()
/// };
/// </code>
/// <para>
/// The default algorithm is <c>TripleDes</c>. If you do not set the
/// <c>Algorithm</c> property after instantiating with this constructor,
/// the SDK will expect the key to be TripleDES. Valid algorithms are
/// <c>PivAlgorithm.TripleDes</c>, <c>PivAlgorithm.Aes128</c>,
/// <c>PivAlgorithm.Aes192</c>, and <c>PivAlgorithm.Aes256</c>.
/// Valid algorithms are <c>PivAlgorithm.TripleDes</c>,
/// <c>PivAlgorithm.Aes128</c>, <c>PivAlgorithm.Aes192</c>, and
/// <c>PivAlgorithm.Aes256</c>. FIPS YubiKeys versions 5.7 and greater require <c>PivAlgorithm.Aes192</c>.
/// </para>
/// <para>
/// Note that you need to authenticate the current PIV management key before
Expand All @@ -190,36 +194,15 @@ private SetManagementKeyCommand()
/// <param name="newKey">
/// The bytes that make up the new management key.
/// </param>
public SetManagementKeyCommand(ReadOnlyMemory<byte> newKey)
: this(newKey, PivTouchPolicy.Default, PivAlgorithm.TripleDes)
/// <param name="algorithm">
/// The algorithm of the new management key.
/// </param>
public SetManagementKeyCommand(ReadOnlyMemory<byte> newKey, PivAlgorithm algorithm)
: this(newKey, PivTouchPolicy.Default, algorithm)
{
}

/// <summary>
/// Initializes a new instance of the SetManagementKeyCommand class. This
/// command takes the new management key and the touch policy as input.
/// </summary>
/// <remarks>
/// Note that a <c>touchPolicy</c> of <c>PivTouchPolicy.Default</c> or
/// <c>None</c> is equivalent to <c>Never</c>.
/// <para>
/// The default algorithm is <c>TripleDes</c>. If you do not set the
/// <c>Algorithm</c> property after instantiating with this constructor,
/// the SDK will expect the key to be TripleDES. Valid algorithms are
/// <c>PivAlgorithm.TripleDes</c>, <c>PivAlgorithm.Aes128</c>,
/// <c>PivAlgorithm.Aes192</c>, and <c>PivAlgorithm.Aes256</c>.
/// </para>
/// <para>
/// Note also that you need to authenticate the current PIV management
/// key before setting it to a new value with this command.
/// </para>
/// </remarks>
/// <param name="newKey">
/// The bytes that make up the new management key.
/// </param>
/// <param name="touchPolicy">
/// The touch policy for the management key.
/// </param>
[Obsolete("This constructor is deprecated. Users must specify management key algorithm type, as it cannot be assumed.")]
public SetManagementKeyCommand(ReadOnlyMemory<byte> newKey, PivTouchPolicy touchPolicy)
: this(newKey, touchPolicy, PivAlgorithm.TripleDes)
{
Expand All @@ -236,7 +219,7 @@ public SetManagementKeyCommand(ReadOnlyMemory<byte> newKey, PivTouchPolicy touch
/// <para>
/// Valid algorithms are <c>PivAlgorithm.TripleDes</c>,
/// <c>PivAlgorithm.Aes128</c>, <c>PivAlgorithm.Aes192</c>, and
/// <c>PivAlgorithm.Aes256</c>,
/// <c>PivAlgorithm.Aes256</c>. FIPS YubiKeys versions 5.7 and greater require <c>PivAlgorithm.Aes192</c>.
/// </para>
/// <para>
/// Note also that you need to authenticate the current PIV management
Expand Down
11 changes: 4 additions & 7 deletions Yubico.YubiKey/tests/unit/Yubico/YubiKey/Piv/Commands/CompleteAuthMgmtKeyCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ public void CreateCommandApdu_GetInsProperty_ReturnsHex87(bool isMutual)
[Theory]
[InlineData(true)]
[InlineData(false)]
public void CreateCommandApdu_GetP1Property_ReturnsThree(bool isMutual)
public void CreateCommandApdu_GetP1Property_Returns10ForAes192(bool isMutual)
{
CommandApdu cmdApdu = GetCommandApdu(isMutual, true);

byte P1 = cmdApdu.P1;

Assert.Equal(3, P1);
Assert.Equal(10, P1);
}

[Theory]
Expand Down Expand Up @@ -257,10 +257,7 @@ private static CompleteAuthenticateManagementKeyCommand GetCommandObject(bool is
}
finally
{
if (!(replacement is null))
{
replacement.RestoreRandomProvider();
}
replacement?.RestoreRandomProvider();
}
}

Expand Down Expand Up @@ -332,7 +329,7 @@ private static InitializeAuthenticateManagementKeyResponse GetInitResponse(bool
0x7C, 0x0A, tag1, 0x08, 0x39, 0xA0, 0xA8, 0xE9, 0xF5, 0x28, 0x87, 0x75, sw1, sw2
});

return new InitializeAuthenticateManagementKeyResponse(responseApdu);
return new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.TripleDes);
}
}
}
12 changes: 6 additions & 6 deletions Yubico.YubiKey/tests/unit/Yubico/YubiKey/Piv/Commands/InitAuthMgmtKeyCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ public class InitializeAuthenticateManagementKeyCommandTests
[Fact]
public void ClassType_DerivedFromPivCommand_IsTrue()
{
var command = new InitializeAuthenticateManagementKeyCommand();
var command = new InitializeAuthenticateManagementKeyCommand(PivAlgorithm.Aes192);

Assert.True(command is IYubiKeyCommand<InitializeAuthenticateManagementKeyResponse>);
}

[Fact]
public void Constructor_Application_Piv()
{
var command = new InitializeAuthenticateManagementKeyCommand();
var command = new InitializeAuthenticateManagementKeyCommand(PivAlgorithm.Aes192);

YubiKeyApplication application = command.Application;

Expand Down Expand Up @@ -146,7 +146,7 @@ public void CreateResponseForApdu_ReturnsCorrectType()
byte sw2 = unchecked((byte)SWConstants.Success);
var responseApdu = new ResponseApdu(
new byte[] { 0x7C, 0x0A, 0x81, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });
var command = new InitializeAuthenticateManagementKeyCommand();
var command = new InitializeAuthenticateManagementKeyCommand(PivAlgorithm.Aes192);

InitializeAuthenticateManagementKeyResponse? response = command.CreateResponseForApdu(responseApdu);

Expand All @@ -164,9 +164,9 @@ private static CommandApdu GetInitAuthMgmtKeyCommandApdu(int constructor)
{
InitializeAuthenticateManagementKeyCommand command = constructor switch
{
0 => new InitializeAuthenticateManagementKeyCommand(false),
1 => new InitializeAuthenticateManagementKeyCommand(true),
_ => new InitializeAuthenticateManagementKeyCommand(),
0 => new InitializeAuthenticateManagementKeyCommand(false, PivAlgorithm.Aes192),
1 => new InitializeAuthenticateManagementKeyCommand(true, PivAlgorithm.Aes192),
_ => new InitializeAuthenticateManagementKeyCommand(PivAlgorithm.Aes192),
};

return command.CreateCommandApdu();
Expand Down
24 changes: 12 additions & 12 deletions Yubico.YubiKey/tests/unit/Yubico/YubiKey/Piv/Commands/InitAuthMgmtKeyResponseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class InitAuthMgmtKeyResponseTests
public void Constructor_GivenNullResponseApdu_ThrowsArgumentNullExceptionFromBase()
{
#pragma warning disable CS8625 // testing null input, disable warning that null is passed to non-nullable arg.
_ = Assert.Throws<ArgumentNullException>(() => new InitializeAuthenticateManagementKeyResponse(null));
_ = Assert.Throws<ArgumentNullException>(() => new InitializeAuthenticateManagementKeyResponse(null, PivAlgorithm.Aes192));
#pragma warning restore CS8625
}

Expand All @@ -39,7 +39,7 @@ public void Constructor_InvalidLength_CorrectException()
new byte[] { 0x7C, 0x09, 0x81, 0x07, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

_ = Assert.Throws<MalformedYubiKeyResponseException>(() =>
new InitializeAuthenticateManagementKeyResponse(responseApdu));
new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192));
}

[Fact]
Expand All @@ -51,7 +51,7 @@ public void Constructor_InvalidT0_CorrectException()
new byte[] { 0x78, 0x0A, 0x81, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

_ = Assert.Throws<MalformedYubiKeyResponseException>(() =>
new InitializeAuthenticateManagementKeyResponse(responseApdu));
new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192));
}

[Fact]
Expand All @@ -63,7 +63,7 @@ public void Constructor_InvalidT2_CorrectException()
new byte[] { 0x7C, 0x0A, 0x82, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

_ = Assert.Throws<MalformedYubiKeyResponseException>(() =>
new InitializeAuthenticateManagementKeyResponse(responseApdu));
new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192));
}

[Fact]
Expand All @@ -75,7 +75,7 @@ public void Constructor_InvalidL1_CorrectException()
new byte[] { 0x7C, 0x0A, 0x81, 0x07, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

_ = Assert.Throws<MalformedYubiKeyResponseException>(() =>
new InitializeAuthenticateManagementKeyResponse(responseApdu));
new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192));
}

[Fact]
Expand All @@ -86,7 +86,7 @@ public void Constructor_SuccessResponseApdu_SetsStatusWordCorrectly()
var responseApdu = new ResponseApdu(
new byte[] { 0x7C, 0x0A, 0x81, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

Assert.Equal(SWConstants.Success, response.StatusWord);
}
Expand All @@ -99,7 +99,7 @@ public void Constructor_SuccessResponseApdu_SetsStatusCorrectly()
var responseApdu = new ResponseApdu(
new byte[] { 0x7C, 0x0A, 0x81, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

Assert.Equal(ResponseStatus.Success, response.Status);
}
Expand All @@ -120,7 +120,7 @@ public void Constructor_SuccessResponseApdu_GetDataCorrectBool(bool isMutual)
var responseApdu = new ResponseApdu(
new byte[] { 0x7C, 0x0A, tag2, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

(bool isMutualAuth, ReadOnlyMemory<byte> clientAuthenticationChallenge) = response.GetData();

Expand All @@ -138,7 +138,7 @@ public void Constructor_SuccessResponseApdu_GetDataCorrectBytes()
var responseApdu = new ResponseApdu(
new byte[] { 0x7C, 0x0A, 0x81, 0x08, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

(bool isMutualAuth, ReadOnlyMemory<byte> clientAuthenticationChallenge) = response.GetData();

Expand All @@ -155,7 +155,7 @@ public void Constructor_FailResponseApdu_SetsStatusWordCorrectly()
byte sw2 = unchecked((byte)SWConstants.ConditionsNotSatisfied);
var responseApdu = new ResponseApdu(new byte[] { sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

Assert.Equal(SWConstants.ConditionsNotSatisfied, response.StatusWord);
}
Expand All @@ -167,7 +167,7 @@ public void Constructor_FailResponseApdu_SetsStatusCorrectly()
byte sw2 = unchecked((byte)SWConstants.ConditionsNotSatisfied);
var responseApdu = new ResponseApdu(new byte[] { sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

Assert.Equal(ResponseStatus.ConditionsNotSatisfied, response.Status);
}
Expand All @@ -179,7 +179,7 @@ public void Constructor_FailResponseApdu_ThrowOnGetData()
byte sw2 = unchecked((byte)SWConstants.ConditionsNotSatisfied);
var responseApdu = new ResponseApdu(new byte[] { sw1, sw2 });

var response = new InitializeAuthenticateManagementKeyResponse(responseApdu);
var response = new InitializeAuthenticateManagementKeyResponse(responseApdu, PivAlgorithm.Aes192);

_ = Assert.Throws<InvalidOperationException>(() => response.GetData());
}
Expand Down
14 changes: 7 additions & 7 deletions Yubico.YubiKey/tests/unit/Yubico/YubiKey/Piv/Commands/SetManagementKeyCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class SetManagementKeyCommandTests
public void ClassType_DerivedFromPivCommand_IsTrue()
{
byte[] mgmtKey = GetMgmtKeyArray();
var command = new SetManagementKeyCommand(mgmtKey, PivTouchPolicy.Always);
var command = new SetManagementKeyCommand(mgmtKey, PivTouchPolicy.Always, PivAlgorithm.TripleDes);

Assert.True(command is IYubiKeyCommand<SetManagementKeyResponse>);
}
Expand All @@ -33,7 +33,7 @@ public void ClassType_DerivedFromPivCommand_IsTrue()
public void Constructor_Application_Piv()
{
byte[] mgmtKey = GetMgmtKeyArray();
var command = new SetManagementKeyCommand(mgmtKey, PivTouchPolicy.Always);
var command = new SetManagementKeyCommand(mgmtKey, PivTouchPolicy.Always, PivAlgorithm.TripleDes);

YubiKeyApplication application = command.Application;

Expand All @@ -45,7 +45,7 @@ public void Constructor_Property_TouchPolicy()
{
byte[] mgmtKey = GetMgmtKeyArray();
PivTouchPolicy touchPolicy = PivTouchPolicy.Always;
var command = new SetManagementKeyCommand(mgmtKey, touchPolicy);
var command = new SetManagementKeyCommand(mgmtKey, touchPolicy, PivAlgorithm.TripleDes);

PivTouchPolicy getPolicy = command.TouchPolicy;

Expand Down Expand Up @@ -206,11 +206,11 @@ private static SetManagementKeyCommand GetCommandObject(int cStyle, PivTouchPoli
switch (cStyle)
{
default:
cmd = new SetManagementKeyCommand(mgmtKey, touchPolicy);
cmd = new SetManagementKeyCommand(mgmtKey, touchPolicy, PivAlgorithm.TripleDes);
break;

case 2:
cmd = new SetManagementKeyCommand(mgmtKey)
cmd = new SetManagementKeyCommand(mgmtKey, PivAlgorithm.TripleDes)
{
TouchPolicy = touchPolicy,
};
Expand All @@ -219,13 +219,13 @@ private static SetManagementKeyCommand GetCommandObject(int cStyle, PivTouchPoli

case 3:
#pragma warning disable IDE0017 // Specifically testing this construction
cmd = new SetManagementKeyCommand(mgmtKey);
cmd = new SetManagementKeyCommand(mgmtKey, PivAlgorithm.TripleDes);
cmd.TouchPolicy = touchPolicy;
break;
#pragma warning restore IDE0017

case 4:
cmd = new SetManagementKeyCommand(mgmtKey);
cmd = new SetManagementKeyCommand(mgmtKey, PivAlgorithm.TripleDes);
break;
}

Expand Down

0 comments on commit 8994a2f

Please sign in to comment.