Skip to content

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

Merged
bartonjs merged 2 commits into
dotnet:masterfrom
vcsjones:47713-macos-network
Feb 1, 2021
Merged

Allow macOS chain building to use network if revocation checking is online#47718
bartonjs merged 2 commits into
dotnet:masterfrom
vcsjones:47713-macos-network

Conversation

@vcsjones

@vcsjones vcsjones commented Feb 1, 2021

Copy link
Copy Markdown
Member

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 ghost added the area-System.Security label Feb 1, 2021
@ghost

ghost commented Feb 1, 2021

Copy link
Copy Markdown

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
Copy Markdown
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;

@bartonjs bartonjs Feb 1, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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

vcsjones commented Feb 1, 2021

Copy link
Copy Markdown
Member Author

@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

bartonjs commented Feb 1, 2021

Copy link
Copy Markdown
Member

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@bartonjs

bartonjs commented Feb 1, 2021

Copy link
Copy Markdown
Member

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

Seems good to me.

@vcsjones

vcsjones commented Feb 1, 2021

Copy link
Copy Markdown
Member Author

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