Skip to content

Use HashSet/SearchValues when known OID list is long #117849

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

Closed
PranavSenthilnathan wants to merge 1 commit into from
Closed

Use HashSet/SearchValues when known OID list is long #117849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 30 additions & 21 deletions src/libraries/Common/src/System/Security/Cryptography/CompositeMLDsa.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Formats.Asn1;
Expand All @@ -25,27 +26,35 @@ public abstract class CompositeMLDsa : IDisposable
#pragma warning restore SA1001
#endif
{
private static readonly string[] s_knownOids =
[
Oids.MLDsa44WithRSA2048PssPreHashSha256,
Oids.MLDsa44WithRSA2048Pkcs15PreHashSha256,
Oids.MLDsa44WithEd25519PreHashSha512,
Oids.MLDsa44WithECDsaP256PreHashSha256,
Oids.MLDsa65WithRSA3072PssPreHashSha512,
Oids.MLDsa65WithRSA3072Pkcs15PreHashSha512,
Oids.MLDsa65WithRSA4096PssPreHashSha512,
Oids.MLDsa65WithRSA4096Pkcs15PreHashSha512,
Oids.MLDsa65WithECDsaP256PreHashSha512,
Oids.MLDsa65WithECDsaP384PreHashSha512,
Oids.MLDsa65WithECDsaBrainpoolP256r1PreHashSha512,
Oids.MLDsa65WithEd25519PreHashSha512,
Oids.MLDsa87WithECDsaP384PreHashSha512,
Oids.MLDsa87WithECDsaBrainpoolP384r1PreHashSha512,
Oids.MLDsa87WithEd448PreHashShake256_512,
Oids.MLDsa87WithRSA3072PssPreHashSha512,
Oids.MLDsa87WithRSA4096PssPreHashSha512,
Oids.MLDsa87WithECDsaP521PreHashSha512,
];
private static readonly KeyFormatHelper.StringLookup s_knownOids =
#if NET9_0_OR_GREATER
new(SearchValues.Create([
#else
new([
#endif
Oids.MLDsa44WithRSA2048PssPreHashSha256,
Oids.MLDsa44WithRSA2048Pkcs15PreHashSha256,
Oids.MLDsa44WithEd25519PreHashSha512,
Oids.MLDsa44WithECDsaP256PreHashSha256,
Oids.MLDsa65WithRSA3072PssPreHashSha512,
Oids.MLDsa65WithRSA3072Pkcs15PreHashSha512,
Oids.MLDsa65WithRSA4096PssPreHashSha512,
Oids.MLDsa65WithRSA4096Pkcs15PreHashSha512,
Oids.MLDsa65WithECDsaP256PreHashSha512,
Oids.MLDsa65WithECDsaP384PreHashSha512,
Oids.MLDsa65WithECDsaBrainpoolP256r1PreHashSha512,
Oids.MLDsa65WithEd25519PreHashSha512,
Oids.MLDsa87WithECDsaP384PreHashSha512,
Oids.MLDsa87WithECDsaBrainpoolP384r1PreHashSha512,
Oids.MLDsa87WithEd448PreHashShake256_512,
Oids.MLDsa87WithRSA3072PssPreHashSha512,
Oids.MLDsa87WithRSA4096PssPreHashSha512,
Oids.MLDsa87WithECDsaP521PreHashSha512,
#if NET9_0_OR_GREATER
], StringComparison.Ordinal));
#else
]);
#endif

private const int MaxContextLength = 255;

Expand Down
5 changes: 1 addition & 4 deletions src/libraries/Common/src/System/Security/Cryptography/DSAKeyFormatHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ namespace System.Security.Cryptography
{
internal static class DSAKeyFormatHelper
{
private static readonly string[] s_validOids =
{
Oids.Dsa,
};
private static readonly KeyFormatHelper.StringLookupArray s_validOids = new([Oids.Dsa]);

internal static void ReadDsaPrivateKey(
ReadOnlyMemory<byte> xBytes,
Expand Down
5 changes: 1 addition & 4 deletions src/libraries/Common/src/System/Security/Cryptography/EccKeyFormatHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ internal static class EccKeyFormatHelper
// there's no real point reading anything bigger than this (for now).
private const int MaxFieldBitSize = 661;

private static readonly string[] s_validOids =
{
Oids.EcPublicKey,
};
private static readonly KeyFormatHelper.StringLookupArray s_validOids = new([Oids.EcPublicKey]);

internal static void ReadSubjectPublicKeyInfo(
ReadOnlySpan<byte> source,
Expand Down
10 changes: 5 additions & 5 deletions src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.Encrypted.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal static partial class KeyFormatHelper
internal delegate TRet ReadOnlySpanFunc<TIn, TRet>(ReadOnlySpan<TIn> span);

internal static unsafe void ReadEncryptedPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlySpan<byte> source,
ReadOnlySpan<char> password,
KeyReader<TRet> keyReader,
Expand All @@ -32,7 +32,7 @@ internal static unsafe void ReadEncryptedPkcs8<TRet>(
}

internal static unsafe void ReadEncryptedPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlySpan<byte> source,
ReadOnlySpan<byte> passwordBytes,
KeyReader<TRet> keyReader,
Expand All @@ -55,7 +55,7 @@ internal static unsafe void ReadEncryptedPkcs8<TRet>(
}

private static void ReadEncryptedPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
ReadOnlySpan<char> password,
KeyReader<TRet> keyReader,
Expand All @@ -73,7 +73,7 @@ private static void ReadEncryptedPkcs8<TRet>(
}

private static void ReadEncryptedPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
ReadOnlySpan<byte> passwordBytes,
KeyReader<TRet> keyReader,
Expand All @@ -91,7 +91,7 @@ private static void ReadEncryptedPkcs8<TRet>(
}

private static void ReadEncryptedPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
ReadOnlySpan<char> password,
ReadOnlySpan<byte> passwordBytes,
Expand Down
43 changes: 33 additions & 10 deletions src/libraries/Common/src/System/Security/Cryptography/KeyFormatHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Collections.Generic;
using System.Diagnostics;
using System.Formats.Asn1;
using System.Runtime.InteropServices;
Expand All @@ -14,7 +15,7 @@ internal static partial class KeyFormatHelper
internal delegate void KeyReader<TRet>(ReadOnlyMemory<byte> key, in AlgorithmIdentifierAsn algId, out TRet ret);

internal static unsafe void ReadSubjectPublicKeyInfo<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlySpan<byte> source,
KeyReader<TRet> keyReader,
out int bytesRead,
Expand All @@ -30,7 +31,7 @@ internal static unsafe void ReadSubjectPublicKeyInfo<TRet>(
}

internal static ReadOnlyMemory<byte> ReadSubjectPublicKeyInfo(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
out int bytesRead)
{
Expand All @@ -49,7 +50,7 @@ internal static ReadOnlyMemory<byte> ReadSubjectPublicKeyInfo(
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e);
}

if (Array.IndexOf(validOids, spki.Algorithm.Algorithm) < 0)
if (!validOids.Contains(spki.Algorithm.Algorithm))
{
throw new CryptographicException(SR.Cryptography_NotValidPublicOrPrivateKey);
}
Expand All @@ -59,7 +60,7 @@ internal static ReadOnlyMemory<byte> ReadSubjectPublicKeyInfo(
}

private static void ReadSubjectPublicKeyInfo<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
KeyReader<TRet> keyReader,
out int bytesRead,
Expand All @@ -80,7 +81,7 @@ private static void ReadSubjectPublicKeyInfo<TRet>(
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e);
}

if (Array.IndexOf(validOids, spki.Algorithm.Algorithm) < 0)
if (!validOids.Contains(spki.Algorithm.Algorithm))
{
throw new CryptographicException(SR.Cryptography_NotValidPublicOrPrivateKey);
}
Expand All @@ -90,7 +91,7 @@ private static void ReadSubjectPublicKeyInfo<TRet>(
}

internal static unsafe void ReadPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlySpan<byte> source,
KeyReader<TRet> keyReader,
out int bytesRead,
Expand All @@ -106,7 +107,7 @@ internal static unsafe void ReadPkcs8<TRet>(
}

internal static ReadOnlyMemory<byte> ReadPkcs8(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
out int bytesRead)
{
Expand All @@ -116,7 +117,7 @@ internal static ReadOnlyMemory<byte> ReadPkcs8(
int read = reader.PeekEncodedValue().Length;
PrivateKeyInfoAsn.Decode(ref reader, source, out PrivateKeyInfoAsn privateKeyInfo);

if (Array.IndexOf(validOids, privateKeyInfo.PrivateKeyAlgorithm.Algorithm) < 0)
if (!validOids.Contains(privateKeyInfo.PrivateKeyAlgorithm.Algorithm))
{
throw new CryptographicException(SR.Cryptography_NotValidPublicOrPrivateKey);
}
Expand All @@ -131,7 +132,7 @@ internal static ReadOnlyMemory<byte> ReadPkcs8(
}

private static void ReadPkcs8<TRet>(
string[] validOids,
IStringLookup validOids,
ReadOnlyMemory<byte> source,
KeyReader<TRet> keyReader,
out int bytesRead,
Expand All @@ -143,7 +144,7 @@ private static void ReadPkcs8<TRet>(
int read = reader.PeekEncodedValue().Length;
PrivateKeyInfoAsn.Decode(ref reader, source, out PrivateKeyInfoAsn privateKeyInfo);

if (Array.IndexOf(validOids, privateKeyInfo.PrivateKeyAlgorithm.Algorithm) < 0)
if (!validOids.Contains(privateKeyInfo.PrivateKeyAlgorithm.Algorithm))
{
throw new CryptographicException(SR.Cryptography_NotValidPublicOrPrivateKey);
}
Expand Down Expand Up @@ -204,5 +205,27 @@ internal static AsnWriter WritePkcs8(
writer.PopSequence();
return writer;
}

internal interface IStringLookup
{
bool Contains(string value);
}

internal sealed class StringLookupArray(string[] backingArray) : IStringLookup
{
public bool Contains(string value) => Array.IndexOf(backingArray, value) >= 0;
Copy link

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The StringLookupArray class stores the backing array but doesn't make a defensive copy. Consider making a copy of the input array to prevent external modifications that could affect lookup behavior.

Suggested change
public bool Contains(string value) => Array.IndexOf(backingArray, value) >= 0;
private readonly string[] _backingArray = (string[])backingArray.Clone();
public bool Contains(string value) => Array.IndexOf(_backingArray, value) >= 0;

Copilot uses AI. Check for mistakes.
}

#if NET9_0_OR_GREATER
internal sealed class StringLookup(SearchValues<string> backingSearchValues) : IStringLookup
{
public bool Contains(string value) => backingSearchValues.Contains(value);
Comment on lines +220 to +222
Copy link
Member

@MihaZupan MihaZupan Jul 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite the name, SearchValues is not beneficial in this case.
SearchValues<string>.Contains(string) is currently backed by a HashSet<string>.Contains(string).

The only searches this is being used for are setOfValues.Contains(string), this is what HashSet/FrozenSet excel at.
You'd want to use SearchValues if you were looking for substrings within a text, i.e. someText.ContainsAny(setOfValues).

}
#else
internal sealed class StringLookup(HashSet<string> backingSet) : IStringLookup
{
public bool Contains(string value) => backingSet.Contains(value);
}
#endif
}
}
4 changes: 2 additions & 2 deletions src/libraries/Common/src/System/Security/Cryptography/MLDsa.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ public abstract partial class MLDsa : IDisposable
#pragma warning restore SA1001
#endif
{
private protected static readonly string[] KnownOids =
private protected static readonly KeyFormatHelper.StringLookupArray KnownOids = new(
[
Oids.MLDsa44,
Oids.MLDsa65,
Oids.MLDsa87,
];
]);

private const int MaxContextLength = 255;

Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Common/src/System/Security/Cryptography/MLKem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace System.Security.Cryptography
[Experimental(Experimentals.PostQuantumCryptographyDiagId, UrlFormat = Experimentals.SharedUrlFormat)]
public abstract partial class MLKem : IDisposable
{
private static readonly string[] s_knownOids = [Oids.MlKem512, Oids.MlKem768, Oids.MlKem1024];
private static readonly KeyFormatHelper.StringLookupArray s_knownOids = new([Oids.MlKem512, Oids.MlKem768, Oids.MlKem1024]);

private bool _disposed;

Expand Down
5 changes: 1 addition & 4 deletions src/libraries/Common/src/System/Security/Cryptography/RSAKeyFormatHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ namespace System.Security.Cryptography
{
internal static partial class RSAKeyFormatHelper
{
private static readonly string[] s_validOids =
{
Oids.Rsa,
};
private static readonly KeyFormatHelper.StringLookupArray s_validOids = new([Oids.Rsa]);

// TODO Currently reading PKCS#1 keys uses BigInteger which is not optimal and uses APIs that are not
// available downlevel. These methods should eventually be replaced with a more efficient implementation
Expand Down
38 changes: 23 additions & 15 deletions src/libraries/Common/src/System/Security/Cryptography/SlhDsa.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,29 @@ public abstract partial class SlhDsa : IDisposable
#pragma warning restore SA1001
#endif
{
private static readonly string[] s_knownOids =
[
Oids.SlhDsaSha2_128s,
Oids.SlhDsaShake128s,
Oids.SlhDsaSha2_128f,
Oids.SlhDsaShake128f,
Oids.SlhDsaSha2_192s,
Oids.SlhDsaShake192s,
Oids.SlhDsaSha2_192f,
Oids.SlhDsaShake192f,
Oids.SlhDsaSha2_256s,
Oids.SlhDsaShake256s,
Oids.SlhDsaSha2_256f,
Oids.SlhDsaShake256f,
];
private static readonly KeyFormatHelper.StringLookup s_knownOids =
#if NET9_0_OR_GREATER
new(SearchValues.Create([
#else
new([
#endif
Oids.SlhDsaSha2_128s,
Oids.SlhDsaShake128s,
Oids.SlhDsaSha2_128f,
Oids.SlhDsaShake128f,
Oids.SlhDsaSha2_192s,
Oids.SlhDsaShake192s,
Oids.SlhDsaSha2_192f,
Oids.SlhDsaShake192f,
Oids.SlhDsaSha2_256s,
Oids.SlhDsaShake256s,
Oids.SlhDsaSha2_256f,
Oids.SlhDsaShake256f,
#if NET9_0_OR_GREATER
], StringComparison.Ordinal));
#else
]);
#endif

private const int MaxContextLength = 255;

Expand Down
7 changes: 2 additions & 5 deletions src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECAlgorithm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@ namespace System.Security.Cryptography
/// </summary>
public abstract class ECAlgorithm : AsymmetricAlgorithm
{
private static readonly string[] s_validOids =
{
Oids.EcPublicKey,
// ECDH and ECMQV are not valid in this context.
};
// ECDH and ECMQV are not valid in this context.
private static readonly KeyFormatHelper.StringLookupArray s_validOids = new([Oids.EcPublicKey]);

private protected static readonly KeySizes[] s_defaultKeySizes =
{
Expand Down
Loading