Skip to content

Commit d97d07e

Browse files
JohnMcPMSJohnMcPMS
authored
MS Store cert pinning updates (#5732)
## Change New certificate pinning guidelines/PKI allow us to pin only a trusted intermediate. This means less churn due to renewals with the Store. Adds functionality to the pinning validation to allow partial chain definitions. This is leveraged to allow chains containing two new intermediate certificates The existing chains are left as is since they continue to be the current in-operation values. ## Validation Adds new tests covering partial chain definitions, etc. Adds a new test to warn about the remaining lifetime of pinning certificates.
1 parent f020909 commit d97d07e

File tree

14 files changed

+545
-59
lines changed

14 files changed

+545
-59
lines changed

‎.github/actions/spelling/expect.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ AMap
1818
Amd
1919
amrutha
2020
ansistring
21+
anyissuer
2122
Aot
2223
APARTMENTTHREADED
2324
apfn
@@ -106,6 +107,7 @@ countryregion
106107
Cov
107108
CPIL
108109
createmanifestmetadata
110+
crt
109111
cswinrt
110112
ctc
111113
CTL
@@ -167,6 +169,7 @@ FECAFEB
167169
fedorapeople
168170
fileinuse
169171
filemode
172+
Filetime
170173
Filtercriteria
171174
Finalizers
172175
fintimes
@@ -452,6 +455,7 @@ removefile
452455
reparse
453456
repeatedkey
454457
REQS
458+
requirenonleaf
455459
restsource
456460
RGBQUAD
457461
rgp
@@ -538,6 +542,7 @@ thiscouldbeapc
538542
threehundred
539543
timespan
540544
Tlg
545+
TLSCAs
541546
tombstoned
542547
transitioning
543548
trimstart

‎src/AppInstallerCLITests/Certificates.cpp‎

Lines changed: 167 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ TEST_CASE("Certificates_NoPinningSucceeds", "[certificates]")
1818
PinningDetails actual;
1919
actual.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
2020

21-
REQUIRE(expected.Validate(actual.GetCertificate()));
21+
REQUIRE(CertificatePinningValidationResult::Accepted == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Leaf));
2222
}
2323

2424
TEST_CASE("Certificates_PublicKeyMismatch", "[certificates]")
@@ -29,7 +29,7 @@ TEST_CASE("Certificates_PublicKeyMismatch", "[certificates]")
2929
PinningDetails actual;
3030
actual.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
3131

32-
REQUIRE(!expected.Validate(actual.GetCertificate()));
32+
REQUIRE(CertificatePinningValidationResult::Rejected == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Leaf));
3333
}
3434

3535
TEST_CASE("Certificates_PublicKeyMatch", "[certificates]")
@@ -40,7 +40,7 @@ TEST_CASE("Certificates_PublicKeyMatch", "[certificates]")
4040
PinningDetails actual;
4141
actual.LoadCertificate(IDX_CERTIFICATE_STORE_ROOT_2, CERTIFICATE_RESOURCE_TYPE);
4242

43-
REQUIRE(expected.Validate(actual.GetCertificate()));
43+
REQUIRE(CertificatePinningValidationResult::Accepted == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Root));
4444
}
4545

4646
TEST_CASE("Certificates_SubjectMismatch", "[certificates]")
@@ -51,7 +51,7 @@ TEST_CASE("Certificates_SubjectMismatch", "[certificates]")
5151
PinningDetails actual;
5252
actual.LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE);
5353

54-
REQUIRE(!expected.Validate(actual.GetCertificate()));
54+
REQUIRE(CertificatePinningValidationResult::Rejected == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Intermediate));
5555
}
5656

5757
TEST_CASE("Certificates_SubjectMatch", "[certificates]")
@@ -62,7 +62,7 @@ TEST_CASE("Certificates_SubjectMatch", "[certificates]")
6262
PinningDetails actual;
6363
actual.LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE);
6464

65-
REQUIRE(expected.Validate(actual.GetCertificate()));
65+
REQUIRE(CertificatePinningValidationResult::Accepted == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Intermediate));
6666
}
6767

6868
TEST_CASE("Certificates_IssuerMismatch", "[certificates]")
@@ -73,7 +73,7 @@ TEST_CASE("Certificates_IssuerMismatch", "[certificates]")
7373
PinningDetails actual;
7474
actual.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
7575

76-
REQUIRE(!expected.Validate(actual.GetCertificate()));
76+
REQUIRE(CertificatePinningValidationResult::Rejected == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Leaf));
7777
}
7878

7979
TEST_CASE("Certificates_IssuerMatch", "[certificates]")
@@ -84,7 +84,7 @@ TEST_CASE("Certificates_IssuerMatch", "[certificates]")
8484
PinningDetails actual;
8585
actual.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
8686

87-
REQUIRE(expected.Validate(actual.GetCertificate()));
87+
REQUIRE(CertificatePinningValidationResult::Accepted == expected.Validate(actual.GetCertificate(), CertificateChainPosition::Leaf));
8888
}
8989

