Failing test disableaiaoptionworks on OSX #47492

runfoapp · 2021-01-26T20:52:31Z

Runfo Tracking Issue: Failing test disableaiaoptionworks on OSX

Build	Definition	Kind	Run Name	Console	Test Results	Run Client
1065396	runtime	PR 50478	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
1056062	runtime	PR 50236	net6.0-windows-Release-x86-CoreCLR_release-Windows.10.Amd64.Server19H1.ES.Open
1046506	runtime	PR 49336	net6.0-windows-Release-x86-CoreCLR_release-Windows.10.Amd64.Server19H1.ES.Open
1038929	runtime	Rolling	net5.0-OSX-Release-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
1034901	runtime	PR 49512	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
1032466	runtime	PR 49460	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
1030556	runtime	Rolling	net6.0-OSX-Release-x64-CoreCLR_release-OSX.1013.Amd64.Open
1018830	runtime	PR 49006	net6.0-windows-Debug-x64-Mono_release-Windows.81.Amd64.Open
974982	runtime	PR 47732	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
966776	runtime	PR 47349	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
965921	runtime	Rolling	net6.0-OSX-Release-x64-Mono_release-OSX.1013.Amd64.Open
965355	runtime	PR 47437	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
964682	runtime	PR 47128	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
963077	runtime	Rolling	net6.0-OSX-Release-x64-CoreCLR_release-OSX.1013.Amd64.Open
962912	runtime	PR 47359	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
961921	runtime	Rolling	net6.0-OSX-Release-x64-Mono_release-OSX.1013.Amd64.Open
961703	runtime	PR 47149	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
961035	runtime	PR 47125	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open

Build Result Summary

Day Hit Count	Week Hit Count	Month Hit Count
0	0	4

dotnet-issue-labeler · 2021-01-26T20:52:34Z

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost · 2021-01-26T21:58:12Z

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Runfo Tracking Issue: Failing test disableaiaoptionworks on OSX

Build	Definition	Kind	Run Name	Console	Test Results	Run Client
965921	runtime	Rolling	net6.0-OSX-Release-x64-Mono_release-OSX.1013.Amd64.Open
965355	runtime	PR 47437	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
964682	runtime	PR 47128	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
963077	runtime	Rolling	net6.0-OSX-Release-x64-CoreCLR_release-OSX.1013.Amd64.Open
962912	runtime	PR 47359	net5.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open	console.log	test results	runclient.py
961921	runtime	Rolling	net6.0-OSX-Release-x64-Mono_release-OSX.1013.Amd64.Open
961703	runtime	PR 47149	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open
961035	runtime	PR 47125	net6.0-OSX-Debug-x64-CoreCLR_checked-OSX.1013.Amd64.Open

Build Result Summary

Day Hit Count	Week Hit Count	Month Hit Count
2	8	8

Author:	runfoapp[bot]
Assignees:	-
Labels:	`area-System.Security`, `blocking-clean-ci`, `untriaged`
Milestone:	-

vcsjones · 2021-01-31T21:32:47Z

Always failing here. Hmm.

runtime/src/libraries/System.Security.Cryptography.X509Certificates/tests/RevocationTests/AiaTests.cs

Line 74 in 025c4d0

    
           Assert.False(chain.Build(endEntity), "Chain build with no intermediate, AIA disabled");

vcsjones · 2021-01-31T22:32:14Z

@bartonjs

I find a few things odd about this.

The docs say:

By default, network fetch of missing certificates is enabled if the trust evaluation includes the SSL policy. Otherwise it is disabled.

My understanding of X509Chain is that it uses a Basic Policy, not a SSL policy.

It seems that SecTrustSetNetworkFetchAllowed controls all network access during chain building, not just AIA. Meaning this, if I understand correctly, affects revocation checking. From Apple's source:

if (!allowFetch) {
    status = SecTrustSetOptionInPolicies(trust->_policies, kSecPolicyCheckNoNetworkAccess, kCFBooleanTrue);
} else {
    status = SecTrustRemoveOptionInPolicies(trust->_policies, kSecPolicyCheckNoNetworkAccess);
}

Along with this comment:

The current SecTrustServer implementation doesn't distinguish between network
access for revocation and network access for fetching.

and if you look in SecPolicyCreateRevocation:

if (revocationFlags & kSecRevocationNetworkAccessDisabled) {
	CFDictionaryAddValue(options, kSecPolicyCheckNoNetworkAccess, kCFBooleanTrue);
} else {
    /* If the caller didn't explicitly disable network access, the revocation policy
     * should override any other policy's network setting.
     * In particular, pairing a revocation policy with BasicX509 should result in
     * allowing network access for revocation unless explicitly disabled.
     * Note that SecTrustSetNetworkFetchAllowed can override even this. */
    CFDictionaryAddValue(options, kSecPolicyCheckNoNetworkAccess, kCFBooleanFalse);
}

So, based on my understanding then, AIA and revocation fetching are internally controlled by a single flag on the policy, kSecPolicyCheckNoNetworkAccess. It seems... possible then that even if you disable AIA fetching, but want revocation checking, the revocation checking is going to re-enable AIA fetching anyway.

