Skip to content

Change Pkcs12SafeContentsBag decoding from recursive to iterative #126513

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
bartonjs merged 2 commits into from
Apr 6, 2026
Merged

Change Pkcs12SafeContentsBag decoding from recursive to iterative #126513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44cdc6e
Change Pkcs12SafeContentsBag decoding from recursive to iterative
bartonjs Apr 3, 2026
078cf6d
Preserve decode failure becoming UnknownBag
bartonjs Apr 3, 2026
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
63 changes: 62 additions & 1 deletion src/libraries/Common/src/System/Security/Cryptography/Pkcs/Pkcs12SafeContents.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
using System.Security.Cryptography.X509Certificates;
using Internal.Cryptography;

using NestedSafeContentsStack = System.Collections.Generic.Stack<(
System.ReadOnlyMemory<byte> Serialized,
System.Security.Cryptography.Pkcs.Pkcs12SafeContentsBag Bag,
System.Collections.Generic.List<System.Security.Cryptography.Pkcs.Pkcs12SafeBag> Container)>;

namespace System.Security.Cryptography.Pkcs
{
#if BUILDING_PKCS
Expand All @@ -30,6 +35,12 @@ public Pkcs12SafeContents()
ConfidentialityMode = Pkcs12ConfidentialityMode.None;
}

private Pkcs12SafeContents(bool isReadOnly)
{
IsReadOnly = isReadOnly;
ConfidentialityMode = Pkcs12ConfidentialityMode.None;
}

internal Pkcs12SafeContents(ReadOnlyMemory<byte> serialized)
{
IsReadOnly = true;
Expand Down Expand Up @@ -278,6 +289,50 @@ public IEnumerable<Pkcs12SafeBag> GetBags()
}

private static List<Pkcs12SafeBag> ReadBags(ReadOnlyMemory<byte> serialized)
{
NestedSafeContentsStack pendingContents = new NestedSafeContentsStack();
List<Pkcs12SafeBag> result = ReadBagsCore(serialized, pendingContents);

while (pendingContents.Count > 0)
{
(ReadOnlyMemory<byte> nestedSerialized, Pkcs12SafeContentsBag bag, List<Pkcs12SafeBag> container) = pendingContents.Pop();
bool replace = true;

try
{
Pkcs12SafeContents? target = bag.SafeContents;
Debug.Assert(target is not null);
target._bags = ReadBagsCore(nestedSerialized, pendingContents);
replace = false;
}
catch (AsnContentException)
{
}
catch (CryptographicException)
{
}

if (replace)
{
// Replace the deferred bag with an unknown bag, since the contents couldn't be parsed.
Pkcs12SafeBag.UnknownBag unknownBag = new(Oids.Pkcs12SafeContentsBag, nestedSerialized)
{
Attributes = bag.Attributes,
};

int idx = container.IndexOf(bag);
Debug.Assert(idx != -1);

container[idx] = unknownBag;
}
}

return result;
}

private static List<Pkcs12SafeBag> ReadBagsCore(
ReadOnlyMemory<byte> serialized,
NestedSafeContentsStack pendingSafeContents)
{
List<SafeBagAsn> serializedBags = new List<SafeBagAsn>();

Expand Down Expand Up @@ -330,8 +385,14 @@ private static List<Pkcs12SafeBag> ReadBags(ReadOnlyMemory<byte> serialized)
bag = Pkcs12SecretBag.DecodeValue(bagValue);
break;
case Oids.Pkcs12SafeContentsBag:
bag = Pkcs12SafeContentsBag.Decode(bagValue);
{
Pkcs12SafeContents deferredContents = new(isReadOnly: true);
Pkcs12SafeContentsBag nested;
nested = Pkcs12SafeContentsBag.CreateWithDeferredContents(bagValue, deferredContents);
bag = nested;
pendingSafeContents.Push((bagValue, nested, bags));
break;
}
}
}
catch (AsnContentException)
Expand Down
12 changes: 11 additions & 1 deletion src/libraries/Common/src/System/Security/Cryptography/Pkcs/Pkcs12SafeContentsBag.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ internal static Pkcs12SafeContentsBag Create(Pkcs12SafeContents copyFrom)
return Decode(writer.Encode());
}

internal static Pkcs12SafeContentsBag Decode(ReadOnlyMemory<byte> encodedValue)
private static Pkcs12SafeContentsBag Decode(ReadOnlyMemory<byte> encodedValue)
{
Pkcs12SafeContents contents = new Pkcs12SafeContents(encodedValue);

Expand All @@ -39,5 +39,15 @@ internal static Pkcs12SafeContentsBag Decode(ReadOnlyMemory<byte> encodedValue)
SafeContents = contents
};
}

