Implement PEM exports for RSA PKCS#1 and ECPrivateKey #61946
Conversation
This implements PEM exports for ECPrivateKey, and RSA PKCS#1 public and private keys.
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks |
| byte[] exported = ExportECPrivateKey(); | ||
|
|
||
| // Fixed to prevent GC moves. | ||
| fixed (byte* pExported = exported) |
There was a problem hiding this comment.
If the key was already exported to the array above, this is too late to guarantee no copies have been made.
There was a problem hiding this comment.
That is true but it's a pattern that occurs quite frequently in other places as a "best effort". Two examples:
But there are many more.
| static bool Export(ECAlgorithm alg, Span<byte> destination, out int bytesWritten) | ||
| { | ||
| return alg.TryExportECPrivateKey(destination, out bytesWritten); | ||
| } | ||
|
|
||
| return PemKeyHelpers.TryExportToPem( | ||
| this, | ||
| PemLabels.EcPrivateKey, | ||
| Export, | ||
| destination, | ||
| out charsWritten); |
There was a problem hiding this comment.
Does Export need to be a local function? Why not just do it inline as a static lambda below and enable the compiler to cache the delegate?
There was a problem hiding this comment.
It looked cleaner this way (to me) and it's not a particularly performance sensitive function. As a lambda it would look like:
return PemKeyHelpers.TryExportToPem(
this,
PemLabels.EcPrivateKey,
static (ECAlgorithm alg, Span<byte> destination, out int bytesWritten) =>
alg.TryExportECPrivateKey(destination, out bytesWritten),
destination,
out charsWritten);If we want to avoid the delegate allocation, I can fix this up and in a few other places that have already been done this way.
| byte[] exported = ExportRSAPrivateKey(); | ||
|
|
||
| // Fixed to prevent GC moves. | ||
| fixed (byte* pExported = exported) |
There was a problem hiding this comment.
Same comment as earlier. This will prevent exported from being moved from this point on, but it could have already been moved any number of times before this point.
Closes #51630. This is a replacement PR for #61487.