Skip to content

[release/6.0] Require UTF8 encoding in GetX509NameInfo - #59172

Merged
danmoseley merged 1 commit into
release/6.0-rc2from
backport/pr-59116-to-release/6.0
Sep 17, 2021
Merged

[release/6.0] Require UTF8 encoding in GetX509NameInfo#59172
danmoseley merged 1 commit into
release/6.0-rc2from
backport/pr-59116-to-release/6.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

Backport of #59116 to release/6.0

/cc @bartonjs@vcsjones

Customer Impact

Customers trying to inspect metadata, on Linux, on an X.509 certificate where the subject CN or issuer CN contains non-ASCII data. Instead of the .NET string containing the non-ASCII characters correctly, the string contains the non-ASCII characters literally encoded into the string using the \U0123 format (with a capital U).

The most common use of a certificate CN is a domain name, which is why this got overlooked in testing. But email (S/MIME) is usually someone's name, and certificate authorities use descriptive names.

Reported externally, via https://stackoverflow.com/questions/69105533/net-5-x509certificate-getnameinfo-uppercases-unicode-escape-characters

Testing

ASCII data is covered by existing tests.
Non-ASCII data, specifically Cyrillic, has been added in a test with this PR.

Risk

Low, based on the existing and new tests, and scoped change.

@ghost

Copy link
Copy Markdown

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

Issue Details

Backport of #59116 to release/6.0

/cc @bartonjs@vcsjones

Customer Impact

Testing

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.Security

Milestone:-

@bartonjsbartonjs added Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review labels Sep 15, 2021
@danmoseleydanmoseley added the Servicing-consider Issue for next servicing release review label Sep 15, 2021
@danmoseley

Copy link
Copy Markdown
Contributor

This missed the branch so it will go to tactics. Marked servicing-consider.

@danmoseleydanmoseley added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 16, 2021
@danmoseleydanmoseley reopened this Sep 16, 2021
@danmoseley
danmoseley changed the base branch from release/6.0 to release/6.0-rc2September 16, 2021 20:38
@danmoseley
danmoseley merged commit 5f384ac into release/6.0-rc2Sep 17, 2021
@danmoseley
danmoseley deleted the backport/pr-59116-to-release/6.0 branch September 17, 2021 02:18
@danmoseley

Copy link
Copy Markdown
Contributor

There were a lot of canceled lanes but as far as I can tell that's an artefact of the rebase or close/reopen.

@ghostghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.SecurityServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danmoseley@bartonjs@vcsjones