Skip to content

Commit 3bcf999

Browse files
DennisDyalloDennisDyallo
authored
Merge pull request #231 from Yubico/dennisdyallo/1.13-fixes
fix: Fixed bug importing PIV private key in legacy classes
2 parents 41da641 + a4b897f commit 3bcf999

31 files changed

+223
-122
lines changed

‎Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/AsnPrivateKeyEncoder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ private static byte[] EncodeECKey(
190190
writer.PopSequence();
191191

192192
// PrivateKey as OCTET STRING
193-
writer.WriteOctetString(ecPrivateKeyHandle.Data);
193+
writer.WriteOctetString(ecPrivateKeyHandle.Data.Span);
194194
writer.PopSequence();
195195

196196
return writer.Encode();
@@ -237,7 +237,7 @@ private static byte[] EncodeCurve25519Key(ReadOnlySpan<byte> privateKey, string
237237
privateKeyWriter.WriteOctetString(privateKey);
238238

239239
using var privateKeyBytesHandle = new ZeroingMemoryHandle(privateKeyWriter.Encode());
240-
writer.WriteOctetString(privateKeyBytesHandle.Data);
240+
writer.WriteOctetString(privateKeyBytesHandle.Data.Span);
241241

242242
// End PrivateKeyInfo SEQUENCE
243243
writer.PopSequence();

‎Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/EcdsaVerify.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ public EcdsaVerify(ECDsa ecdsa)
158158
/// <exception cref="ArgumentException">
159159
/// The key is not for a supported algorithm or curve, or is malformed.
160160
/// </exception>
161+
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey PivRsaPublic/PivRsaPrivateKey is deprecated. Use implementations of ECPublicKey, ECPrivateKey and RSAPublicKey, RSAPrivateKey instead", false)]
161162
public EcdsaVerify(PivPublicKey pivPublicKey)
162163
{
163164
if (pivPublicKey is null)
@@ -171,6 +172,21 @@ public EcdsaVerify(PivPublicKey pivPublicKey)
171172

172173
ECDsa = ConvertPublicKey(publicPointSpan.ToArray());
173174
}
175+
176+
public EcdsaVerify(ECPublicKey publicKey)
177+
{
178+
if (publicKey is null)
179+
{
180+
throw new ArgumentNullException(nameof(publicKey));
181+
}
182+
183+
if (!publicKey.KeyType.IsECDsa())
184+
{
185+
throw new ArgumentException("Invalid key type", nameof(publicKey));
186+
}
187+
188+
ECDsa = ConvertPublicKey(publicKey.PublicPoint);
189+
}
174190

175191
/// <summary>
176192
/// Create an instance of the <see cref="EcdsaVerify"/> class using the
@@ -331,7 +347,8 @@ public bool VerifyDigestedData(
331347

332348
private static ECDsa ConvertPublicKey(ReadOnlyMemory<byte> encodedEccPoint)
333349
{
334-
int minEncodedPointLength = (KeyDefinitions.P256.LengthInBytes * 2) + 1; // This is the minimum length for an encoded point on P-256 (0x04 || x || y)
350+
// This is the minimum length for an encoded point on P-256 (0x04 || x || y)
351+
int minEncodedPointLength = (KeyDefinitions.P256.LengthInBytes * 2) + 1;
335352
if (encodedEccPoint.Length < minEncodedPointLength || encodedEccPoint.Span[0] != EncodedPointTag)
336353
{
337354
throw new ArgumentException(ExceptionMessages.UnsupportedAlgorithm);

‎Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/ZeroingMemoryHandle.cs

Lines changed: 8 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -13,76 +13,34 @@
1313
// limitations under the License.
1414

1515
using System;
16-
using System.Collections;
17-
using System.Collections.Generic;
1816
using System.Security.Cryptography;
1917

2018
namespace Yubico.YubiKey.Cryptography;
2119

22-
#pragma warning disable CA1710
23-
internal class ZeroingMemoryHandle : IDisposable, IReadOnlyCollection<byte>, IEnumerable<byte>
24-
#pragma warning restore CA1710
20+
internal class ZeroingMemoryHandle : IDisposable
2521
{
26-
private readonly byte[] _data;
22+
private readonly Memory<byte> _data;
2723
private bool _disposed;
28-
2924
public int Length => _disposed ? 0 : _data.Length;
30-
public int Count => Length; // For IReadOnlyCollection
31-
32-
public byte this[int index] => _disposed
33-
? throw new ObjectDisposedException(nameof(ZeroingMemoryHandle))
34-
: _data[index];
25+
public int Count => Length;
3526

36-
public ZeroingMemoryHandle(byte[] data)
27+
public ZeroingMemoryHandle(Memory<byte> data)
3728
{
38-
_data = data ?? throw new ArgumentNullException(nameof(data));
29+
_data = data;
3930
}
4031

41-
public ReadOnlySpan<byte> AsSpan() => _disposed
42-
? ReadOnlySpan<byte>.Empty
43-
: _data.AsSpan();
44-
45-
public byte[] Data => _disposed
32+
public Memory<byte> Data => _disposed
4633
? throw new ObjectDisposedException(nameof(ZeroingMemoryHandle))
4734
: _data;
48-
49-
public void CopyTo(byte[] destination, int destinationIndex = 0)
50-
{
51-
if (_disposed)
52-
{
53-
throw new ObjectDisposedException(nameof(ZeroingMemoryHandle));
54-
}
55-
56-
Buffer.BlockCopy(_data, 0, destination, destinationIndex, _data.Length);
57-
}
58-
59-
public ReadOnlySpan<byte> Slice(int start, int length) => _disposed
60-
? ReadOnlySpan<byte>.Empty
61-
: _data.AsSpan(start, length);
62-
63-
public IEnumerator<byte> GetEnumerator()
64-
{
65-
if (_disposed)
66-
{
67-
throw new ObjectDisposedException(nameof(ZeroingMemoryHandle));
68-
}
69-
70-
for (int i = 0; i < _data.Length; i++)
71-
{
72-
yield return _data[i];
73-
}
74-
}
75-
76-
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
77-
35+
7836
public void Dispose()
7937
{
8038
if (_disposed)
8139
{
8240
return;
8341
}
8442

85-
CryptographicOperations.ZeroMemory(_data);
43+
CryptographicOperations.ZeroMemory(_data.Span);
8644
_disposed = true;
8745
GC.SuppressFinalize(this);
8846
}

‎Yubico.YubiKey/src/Yubico/YubiKey/Piv/Commands/GenerateKeyPairResponse.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ namespace Yubico.YubiKey.Piv.Commands
8585
/// PivPublicKey pubKey = generateKeyPairResponse.GetData();
8686
/// </code>
8787
/// </remarks>
88+
#pragma warning disable CS0618 // Type or member is obsolete
8889
public class GenerateKeyPairResponse : PivResponse, IYubiKeyResponseWithData<PivPublicKey>
90+
#pragma warning restore CS0618 // Type or member is obsolete
8991
{
9092
private byte _slotNumber;
9193
private PivAlgorithm _algorithm;
@@ -190,11 +192,14 @@ public GenerateKeyPairResponse(
190192
/// <exception cref="InvalidOperationException">
191193
/// Thrown when <see cref="YubiKeyResponse.Status"/> is not <see cref="ResponseStatus.Success"/>.
192194
/// </exception>
195+
#pragma warning disable CS0618 // Type or member is obsolete
193196
public PivPublicKey GetData() =>
194197
Status switch
195198
{
196199
ResponseStatus.Success => PivPublicKey.Create(ResponseApdu.Data, Algorithm),
197200
_ => throw new InvalidOperationException(StatusMessage),
198201
};
202+
#pragma warning restore CS0618 // Type or member is obsolete
203+
199204
}
200205
}

‎Yubico.YubiKey/src/Yubico/YubiKey/Piv/Commands/ImportAsymmetricKeyCommand.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ private ImportAsymmetricKeyCommand()
215215
/// <exception cref="ArgumentException">
216216
/// The <c>privateKey</c> argument does not contain a key.
217217
/// </exception>
218-
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey is deprecated. Use IPublicKey, IPrivateKey instead", false)]
218+
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey, PivRsaPublic/PivRsaPrivateKey is deprecated. Use implementations of ECPublicKey, ECPrivateKey and RSAPublicKey, RSAPrivateKey instead", false)]
219+
219220
public ImportAsymmetricKeyCommand(
220221
PivPrivateKey privateKey,
221222
byte slotNumber,
@@ -274,7 +275,8 @@ public ImportAsymmetricKeyCommand(
274275
/// <exception cref="ArgumentException">
275276
/// The <c>privateKey</c> argument does not contain a key.
276277
/// </exception>
277-
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey is deprecated. Use IPublicKey, IPrivateKey instead", false)]
278+
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey, PivRsaPublic/PivRsaPrivateKey is deprecated. Use implementations of ECPublicKey, ECPrivateKey and RSAPublicKey, RSAPrivateKey instead", false)]
279+
278280
public ImportAsymmetricKeyCommand(PivPrivateKey privateKey)
279281
{
280282
if (privateKey is null)

‎Yubico.YubiKey/src/Yubico/YubiKey/Piv/Converters/KeyExtensions.cs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,38 @@ public static class KeyExtensions
3333
/// <param name="parameters">The public key parameters to encode.</param>
3434
/// <returns>A BER encoded byte array containing the encoded public key.</returns>
3535
/// <exception cref="ArgumentException">Thrown when the key type is not supported.</exception>
36-
public static Memory<byte> EncodeAsPiv(this IPublicKey parameters)
37-
{
38-
return parameters switch
36+
public static Memory<byte> EncodeAsPiv(this IPublicKey parameters) =>
37+
parameters switch
3938
{
4039
ECPublicKey p => PivKeyEncoder.EncodeECPublicKey(p),
4140
RSAPublicKey p => PivKeyEncoder.EncodeRSAPublicKey(p),
4241
Curve25519PublicKey p => PivKeyEncoder.EncodeCurve25519PublicKey(p),
43-
_ => throw new ArgumentException("The type conversion for the specified key type is not supported", nameof(parameters))
42+
43+
#pragma warning disable CS0618 // Type or member is obsolete
44+
PivPublicKey p => p.PivEncodedPublicKey.ToArray(),
45+
#pragma warning restore CS0618 // Type or member is obsolete
46+
_ => throw new ArgumentException(
47+
"The type conversion for the specified key type is not supported", nameof(parameters))
4448
};
45-
}
46-
49+
4750
/// <summary>
4851
/// Encodes a private key into the PIV format.
4952
/// </summary>
5053
/// <param name="parameters">The private key parameters to encode.</param>
5154
/// <returns>A BER encoded byte array containing the encoded private key.</returns>
5255
/// <exception cref="ArgumentException">Thrown when the key type is not supported or when RSA key components have invalid lengths.</exception>
53-
public static Memory<byte> EncodeAsPiv(this IPrivateKey parameters)
54-
{
55-
return parameters switch
56+
/// <remarks>This method returns a newly allocated array containing sensitive information.</remarks>
57+
public static Memory<byte> EncodeAsPiv(this IPrivateKey parameters) =>
58+
parameters switch
5659
{
5760
ECPrivateKey p => PivKeyEncoder.EncodeECPrivateKey(p),
5861
RSAPrivateKey p => PivKeyEncoder.EncodeRSAPrivateKey(p),
5962
Curve25519PrivateKey p => PivKeyEncoder.EncodeCurve25519PrivateKey(p),
60-
_ => throw new ArgumentException("The type conversion for the specified key type is not supported", nameof(parameters))
63+
64+
#pragma warning disable CS0618 // Type or member is obsolete
65+
PivPrivateKey p => p.EncodedPrivateKey.ToArray(),
66+
#pragma warning restore CS0618 // Type or member is obsolete
67+
_ => throw new ArgumentException(
68+
"The type conversion for the specified key type is not supported", nameof(parameters))
6169
};
62-
}
6370
}

‎Yubico.YubiKey/src/Yubico/YubiKey/Piv/Converters/PivKeyEncoder.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@
2020
namespace Yubico.YubiKey.Piv.Converters;
2121

2222
/// <summary>
23-
/// This class converts from a Piv Encoded Key to either instances of the common IPublicKey and IPrivateKey
24-
/// or concrete the concrete types that inherit these interfaces.
23+
/// This class converts from IPublicKey, IPrivateKey and implementations of either to a PIV-encoded key.
2524
/// </summary>
26-
internal class PivKeyEncoder
25+
internal static class PivKeyEncoder
2726
{
27+
[Obsolete("KeyExtensions instead", false)]
2828
public static Memory<byte> EncodePublicKey(IPublicKey publicKey)
2929
{
3030
return publicKey switch
@@ -35,7 +35,7 @@ public static Memory<byte> EncodePublicKey(IPublicKey publicKey)
3535
_ => throw new ArgumentException("Unsupported public key type."),
3636
};
3737
}
38-
38+
3939
public static Memory<byte> EncodeRSAPublicKey(RSAPublicKey publicKey)
4040
{
4141
var rsaParameters = publicKey.Parameters;
@@ -70,17 +70,17 @@ public static Memory<byte> EncodeECPublicKey(ECPublicKey publicKey)
7070

7171
return tlvWriter.Encode();
7272
}
73-
74-
public static Memory<byte> EncodePrivateKey(IPrivateKey publicKey)
75-
{
76-
return publicKey switch
73+
74+
[Obsolete("KeyExtensions instead", false)]
75+
public static Memory<byte> EncodePrivateKey(IPrivateKey publicKey) =>
76+
publicKey switch
7777
{
7878
Curve25519PrivateKey curve25519PrivateKey => EncodeCurve25519PrivateKey(curve25519PrivateKey),
7979
ECPrivateKey ecPrivateKey => EncodeECPrivateKey(ecPrivateKey),
8080
RSAPrivateKey rsaPrivateKey => EncodeRSAPrivateKey(rsaPrivateKey),
81+
8182
_ => throw new ArgumentException("Unsupported public key type."),
8283
};
83-
}
8484

8585
public static Memory<byte> EncodeECPrivateKey(ECPrivateKey privateKey)
8686
{

‎Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivEccPrivateKey.cs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@
1313
// limitations under the License.
1414

1515
using System;
16+
using System.Collections.Generic;
1617
using System.Globalization;
18+
using System.Linq;
1719
using System.Security.Cryptography;
1820
using Yubico.Core.Tlv;
21+
using Yubico.YubiKey.Cryptography;
1922

2023
namespace Yubico.YubiKey.Piv
2124
{
@@ -27,11 +30,11 @@ namespace Yubico.YubiKey.Piv
2730
/// of a point on the curve. So for ECC P-256, each coordinate is 32 bytes
2831
/// (256 bits), so the private value will be 32 bytes.
2932
/// </remarks>
33+
[Obsolete(
34+
"Usage of PivEccPublic/PivEccPrivateKey, PivRsaPublic/PivRsaPrivateKey is deprecated. Use implementations of ECPublicKey, ECPrivateKey and RSAPublicKey, RSAPrivateKey instead",
35+
false)]
3036
public sealed class PivEccPrivateKey : PivPrivateKey
3137
{
32-
private const int EccP256PrivateKeySize = 32;
33-
private const int EccP384PrivateKeySize = 48;
34-
3538
private Memory<byte> _privateValue;
3639

3740
// <summary>
@@ -66,29 +69,19 @@ private PivEccPrivateKey()
6669
/// <exception cref="ArgumentException">
6770
/// The size of the private value is not supported by the YubiKey.
6871
/// </exception>
72+
[Obsolete("Usage of PivEccPublic/PivEccPrivateKey is deprecated. Use ECPublicKey, ECPrivateKey instead", false)]
6973
public PivEccPrivateKey(
70-
ReadOnlySpan<byte> privateValue,
74+
ReadOnlySpan<byte> privateValue,
7175
PivAlgorithm? algorithm = null)
7276
{
7377
if (algorithm.HasValue)
7478
{
75-
int expectedSize = algorithm.Value.GetPivKeyDefinition().KeyDefinition.LengthInBytes;
76-
if(privateValue.Length != expectedSize)
77-
{
78-
throw new ArgumentException(
79-
string.Format(
80-
CultureInfo.CurrentCulture, ExceptionMessages.InvalidPrivateKeyData));
81-
}
79+
ValidateSize(privateValue, algorithm.Value);
80+
Algorithm = algorithm.Value;
8281
}
8382
else
8483
{
85-
Algorithm = privateValue.Length switch
86-
{
87-
EccP256PrivateKeySize => PivAlgorithm.EccP256,
88-
EccP384PrivateKeySize => PivAlgorithm.EccP384,
89-
_ => throw new ArgumentException(string.Format(
90-
CultureInfo.CurrentCulture, ExceptionMessages.InvalidPrivateKeyData))
91-
};
84+
Algorithm = GetBySize(privateValue);
9285
}
9386

9487
int tag = Algorithm switch
@@ -97,13 +90,38 @@ public PivEccPrivateKey(
9790
PivAlgorithm.EccX25519 => PivConstants.PrivateECX25519Tag,
9891
_ => PivConstants.PrivateECDsaTag
9992
};
100-
93+
10194
var tlvWriter = new TlvWriter();
10295
tlvWriter.WriteValue(tag, privateValue);
10396
EncodedKey = tlvWriter.Encode();
10497
_privateValue = new Memory<byte>(privateValue.ToArray());
10598
}
10699

100+
private static void ValidateSize(ReadOnlySpan<byte> privateValue, PivAlgorithm algorithm)
101+
{
102+
int expectedSize = algorithm.GetPivKeyDefinition().KeyDefinition.LengthInBytes;
103+
if (privateValue.Length != expectedSize)
104+
{
105+
throw new ArgumentException(
106+
string.Format(
107+
CultureInfo.CurrentCulture, ExceptionMessages.InvalidPrivateKeyData));
108+
}
109+
}
110+
111+
private static PivAlgorithm GetBySize(ReadOnlySpan<byte> privateValue)
112+
{
113+
int privateValueSize = privateValue.Length;
114+
var allowed = new List<KeyDefinition> { KeyDefinitions.P256, KeyDefinitions.P384 };
115+
if (allowed.SingleOrDefault(kd => kd.LengthInBytes == privateValueSize) is { } keyDefinition)
116+
{
117+
return keyDefinition.GetPivKeyDefinition().Algorithm;
118+
}
119+
120+
throw new ArgumentException(
121+
string.Format(
122+
CultureInfo.CurrentCulture, ExceptionMessages.InvalidPrivateKeyData));
123+
}
124+
107125
/// <summary>
108126
/// Create a new instance of an ECC private key object based on the
109127
/// encoding.

0 commit comments

Comments
 (0)