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

Handle IdLinkage X509 chain building status. #41691

Merged
merged 2 commits into from Sep 9, 2020

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented Sep 1, 2020

In macOS, when building an X509 chain, if a leaf and issuing certificate form a chain, but the subject and authority key identifiers do not match, an IdLinkage status is reported.

To match Windows, we ignore this chain status.

Linux appears to use these extensions when building a chain, and reports PartialChain even if the attributes were not strictly needed to aid in building a chain.

Additionally, this updates the debugging output for macOS to use an allocating function to convert a CFString to a C string. CFStringGetCStringPtr will return "NULL if the internal storage of theString does not allow this to be returned efficiently." which it appears to have started doing for me.

Closes #41678

In macOS, when building an X509 chain, if a leaf and issuing certificate
form a chain, but the subject and authority key identifiers do not match,
an IdLinkage status is reported.

To match Windows, we ignore this chain status.

Linux appears to use these extensions when building a chain, and reports
PartialChain even if the attributes were not strictly needed to aid in
building a chain.

Additionally, this updates the debugging output for macOS to use an allocating
function to convert a CFString to a C string. CFStringGetCStringPtr will return
"NULL if the internal storage of theString does not allow this to be returned efficiently."
which it appears to have started doing for me.
@ghost
Copy link

ghost commented Sep 1, 2020

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

@bartonjs
Copy link
Member

bartonjs commented Sep 1, 2020

We should probably port this to 5, but I don't think it'll get accepted into 3.1 or 2.1 without a "why it happens in the real world" problem.

@vcsjones
Copy link
Member Author

vcsjones commented Sep 1, 2020

We should probably port this to 5, but I don't think it'll get accepted into 3.1 or 2.1 without a "why it happens in the real world" problem.

Yeah this is one of the weirder ones. Off the top of my head I think this would be a Mozilla root store policy violation, so unlikely a real CA will issue a cert like this.

@bartonjs
Copy link
Member

bartonjs commented Sep 9, 2020

Apparently I haven't been remembering to look at PR tabs to see if tests finished. There's a merge conflict in this one now :(

@vcsjones
Copy link
Member Author

vcsjones commented Sep 9, 2020

@bartonjs no worries. If you still want to port this one to 5, I don't know what the backport bot will do with the merge commit. If it doesn't work I can manually port.

@bartonjs bartonjs merged commit 2f378cb into dotnet:master Sep 9, 2020
@bartonjs
Copy link
Member

bartonjs commented Sep 9, 2020

/backport to release/5.0-rc2

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2020

Started backporting to release/5.0-rc2: https://github.com/dotnet/runtime/actions/runs/246798667

@vcsjones vcsjones deleted the 41678-fix branch September 9, 2020 18:46
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 7, 2020
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.

macOS: subjectKeyIdentifier and authorityKeyIdentifier mismatch crashes chain building
3 participants