I wrote a quick test for this, and indeed, DisableCertificateDownloads also disables revocation checking.

        [Theory]
        [MemberData(nameof(AllViableRevocation))]
        public static void WhatIsEvenGoingOn(PkiOptions pkiOptions)
        {
            SimpleTest(
                pkiOptions,
                (root, intermediate, endEntity, holder, responder) =>
                {
                    DateTimeOffset now = DateTimeOffset.UtcNow;
                    intermediate.Revoke(endEntity, now);
                    holder.Chain.ChainPolicy.VerificationTime = now.AddSeconds(1).UtcDateTime;

                    // Remove the line below and the test will pass
                    holder.Chain.ChainPolicy.DisableCertificateDownloads = true;

                    SimpleRevocationBody(
                        holder,
                        endEntity,
                        rootRevoked: false,
                        issrRevoked: false,
                        leafRevoked: true);
                });
        }

bartonjs · 2021-02-01T15:49:20Z

I wrote a quick test for this, and indeed, DisableCertificateDownloads also disables revocation checking.

Well, revocation checking won't happen unless it can build a chain to a trust anchor. With AIA off is it completing the chain and just failing as RevocationStatusUnknown?

We can certainly, on macOS, say that if the revocation mode is Online that'll override DisableCertificateDownloads. Not the happiest of states, but it is two caller-controlled elements and probably doesn't come up much.

I guess now I'm confused as to why this ever worked.

vcsjones · 2021-02-01T16:15:20Z

@bartonjs

With AIA off is it completing the chain and just failing as RevocationStatusUnknown?

That appears to be the case. A complete chain is formed, revocation checking is still skipped.

Expected: X509ChainStatusFlags[] [NoError, NoError, Revoked]
Actual:   X509ChainStatusFlags[] [NoError, NoError, RevocationStatusUnknown]

I guess now I'm confused as to why this ever worked.

I'm still trying to get my head around what is happening. I am finding that the documentation around this is on the misleading side, and it's a bit slow going for me to dig through source code.

It seems to me, that (perhaps unrelated from this issue) this unearthed the issue that we should maybe change the macOS chain builder to ignore DisableCertificateDownloads if revocation checking is enabled.

bartonjs · 2021-02-01T19:06:48Z

That appears to be the case. A complete chain is formed, revocation checking is still skipped.

Ah, because I was helpful in SimpleTest. It is already adding the intermediate to chain.ChainPolicy.ExtraStore, so that's the "AIA not needed, but revocation gets impacted" test (maybe you knew that, but I had forgotten). Since I had not assumed that line, I expected a length 1 PartialChain result (though, thinking about it, I'm not sure why I would have assumed the root was in CustomTrustStore without the intermed being in ExtraStore... but that's what I did).

I am finding that the documentation around this is on the misleading side

Maybe it used to be only for AIA and they recently decided that obviously it should also apply to OCSP/CRL? The sudden pass rate change does suggest it was an OS change, not a .NET change, that triggered things. I'm pretty well aware of how easy it is for documentation to be out of date 😄.

bartonjs · 2021-02-01T23:58:20Z

With #47718 merged, I think the action left in this issue is to make the DisableCertificateDownloads test only check NoCheck revocation on macOS (and remove the activeissue attribute).

vcsjones · 2021-02-02T00:10:09Z

@bartonjs yeah, that's sensible. Will open a PR soon.

dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 26, 2021

hoyosjs added area-System.Security blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' labels Jan 26, 2021

runfoapp bot mentioned this issue Jan 26, 2021

Infrastructure - Status/Health #702

Closed

hoyosjs mentioned this issue Jan 26, 2021

Disable DisableAiaOptionWorks on macOS #47497

Merged

hoyosjs added bug disabled-test The test is disabled in source code against the issue os-mac-os-x macOS aka OSX and removed untriaged New issue has not been triaged by the area owner blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' labels Jan 26, 2021

vcsjones mentioned this issue Feb 1, 2021

DisableCertificateDownloads on macOS disables online revocation checking #47713

Closed

vcsjones mentioned this issue Feb 2, 2021

Fix DisableAiaOptionWorks and remove ActiveIssue. #47766

Merged

ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2021

bartonjs closed this as completed in #47766 Feb 2, 2021

ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 2, 2021

ghost locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Failing test disableaiaoptionworks on OSX #47492

Failing test disableaiaoptionworks on OSX #47492

runfoapp bot commented Jan 26, 2021 •

edited

Loading

dotnet-issue-labeler bot commented Jan 26, 2021

ghost commented Jan 26, 2021

vcsjones commented Jan 31, 2021

vcsjones commented Jan 31, 2021 •

edited

Loading

bartonjs commented Feb 1, 2021

vcsjones commented Feb 1, 2021 •

edited

Loading

bartonjs commented Feb 1, 2021

bartonjs commented Feb 1, 2021

vcsjones commented Feb 2, 2021

Failing test disableaiaoptionworks on OSX #47492

Failing test disableaiaoptionworks on OSX #47492

Comments

runfoapp bot commented Jan 26, 2021 • edited Loading

dotnet-issue-labeler bot commented Jan 26, 2021

ghost commented Jan 26, 2021

vcsjones commented Jan 31, 2021

vcsjones commented Jan 31, 2021 • edited Loading

bartonjs commented Feb 1, 2021

vcsjones commented Feb 1, 2021 • edited Loading

bartonjs commented Feb 1, 2021

bartonjs commented Feb 1, 2021

vcsjones commented Feb 2, 2021

runfoapp bot commented Jan 26, 2021 •

edited

Loading

vcsjones commented Jan 31, 2021 •

edited

Loading

vcsjones commented Feb 1, 2021 •

edited

Loading