crypto: trim input for NETSCAPE_SPKI_b64_decode

[codebytere]

This PR adds exta handling for spkac decoding if we are using OpenSSL instead of BoringSSL.

In our smoke tests, Electron was finding a consistent failure in parallel/test-crypto-certificate. In tracking this down, I found that the issue originated with a strict requirement in BoringSSL that base64 strings be evenly divisible by 4 in their implementation of NETSCAPE_SPKI_b64_decode - a requirement which Node.js' cert tests were mysteriously failing. It turned out that the cause of the failure was that the fixture files were being read and their newlines left intact, so the number of characters was always a single character over the actual number of characters in the .spkac file.

This fixes that issue by trimming the newlines out of the file cc @jasnell

[tniessen]

For posterity, the issue arises from the fact that, within NETSCAPE_SPKI_b64_decode,

• OpenSSL uses EVP_DecodeBlock, which explicitly removes trailing whitespace etc.,
• BoringSSL uses EVP_DecodedLength and EVP_DecodeBase64, which do not remove trailing whitespace etc.

Personally, I don't think adapting the way we read keys to be compatible with BoringSSL's implementation details is the best solution.

Also, the reason for this change and for trimming the keys is difficult if not impossible to determine without going through the git history.

Lastly, if the behavior of OpenSSL or BoringSSL ever changes to match that of the other library, we won't notice if we always trim the keys in our tests (but real applications don't).

I'd personally prefer to

• either adapt the specific test to trim the strings only if the crypto library is BoringSSL, or
• change the implementation in src to trim the input if the crypto library is BoringSSL.

Whichever option we go with, the reason for the trimming should be explained in the code.

[tniessen]

(Or, if BoringSSL is closer to being "correct" than OpenSSL, then we should adopt BoringSSL's behavior.)

[codebytere]

@tniessen updated with the second approach you suggested!

[tniessen]

@tniessen updated with the second approach you suggested!

Thank you @codebytere, I think this is a better solution overall 😃

(FWIW, I believe that EVP_DecodeBlock will also refuse inputs that start with whitespace, but if that never occurs in practice, there is no reason to "fix" that.)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/40914/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/40915/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/40932/

[codebytere]

Landed in f7668fa