macOS: Allow loading first X509 certificate from PEM sequence

[vcsjones]

AppleCryptoNative_X509GetContentType is currently returning PAL_X509Unknown when attempting to load (or use GetCertContentType) on a file that contains multiple PEM encoded certificates.

Windows and OpenSSL currently treat it as a single certificate by only using the first certificate in the file. This fixes AppleCryptoNative_X509GetContentType to return PAL_Certificate if it is an aggregate of X509 certificates and match the behavior of other platforms.

Fixing also fixes loading the certificate since macOS isn't ambiguous as to what is trying to be done, so it starts loading the first certificate in the PEM sequence as well.

Fixes #29910

/cc @bartonjs

[vcsjones]

@dotnet-bot test this please (Doesn't look related to my changes...)

[vcsjones]

/azp help

[azure-pipelines]

Supported commands
     help:
          Get descriptions, examples and documentation about supported commands
          Example: help "command_name"
     run:
          Run all pipelines or a specific pipeline for this repository using a comment. Use
          this command by itself to trigger all related pipelines, or specify a pipeline
          to run.
          Example: "run" or "run pipeline_name"

See additional documentation.

[vcsjones]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[vcsjones]

... that did not do what I expected it to do. @safern halp?

[safern]

... that did not do what I expected it to do. @safern halp?

"/azp run" without specifying a build definition, will run all including outerloop builds.

if you want to just run corefx-ci you have to do "/azp run corefx-ci" -- running an individual leg i.e corefx-ci (Windows -x64 Debug) is not yet supported 😢

The reason why jobs show as duplicated when you run them, is because azure devops has a bug which I reported, please see this thread, where I explain the bug #35126 (comment)

I'm working on adding the docs to explain all this, just been doing it in parallel with other stuff I have to do. I expect it today or tomorrow to be done.

[vcsjones]

@safern

if you want to just run corefx-ci you have to do "/azp run corefx-ci"

OK, makes sense. I'll be sure to do that unless I intend on running outer loop.

The reason why jobs show as duplicated when you run them

Ah I didn't even notice they were dupes. I thought my PR was being validated against Windows ME or something.

[vcsjones]

@bartonjs

OK, results are in. Windows and OpenSSL are not happy with a concatenated private key + certificate, but macOS is. I can either make the tests PlatformSpecific for macOS, or remove the test altogether.

I also tried adding a test for X509Certificate2.GetCertContentType when you give it jibberish since I saw there were no negative tests for it that I could see. I initially wrote it expecting that it would return Unknown since that is what macOS is doing. However Windows and Unix throw. Looking at the Unix source, it looks like it throws intentionally to match Windows' behavior. It seems then that the macOS PAL should be throwing if Unknown is returned.

Thoughts?

[bartonjs]

OK, results are in. Windows and OpenSSL are not happy with a concatenated private key + certificate, but macOS is. I can either make the tests PlatformSpecific for macOS, or remove the test altogether.

It'd be nice if it could also fail on macOS, I guess; but I'm fine with "delete the test" and let it be a weird corner case of success.

I also tried adding a test for X509Certificate2.GetCertContentType when you give it jibberish since I saw there were no negative tests for it that I could see. I initially wrote it expecting that it would return Unknown since that is what macOS is doing. However Windows and Unix throw. Looking at the Unix source, it looks like it throws intentionally to match Windows' behavior. It seems then that the macOS PAL should be throwing if Unknown is returned.

Throwing-to-match sounds good. We could debate if it should return unknown instead of throw, but matching seems nicest.

[vcsjones]

/azp run corefx-ci 🤞

[azure-pipelines]

No pipelines are associated with this pull request.

[safern]

LOL, the emoji is causing it to not find the pipeline because it takes everything after "run", therefore it thinks the emoji is part of the name.

[vcsjones]

/azp run corefx-ci

🤞🤞

[azure-pipelines]

No pipelines are associated with this pull request.

[vcsjones]

/azp run corefx-ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[vcsjones]

@bartonjs this seems "green" now with feedback changes.

[bartonjs]

The corefx-ci failure seems to have been an infrastructure error after the tests succeeded,.

[stephentoub]

Thanks.