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

Allow macOS chain building to use network if revocation checking is online #47718

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented Feb 1, 2021

Fixes #47713

The DisableCertificateDownloads property on the chain policy controls all
network activity when building a chain on macOS, not just AIA fetching. If
set to true, the (default) revocation policy would fail because the network
would be treated as unavailable. On macOS, as a work around, permit the
network activity if revocation checking is explicitly enabled.

…nline.

The DisableCertificateDownloads property on the chain policy controls all
network activity when building a chain on macOS, not just AIA fetching. If
set to true, the (default) revocation policy would fail because the network
would be treated as unavailable. On macOS, as a work around, permit the
network activity if revocation checking is explicitly enabled.
@ghost
Copy link

ghost commented Feb 1, 2021

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

Issue Details

Possible solution for #47713

The DisableCertificateDownloads property on the chain policy controls all
network activity when building a chain on macOS, not just AIA fetching. If
set to true, the (default) revocation policy would fail because the network
would be treated as unavailable. On macOS, as a work around, permit the
network activity if revocation checking is explicitly enabled.

Author: vcsjones
Assignees: -
Labels:

area-System.Security

Milestone: -

// There is no way to independently enable or disable online revocation checking
// and AIA fetching. If the caller specifies they want Online revocation checking,
// then we need to allow network operations (including AIA fetching.)
bool revocationRequiresNetwork = revocationMode == X509RevocationMode.Online;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seem to recall macOS does not support Offline. Does Offline act as NoCheck or Online? In other words, should this be:

Suggested change
bool revocationRequiresNetwork = revocationMode == X509RevocationMode.Online;
bool revocationRequiresNetwork = revocationMode != X509RevocationMode.NoCheck;

Copy link
Member

@bartonjs bartonjs Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a question for the ages 😄. I don't know what macOS does with CRL or OCSP caching, so it's hard to know which one is right. Maybe something like

  • With everything on do a revocation check
  • Make sure the OCSP responses include the nextUpdate time (or whatever it's called) and it's at least 5 minutes in the future.
  • Put the network state in no downloads (let Offline + DisableDownloads be "no downloads")
  • Change the mode to Offline
  • Build the chain again

If that succeeds, then there's caching and we want Online vs not. If that fails then we probably decide that we want NoCheck vs not, since we've let Offline be Online on macOS historically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, given this line here, we opt in to the revocation checking polices as long as != NoCheck. It's done that for a while, prior to .NET 5, so Offline was being treated as Online.

SafeCreateHandle policiesArray = PreparePoliciesArray(revocationMode != X509RevocationMode.NoCheck);

Given that, it probably makes sense to keep that behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that, it probably makes sense to keep that behavior.

Well, previously we had two booleans that seemed indepdent:

  • Do revocation checking
  • Networky stuff (interpreted as "use AIA")

If validity-period OCSP means that there's a meaningful "revocation yes, download no" then Offline could be Offline if DisableCertificateDownloads is true, or Online when it's false.

But that's definitely a hard thing to test, document, and maintain, so I'm probably OK with "on macOS Offline means Online".

NoCheck must be used to disable network.
@vcsjones
Copy link
Member Author

vcsjones commented Feb 1, 2021

@bartonjs I will de-draft this since I think this is the right thing to do. If you agree, I would appreciate and outer-loop run to get a sense of what other platforms I might need to chip away at.

My hesitation is around the impact of this. I know DisableCertificateDownloads was done for some internal reasons (but probably less of a concern on macOS...). Perhaps I am being overly cautious.

@vcsjones vcsjones marked this pull request as ready for review February 1, 2021 20:19
@bartonjs
Copy link
Member

bartonjs commented Feb 1, 2021

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bartonjs
Copy link
Member

bartonjs commented Feb 1, 2021

I will de-draft this since I think this is the right thing to do.

Seems good to me.

@vcsjones
Copy link
Member Author

vcsjones commented Feb 1, 2021

Looks like all of the failures are unrelated.

@bartonjs bartonjs merged commit 35f4dad into dotnet:master Feb 1, 2021
@vcsjones vcsjones deleted the 47713-macos-network branch February 1, 2021 23:57
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DisableCertificateDownloads on macOS disables online revocation checking
2 participants