internal static Pkcs12SafeContentsBag CreateWithDeferredContents(
ReadOnlyMemory<byte> encodedValue,
Pkcs12SafeContents deferredContents)
{
return new Pkcs12SafeContentsBag(encodedValue)
{
SafeContents = deferredContents
};
}
}
}
136 changes: 136 additions & 0 deletions src/libraries/System.Security.Cryptography.Pkcs/tests/Pkcs12/Pkcs12InfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Formats.Asn1;
using System.Linq;
using System.Security.Cryptography.X509Certificates;
using System.Text;
Expand Down Expand Up @@ -286,6 +287,141 @@ public static void DecodeWithTooManyPbeIterations()
}
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public static void DecodeDeeplyNestedPfx(bool withDecodeError)
{
const int Depth = 3333;
int errorTarget = withDecodeError ? Depth / 4 * 3 : -1;
byte[] pfx = BuildPfx(Depth, errorTarget);

Pkcs12Info info = Pkcs12Info.Decode(pfx, out _, skipCopy: true);
Pkcs12SafeContents contents = Assert.Single(info.AuthenticatedSafe);
Pkcs12SafeContents deepContents = contents;

int count = withDecodeError ? errorTarget - 1: Depth;

for (int i = 0; i < count; i++)
{
Pkcs12SafeBag bag = Assert.Single(contents.GetBags());
Pkcs12SafeContentsBag typedBag = Assert.IsType<Pkcs12SafeContentsBag>(bag);
contents = typedBag.SafeContents;
}

Pkcs12SafeBag finalBag = Assert.Single(contents.GetBags());

if (withDecodeError)
{
Assert.IsNotType<Pkcs12SecretBag>(finalBag);
Assert.Equal("1.2.840.113549.1.12.10.1.6", finalBag.GetBagId().Value);

ValueAsnReader reader = new ValueAsnReader(finalBag.EncodedBagValue.Span, AsnEncodingRules.BER);
ValueAsnReader inner = reader.ReadSequence();
reader.ThrowIfNotEmpty();
reader = inner;

inner = reader.ReadSequence();
reader.ThrowIfNotEmpty();
reader = inner;

reader.ReadObjectIdentifier();
inner = reader.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 0));
AssertExtensions.TrueExpression(inner.ReadBoolean());
}
else
{
Pkcs12SecretBag secretBag = Assert.IsType<Pkcs12SecretBag>(finalBag);
Assert.Equal("1.2.840.113549.1.9.21", secretBag.GetSecretType().Value);
ReadOnlySpan<byte> expectedValue = [0x04, 0x03, 0x01, 0x02, 0x03];
AssertExtensions.SequenceEqual(expectedValue, secretBag.SecretValue.Span);
}

Pkcs12SafeContents newContents = new Pkcs12SafeContents();
// Assert.NoThrow
newContents.AddNestedContents(deepContents);

static byte[] BuildPfx(int depth, int errorDepth)
{
// If depth is 0, the output is 80 bytes.
// Each nested bag adds a 13 byte OID, and 3 CONSTRUCTED values each with a 1-byte tag.
// As the depth increases, the length-length increases for the CONSTRUCTED values, up
// to 3 length-value bytes for the worst case (assuming we don't pass a big enough depth
// to exceed 16.7MB).
// So, call each nested bag 13 + 3 * (1 + 1 + 3) = 28 bytes.
// The 80 bytes of overhead can be ignored, because of the over-counting for the low iteration count.
// For 3333 the estimate is 93,324, and the actual output is 85,520.
AsnWriter builder = new AsnWriter(AsnEncodingRules.DER, checked(28 * depth));
Asn1Tag context0 = new Asn1Tag(TagClass.ContextSpecific, 0);
const string Pkcs7Data = "1.2.840.113549.1.7.1";
const string Pkcs12SafeContents = "1.2.840.113549.1.12.10.1.6";

using (builder.PushSequence())
{
builder.WriteInteger(3);

using (builder.PushSequence())
{
builder.WriteObjectIdentifier(Pkcs7Data);

using (builder.PushSequence(context0))
using (builder.PushOctetString())
using (builder.PushSequence())
{
using (builder.PushSequence())
{
builder.WriteObjectIdentifier(Pkcs7Data);

using (builder.PushSequence(context0))
using (builder.PushOctetString())
using (builder.PushSequence())
{
for (int i = 0; i < depth; i++)
{
builder.PushSequence();
builder.WriteObjectIdentifier(Pkcs12SafeContents);
builder.PushSequence(context0);

if (i == errorDepth)
{
builder.WriteBoolean(true);
}

builder.PushSequence();
}

using (builder.PushSequence())
{
builder.WriteObjectIdentifier("1.2.840.113549.1.12.10.1.5");

using (builder.PushSequence(context0))
using (builder.PushSequence())
{
builder.WriteObjectIdentifier("1.2.840.113549.1.9.21");

using (builder.PushSequence(context0))
{
builder.WriteOctetString([0x01, 0x02, 0x03]);
}
}
}

for (int i = 0; i < depth; i++)
{
builder.PopSequence();
builder.PopSequence(context0);
builder.PopSequence();
}
}
}
}
}
}

return builder.Encode();
}
}

public static IEnumerable<object[]> DecodeWithHighPbeIterationsData
{
get
Expand Down
Loading