Skip to content
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.

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

Fix unbound MAC work in GetCertContentType #100163

Merged
merged 2 commits into from
Mar 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ private static DSA DecodeDsaPublicKey(byte[] encodedKeyValue, byte[] encodedPara
public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)
{
const int errSecUnknownFormat = -25257;

if (rawData.IsEmpty)
{
// Throw to match Windows and Unix behavior.
Expand All @@ -119,7 +120,7 @@ public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)

X509ContentType contentType = Interop.AppleCrypto.X509GetContentType(rawData);

// Apple doesn't seem to recognize PFX files with no MAC, so try a quick maybe-it's-a-PFX test
// Apple's native check can't check for PKCS12, so do a quick decode test to see if it is PKCS12 / PFX.
if (contentType == X509ContentType.Unknown)
{
try
Expand All @@ -128,9 +129,11 @@ public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)
{
fixed (byte* pin = rawData)
{
AsnValueReader reader = new AsnValueReader(rawData, AsnEncodingRules.BER);

using (var manager = new PointerMemoryManager<byte>(pin, rawData.Length))
{
PfxAsn.Decode(manager.Memory, AsnEncodingRules.BER);
PfxAsn.Decode(ref reader, manager.Memory, out _);
}

contentType = X509ContentType.Pkcs12;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.DotNet.RemoteExecutor;
using Microsoft.DotNet.XUnitExtensions;
using Xunit;

namespace System.Security.Cryptography.X509Certificates.Tests
{
[SkipOnPlatform(TestPlatforms.Browser, "Browser doesn't support X.509 certificates")]
public class PfxIterationCountTests_X509Certificate2 : PfxIterationCountTests
{
internal override X509Certificate Import(byte[] blob)
Expand All @@ -22,5 +27,29 @@ internal override X509Certificate Import(string fileName, string password)

internal override X509Certificate Import(string fileName, SecureString password)
=> new X509Certificate2(fileName, password);


[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public static void Import_IterationCountLimitExceeded_ThrowsInAllottedTime()
{
const int AllottedTime = 5000;

if (!PfxTests.Pkcs12PBES2Supported)
{
throw new SkipTestException("Pkcs12NoPassword100MRounds uses PBES2, which is not supported on this version.");
}

RemoteInvokeOptions options = new()
{
TimeOut = AllottedTime
};

RemoteExecutor.Invoke(static () =>
{
byte[] blob = TestData.Pkcs12NoPassword100MRounds;
CryptographicException ce = Assert.Throws<CryptographicException>(() => new X509Certificate2(blob));
Assert.Contains(FwlinkId, ce.Message);
}, options).Dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3437,6 +3437,26 @@ internal static DSAParameters GetDSA1024Params()
"04020105000420AD0EB570ACFB8357A8E99B17672353CFBA69C76FFE5B6BC113" +
"05577F12AE24040408D04E60444B79672302030927C1").HexToByteArray();

internal static readonly byte[] Pkcs12NoPassword100MRounds = Convert.FromBase64String(
"MIIDygIBAzCCA4QGCSqGSIb3DQEHAaCCA3UEggNxMIIDbTCCA2kGCSqGSIb3DQEHBqCCA1owggNW" +
"AgEAMIIDTwYJKoZIhvcNAQcBMF4GCSqGSIb3DQEFDTBRMDAGCSqGSIb3DQEFDDAjBBCNparJkj/3" +
"Uk8N7n0KCMeQAgEBMAwGCCqGSIb3DQILBQAwHQYJYIZIAWUDBAEqBBAcqpBrSDFcXYAWVWKcsEi9" +
"gIIC4P/ANdPYWI1vBH1U5sZGMIwLjY96pYaBelyZd0ZfKA8QfGHVNP9+E9hplBKGvRfIMiqmFutj" +
"RO4v7Ls8HZEk0hwBt9+6zXPWDJLxBDfSMHUd08+ZAH1yzEqq8aBMyIRVHOQkJFuFuCQJ9Ke5HzVi" +
"39S1rgHpnKYFvy+xZAhgI9OO1YxuFt4P9nhlEV/JCoyEQ/2iY99kKc3z7ArrV7BBFhfYGKhWQCBu" +
"kAmNBKweRldNWgDuW21WJEl5sByOmyDwpiK55Zxy1K1aIY8DYJTtIzzcX4CILaj6tClMH1G9w4jW" +
"BkQI2CG4vCsMl/28BbIP9EyH2C+gBAxvc1N32y3NSvO0/GPVenmQFF9KBMc4FVy4Z21syMKzUkBi" +
"PtIbDkcQbGAfyPgFk4SXCgn8OpIIvOOGI50/r+Hj14qex9VIrlwAAWCH8Y+YjwqFAQJYHQpb47zp" +
"B1fTwJFOrsXrBgLUzJLZKLR43yW2E9u6b8RsTuFHjh985naCHLuWPYOXS1zduBpHKpwoPUyCwD2r" +
"DAokCvA7RCsSXroUkpJarN4CAqsEB8COnzV1Dl2xcAYMerJxrTCKX6WIQUYo0/qeCoqTT38lDAlE" +
"7Ydjyx12iVM6eWejAdjORvlVtCQQtCxz8fZpdFGbMP8rf35A8hu++e4u0CLHnhTx38zPIm6H6YfN" +
"qj5h1Kz0xLzqnRfa7EGfDEERSHOy/DqNY4nUNG2DTjGOHy1QJelToG7Vo2L7CCZV+leX0nwLNExf" +
"hKEp+uQCiYSJe9iDm9fS9VymED79OJbr2bxdq3MggEGksLZv/H0ZT8Wsue0vq9jQ6J6YIEM+DKYr" +
"Zt2l4WgTBEKbpqmRvOqYRh9O8Sp+3IRNPzMC2ehzlYXqoPbtG4vxpoRsAMCM/W2x61jbsBSaNSFA" +
"eaUwcnKswRg30UonHUAIOJkqtadI57WE/Rat5eHVyya9f7ZN8bTFZjx0BQs6Bo8PK9yfqoidSN8w" +
"PTAfMAcGBSsOAwIaBBTt8zpgzygINykjoAwr2GKEywYFwgQUA+L1vfCVASwiE++gTfRgIScMGycC" +
"BAX14QA=");

internal static readonly byte[] Pkcs12OpenSslOneCertDefaultEmptyPassword =
("308209CF0201033082098506092A864886F70D010701A0820976048209723082" +
"096E308203E206092A864886F70D010706A08203D3308203CF020100308203C8" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,18 @@ PAL_X509ContentType AppleCryptoNative_X509GetContentType(uint8_t* pbData, int32_
// The sniffing order is:
// * X509 DER
// * PKCS7 PEM/DER
// * PKCS12 DER (or PEM if Apple has non-standard support for that)
// * X509 PEM or PEM aggregate (or DER, but that already matched)
//
// If the X509 PEM check is done first SecItemImport will erroneously match
// some PKCS#7 blobs and say they were certificates.
//
// Likewise, if the X509 DER check isn't done first, Apple will report it as
// being a PKCS#7.
//
// This does not attempt to open a PFX / PKCS12 as Apple does not provide
// a suitable API to determine if it is PKCS12 without doing potentially
// unbound MAC / KDF work. Instead, let that return Unknown and let the managed
// decoding do the check.
SecCertificateRef certref = SecCertificateCreateWithData(NULL, cfData);

if (certref != NULL)
Expand All @@ -104,41 +108,6 @@ PAL_X509ContentType AppleCryptoNative_X509GetContentType(uint8_t* pbData, int32_
}
}

dataFormat = kSecFormatPKCS12;
actualFormat = dataFormat;
itemType = kSecItemTypeAggregate;
actualType = itemType;

osStatus = SecItemImport(cfData, NULL, &actualFormat, &actualType, 0, NULL, NULL, NULL);

if (osStatus == errSecPassphraseRequired)
{
dataFormat = kSecFormatPKCS12;
actualFormat = dataFormat;
itemType = kSecItemTypeAggregate;
actualType = itemType;

SecItemImportExportKeyParameters importParams;
memset(&importParams, 0, sizeof(SecItemImportExportKeyParameters));

importParams.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
importParams.passphrase = CFSTR("");

osStatus = SecItemImport(cfData, NULL, &actualFormat, &actualType, 0, &importParams, NULL, NULL);

CFRelease(importParams.passphrase);
importParams.passphrase = NULL;
}

if (osStatus == noErr || osStatus == errSecPkcs12VerifyFailure)
{
if (actualType == itemType && actualFormat == dataFormat)
{
CFRelease(cfData);
return PAL_Pkcs12;
}
}

dataFormat = kSecFormatX509Cert;
actualFormat = dataFormat;
itemType = kSecItemTypeCertificate;
Expand Down