fix: Enable X509_V_FLAG_TRUSTED_FIRST flag in BoringSSL

[jviotti]

This flag is set by default on OpenSSL.

Fixes: #31212
Signed-off-by: Juan Cruz Viotti jv@jviotti.com

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fix Let's Encrypt DST Root CA X3 certificate expiration.

[jviotti]

We are planning to work on a test case, but probably as another PR just to get this one merged given its urgency

[MarshallOfSound]

I'm somewhat concerned that the bug only impacts the nodejs http module but this patch impacts every consumer of boringssl. Is there any way to patch this in to node specifically?

[codebytere]

cc @davidben - do you think this is the right fix?

[jviotti]

@MarshallOfSound

I'm somewhat concerned that the bug only impacts the nodejs http module but this patch impacts every consumer of boringssl. Is there any way to patch this in to node specifically?

Yeah, I get your point. On the other hand, every consumer of BoringSSL that doesn't overwrite the default flags would face the same problem, right? To your point, Chromium does seem to work by default, so I guess they already overwrite it in all places?

[clavin]

Compiled a test build with this patch. I can confirm I was able to successfully make a request to https://letsencrypt.org using https.get with this patch from node in the main process.

[laurent22]

If you merge this please consider backporting the fix to previous Electron versions. In our case, we are somewhat stuck at version 10 due to the Electron sandbox changes. We'll upgrade some day but it's not trivial, and I'm sure many people likewise don't have the resources to upgrade to a major version, yet would certainly consider upgrading to a patch version.

[VerteDinde]

@laurent22 We're planning on backporting the fix into all of our current stable release lines (Electron 12-15) and beta line (Electron 16) 👍 If people reading have concerns, let's discuss in the original issue so we don't distract from any fix discussions here 🙂

[MarshallOfSound]

Approved ref: https://electronhq.slack.com/archives/C5VT8SQ8K/p1633033212040400?thread_ts=1633022658.031900&cid=C5VT8SQ8K

[release-clerk]

Release Notes Persisted

Fix Let's Encrypt DST Root CA X3 certificate expiration.

[trop]

I have automatically backported this PR to "12-x-y", please check out #31214

[marxin]

Are you planning to make the change in the upstream BoringSSL library:
https://boringssl.googlesource.com/boringssl
?

[deepak1556]

It is being tracked here https://bugs.chromium.org/p/boringssl/issues/detail?id=439

[GermanCoding]

Yeah, I get your point. On the other hand, every consumer of BoringSSL that doesn't overwrite the default flags would face the same problem, right? To your point, Chromium does seem to work by default, so I guess they already overwrite it in all places?

Chromium has it's own verifier code and doesn't use BoringSSL at all for this purpose - which is also why it's not affected..

[Ciberusps]

@VerteDinde

Do u plan to backport fix for electron v8 & v11 also?

[zeeker999]

Could you please backport this to v11 as well? Our product is still using the v11 since some bugs(for example #30666 and #31016) are unfixed and we can't upgrade to the latest stable for the moment.

Sorry to ping you, I'm not sure to ping which one since this bug should affect all stable releases and the electron team should take action now.
@MarshallOfSound

[foxt]

We're also requesting a backport to V9 if at all possible.