Fix OpenSSL 3 reporting an OutOfMemoryException for missing private key

[vcsjones]

Contributes to #63624.

This, with #63655, gives me a green run of S.S.Cryptography in Fedora Rawhide with OpenSSL 3.

[ghost]

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

Issue Details

---

Contributes to #63624.

This, with #63655, gives me a green run of S.S.Cryptography in Fedora Rawhide with OpenSSL 3.

Author:	vcsjones
Assignees:	-
Labels:	area-System.Security
Milestone:	-

[bartonjs]

In the malloc error case here, do we need to manipulate the queue in any way? (e.g. is this checking the same end of the queue that we use as the basis of a throw?)

[vcsjones]

e.g. is this checking the same end of the queue that we use as the basis of a throw?

It's not, because that is what we are trying not to do. If we check the same end as the basis of the throw, then we are going to throw an OutOfMemoryException. We can't tell the difference between an OOM and "no private key". I though the next best thing we could do is check to see if the most-recent error thrown is an OOM. In that case, since we are peeking, it Weill just get thrown as-is.

Unfortunately I could not find a lot of flexibility in OpenSSL's error APIs. There is no way to examine the queue without consuming it, beyond peeking at the most recent item. Also, there is no way to determine the depth of the queue, as far as I could tell.

[bartonjs]

I think we're talking about opposing conditions 😄

If the error isn't malloc, we return -2. The caller special cases that throw.

If the error is malloc, we return -1. The caller then throws an exception. I believe we want it to throw OOM, and I'm just making sure that the error reported by ERR_peek_error() is the one we're going to end up with in the throw... but I believe that we currently throw ERR_peek_last_error().

We could bridge that gap with something like

const char* file;
const char* line;
ERR_peek_error_line(&file, &line);
ERR_put_error(ERR_GET_LIB(error), ERR_GET_FUNC(error), ERR_GET_REASON(err), file, line);
return -1;

which would copy the head to the tail (until we revisit our exception model)

[vcsjones]

(until we revisit our exception model)

Ah. Okay. I was predicating my fix on that eventually happening. If we want to explicitly make sure we throw the OOM until then, that's fine.

[bartonjs]

That's fair. Hmm. I think we should be defensive now, just so we don't end up getting preview bug reports for exceptions that make no sense.

Alternatively, make the managed caller just explicitly throw OOM on -1, and TODO it against #63804

[vcsjones]

Well, ERR_put_error is deprecated in OpenSSL 3 and it seems silly to use it in an OpenSSL 3-only code path.

<sigh>.

[vcsjones]

Well, we don't really need to preserve the line. That never appears in the OutOfMemoryException.

[vcsjones]

Okay, I noticed that we have our own ERR_put_error. I think that is what you're looking for? (ERR_GET_FUNC was removed in OpenSSL 3).

[bartonjs]

/backport to release/6.0

[github-actions]

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1714595694

[github-actions]

@bartonjs backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix OpenSSL 3 reporting an OutOfMemoryException for missing private key.
Using index info to reconstruct a base tree...
M	src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs
A	src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c
A	src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.h
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs
CONFLICT (content): Merge conflict in src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix OpenSSL 3 reporting an OutOfMemoryException for missing private key.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[vcsjones]

@bartonjs I will manually back port this one.