9090
TEST_CASE("Certificates_ChainLengthDiffers", "[certificates]")
@@ -104,6 +104,122 @@ TEST_CASE("Certificates_ChainLengthDiffers", "[certificates]")
104104
REQUIRE(!config.Validate(details.GetCertificate()));
105105
}
106106

107+
TEST_CASE("Certificates_ChainLengthDiffers_Partial", "[certificates]")
108+
{
109+
PinningChain chain;
110+
auto chainElement = chain.Root();
111+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_ROOT_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey);
112+
chainElement = chainElement.Next();
113+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::Subject | PinningVerificationType::Issuer);
114+
chain.PartialChain();
115+
116+
PinningConfiguration config;
117+
config.AddChain(chain);
118+
119+
PinningDetails details;
120+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
121+
122+
REQUIRE(config.Validate(details.GetCertificate()));
123+
}
124+
125+
TEST_CASE("CertificateChain_AnyIssuer_Intermediate", "[certificates]")
126+
{
127+
PinningChain chain;
128+
auto chainElement = chain.Root();
129+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
130+
chain.PartialChain();
131+
132+
PinningConfiguration config;
133+
config.AddChain(chain);
134+
135+
PinningDetails details;
136+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
137+
138+
REQUIRE(config.Validate(details.GetCertificate()));
139+
}
140+
141+
TEST_CASE("CertificateChain_AnyIssuer_IntermediateDiffers", "[certificates]")
142+
{
143+
PinningChain chain;
144+
auto chainElement = chain.Root();
145+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_1, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
146+
chain.PartialChain();
147+
148+
PinningConfiguration config;
149+
config.AddChain(chain);
150+
151+
PinningDetails details;
152+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
153+
154+
REQUIRE(!config.Validate(details.GetCertificate()));
155+
}
156+
157+
TEST_CASE("CertificateChain_AnyIssuer_IntermediateAndLeaf", "[certificates]")
158+
{
159+
PinningChain chain;
160+
auto chainElement = chain.Root();
161+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
162+
chainElement = chainElement.Next();
163+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::Subject | PinningVerificationType::Issuer);
164+
chain.PartialChain();
165+
166+
PinningConfiguration config;
167+
config.AddChain(chain);
168+
169+
PinningDetails details;
170+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
171+
172+
REQUIRE(config.Validate(details.GetCertificate()));
173+
}
174+
175+
TEST_CASE("CertificateChain_AnyIssuer_Leaf", "[certificates]")
176+
{
177+
PinningChain chain;
178+
auto chainElement = chain.Root();
179+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer);
180+
chain.PartialChain();
181+
182+
PinningConfiguration config;
183+
config.AddChain(chain);
184+
185+
PinningDetails details;
186+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
187+
188+
REQUIRE(config.Validate(details.GetCertificate()));
189+
}
190+
191+
TEST_CASE("CertificateChain_AnyIssuer_LeafDiffers", "[certificates]")
192+
{
193+
PinningChain chain;
194+
auto chainElement = chain.Root();
195+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_1, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer);
196+
chain.PartialChain();
197+
198+
PinningConfiguration config;
199+
config.AddChain(chain);
200+
201+
PinningDetails details;
202+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
203+
204+
REQUIRE(!config.Validate(details.GetCertificate()));
205+
}
206+
207+
TEST_CASE("CertificateChain_AnyIssuer_SameLeaf_RequireNonLeaf", "[certificates]")
208+
{
209+
PinningChain chain;
210+
auto chainElement = chain.Root();
211+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
212+
chain.PartialChain();
213+
214+
PinningConfiguration config;
215+
config.AddChain(chain);
216+
217+
PinningDetails details;
218+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
219+
220+
REQUIRE(!config.Validate(details.GetCertificate()));
221+
}
222+
107223
TEST_CASE("Certificates_EmptyChainRejects", "[certificates]")
108224
{
109225
PinningChain chain;
@@ -183,3 +299,47 @@ TEST_CASE("Certificates_MultipleChains_Success", "[certificates]")
183299

184300
REQUIRE(config.Validate(details.GetCertificate()));
185301
}
302+
303+
TEST_CASE("CertificateChain_Position", "[certificates]")
304+
{
305+
CertificateChainPosition positions[3]{ CertificateChainPosition::Unknown, CertificateChainPosition::Unknown, CertificateChainPosition::Unknown };
306+
307+
PinningChain chain;
308+
auto chainElement = chain.Root();
309+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_ROOT_2, CERTIFICATE_RESOURCE_TYPE).SetCustomValidationFunction([&](const PinningDetails&, PCCERT_CONTEXT, CertificateChainPosition position) { positions[0] = position; return true; });
310+
chainElement = chainElement.Next();
311+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_INTERMEDIATE_2, CERTIFICATE_RESOURCE_TYPE).SetCustomValidationFunction([&](const PinningDetails&, PCCERT_CONTEXT, CertificateChainPosition position) { positions[1] = position; return true; });
312+
chainElement = chainElement.Next();
313+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE).SetCustomValidationFunction([&](const PinningDetails&, PCCERT_CONTEXT, CertificateChainPosition position) { positions[2] = position; return true; });
314+
315+
PinningConfiguration config;
316+
config.AddChain(chain);
317+
318+
PinningDetails details;
319+
details.LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE);
320+
321+
REQUIRE(config.Validate(details.GetCertificate()));
322+
323+
REQUIRE(CertificateChainPosition::Root == positions[0]);
324+
REQUIRE(CertificateChainPosition::Intermediate == positions[1]);
325+
REQUIRE(CertificateChainPosition::Leaf == positions[2]);
326+
}
327+
328+
TEST_CASE("CertificateChain_Position_SelfSigned", "[certificates]")
329+
{
330+
CertificateChainPosition positions[1]{ CertificateChainPosition::Unknown };
331+
332+
PinningChain chain;
333+
auto chainElement = chain.Root();
334+
chainElement->LoadCertificate(IDX_CERTIFICATE_STORE_ROOT_2, CERTIFICATE_RESOURCE_TYPE).SetCustomValidationFunction([&](const PinningDetails&, PCCERT_CONTEXT, CertificateChainPosition position) { positions[0] = position; return true; });
335+
336+
PinningConfiguration config;
337+
config.AddChain(chain);
338+
339+
PinningDetails details;
340+
details.LoadCertificate(IDX_CERTIFICATE_STORE_ROOT_2, CERTIFICATE_RESOURCE_TYPE);
341+
342+
REQUIRE(config.Validate(details.GetCertificate()));
343+
344+
REQUIRE((CertificateChainPosition::Root | CertificateChainPosition::Leaf) == positions[0]);
345+
}

