Skip to content

Commit cf4eb46

Browse files
JohnMcPMSJohnMcPMS
authored
MS Store cert pinning updates (1.11) (#5735)
Cherry-pick #5732 to 1.11 ## 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 e75fd58 commit cf4eb46

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
@@ -105,6 +106,7 @@ countryregion
105106
Cov
106107
CPIL
107108
createmanifestmetadata
109+
crt
108110
cswinrt
109111
ctc
110112
CTL
@@ -166,6 +168,7 @@ FECAFEB
166168
fedorapeople
167169
fileinuse
168170
filemode
171+
Filetime
169172
Filtercriteria
170173
Finalizers
171174
fintimes
@@ -450,6 +453,7 @@ removefile
450453
reparse
451454
repeatedkey
452455
REQS
456+
requirenonleaf
453457
restsource
454458
RGBQUAD
455459
rgp
@@ -536,6 +540,7 @@ thiscouldbeapc
536540
threehundred
537541
timespan
538542
Tlg
543+
TLSCAs
539544
tombstoned
540545
transitioning
541546
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)