‎src/AppInstallerCLITests/Sources.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,3 +1339,24 @@ TEST_CASE("RepoSources_BuiltInDesktopFrameworkSourceAlwaysCreatable", "[sources]
13391339
Source source(WellKnownSource::DesktopFrameworks);
13401340
REQUIRE(source);
13411341
}
1342+
1343+
TEST_CASE("RepoSources_MicrosoftStore_CertificatePinningLifetimeCheck", "[sources]")
1344+
{
1345+
TestHook_ClearSourceFactoryOverrides();
1346+
1347+
GroupPolicyTestOverride policies;
1348+
policies.SetState(TogglePolicy::Policy::BypassCertificatePinningForMicrosoftStore, PolicyState::Disabled);
1349+
Source source(WellKnownSource::MicrosoftStore);
1350+
REQUIRE_FALSE(source.GetDetails().CertificatePinningConfiguration.IsEmpty());
1351+
1352+
// The configuration's remaining lifetime is the *maximum* of the remaining lifetimes of the individual chains.
1353+
// A chain's remaining lifetime is the *minimum* of the remaining lifetimes of the individual certificates.
1354+
// A certificate's remaining lifetime is a value between 0.0 and 1.0 that is the ratio of remaining valid time to total valid time.
1355+
1356+
// The goal of this test is to warn when the pinning configuration may be in danger of expiration; either via certificate validity or
1357+
// more likely by renewals causing the pinning to reject the new, correct certificates. It operates in percentage lifetime to normalize
1358+
// the values across the chain.
1359+
INFO("If this test has failed, the pinning certificates may be nearing expiration and should be investigated.");
1360+
double lifetimePercentage = source.GetDetails().CertificatePinningConfiguration.GetRemainingLifetimePercentage();
1361+
REQUIRE(lifetimePercentage > 0.25);
1362+
}

‎src/AppInstallerRepositoryCore/SourceList.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,9 +343,22 @@ namespace AppInstaller::Repository
343343
chainElement2 = chainElement2.Next();
344344
chainElement2->LoadCertificate(IDX_CERTIFICATE_STORE_LEAF_2, CERTIFICATE_RESOURCE_TYPE).SetPinning(PinningVerificationType::Subject | PinningVerificationType::Issuer);
345345

346+
// See https://aka.ms/AzureTLSCAs (internal) for the source of these CAs
347+
PinningChain chain3;
348+
chain3.PartialChain().Root()->
349+
LoadCertificate(IDX_CERTIFICATE_MS_TLS_ECC_ROOT_G2, CERTIFICATE_RESOURCE_TYPE).
350+
SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
351+
352+
PinningChain chain4;
353+
chain4.PartialChain().Root()->
354+
LoadCertificate(IDX_CERTIFICATE_MS_TLS_RSA_ROOT_G2, CERTIFICATE_RESOURCE_TYPE).
355+
SetPinning(PinningVerificationType::PublicKey | PinningVerificationType::AnyIssuer | PinningVerificationType::RequireNonLeaf);
356+
346357
details.CertificatePinningConfiguration = PinningConfiguration("Microsoft Store Source");
347358
details.CertificatePinningConfiguration.AddChain(std::move(chain));
348359
details.CertificatePinningConfiguration.AddChain(std::move(chain2));
360+
details.CertificatePinningConfiguration.AddChain(std::move(chain3));
361+
details.CertificatePinningConfiguration.AddChain(std::move(chain4));
349362
}
350363

351364
return details;

0 commit comments

Comments
 (0)