Configurable certes verification certificates

[Mafii]

Usage of the new API:

options.AdditionalIssuers = new[] { File.ReadAllText("letsencrypt-stg-root-x2.pem") };

When starting with e.g. the Staging Root certificate from Let's Encrypt, Lettuce Encrypt will still issue the following warning in the end:

Failed to validate certificate for [certificate...]. This could cause an outage of your app.

This can be ignored.

Why is this needed? What value does this PR add?

• Certes has a hard-coded amount of trusted root certificates.
  • See https://github.com/fszlin/certes/tree/main/src/Certes/Resources/Certificates
  • That those are hardcoded, which is not optimal, is also mentioned here: https://github.com/fszlin/certes/pull/233#issuecomment-726581168
  • The x1 staging certificate is embedded in certes' dll, but it is outdated and unused by Let's Encrypt today.
• Certes has an API of AddIssuer(s) that can be used to add Certificates to that list of embedded certificates.
• Exposing this API through Lettuce Encrypt allows it to be configured for any other ACME/certes compatible service - including the current Let's Encrypt staging server, without updating certes' embedded certificates.
• This PR probably reduces the future maintenance effort of Lettuce Encrypt and/or amount of Issues. There are a certain amount of forks that do nothing other than adding such root certificates (in the same place as this PR), but hard-coded.

Related issues

This PR is likely the "extension point" you talked about in this comment: https://github.com/natemcmaster/LettuceEncrypt/pull/230#issuecomment-967759204

This PR may partially solve issues such as #274, I am not very confident in that assessment however.

It may also make #263 possible - I don't think your proposed solution was sufficient as certes should to my understanding throw because the google acme server's root certificate isn't well known. I may be wrong here as well.

It for sure solves #278, as I have tested that myself. The pull requests #276 and #277 were initial steps/solutions to getting LettuceEncrypt running in a project using the staging server with ngrok (v3) and the current version of LettuceEncrypt. I would recommend taking a look at them as well.

Additionally, once the dst-root-ca-x3.pem expires in 2030 and the isrg-root-x1.pem expires in 2035, Lettuce Encrypt will have to upgrade certes (if certes adds the new certificates as embedded resources) or manually pass in the new Let's Encrypt root certificate every time. This change will be less dramatic/annoying with this PR, as users of Lettuce Encrypt can workaround/fix this "problem" themselves without having to upgrade any packages to a new version.

Alternative Architecture considerations

Another way of adding the functionality contained in this PR would be to add the additional certificates to ICertificateAuthorityConfiguration, as it kind of belongs there. I realized this looking at a comment in #263.

[Mafii]

@natemcmaster if you prefer, this feature could also be adjusted to only be available in staging mode (logging a warning when used within a production environment, e.g. "... was not applied as it is only available in staging mode"). I don't think the current API has any security implications, but it would be the more defensive way of doing this, ensuring no production environment is impacted.

I am willing to change any parts this PR as you see fit - feel free to add any feedback if you like, even if the PR is not merged in the end.

[codecov[bot]]

Codecov Report

Merging #279 (da0ef28) into main (1084963) will decrease coverage by 0.07%. The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
- Coverage   46.23%   46.16%   -0.08%     
==========================================
  Files          45       45              
  Lines        1116     1120       +4     
==========================================
+ Hits          516      517       +1     
- Misses        600      603       +3     

Impacted Files	Coverage Δ
.../LettuceEncrypt/Internal/AcmeCertificateFactory.cs	0.00% <0.00%> (ø)
src/LettuceEncrypt/LettuceEncryptOptions.cs	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[natemcmaster]

Thanks @Mafii. This PR seems fine to me as is. There is one additional thing I think would be useful.

Another way of adding the functionality contained in this PR would be to add the additional certificates to ICertificateAuthorityConfiguration, as it kind of belongs there.

What do you think about doing both things? I could see it being useful to have both the ability to manually add issuers, and also to have your CA configuration provide this. I'm guessing that addressing #263 would be a matter of adding a class which implements ICertificateAuthorityConfiguration and provides the issuer certs required.

Thoughts?

[Mafii]

@natemcmaster thanks so much! I just tried to add this to the ICertificateAuthorityConfiguration interface, and stumbled upon a (sadly classic) problem with dotnet standard 2.0 targeting - there are no default interface implementations available.

I've considered some options:

Option 1: Source and Runtime breaking change for standard 2.0, but not for core 3.1 and later:

Reasonably, this means LettuceEncrypt v2.0 which seems overkill for the (arguably useful but limited in functionality) value added.

We could lie about the version, but this is problematic for standard 2.0 users. I don't like this option.

    /// <summary>
    /// Certificates passed to certes before building the successfully downloaded certificate,
    /// used internally by certes to verify the issuer for authenticity.
    /// </summary>
    /// <seealso cref="LettuceEncryptOptions.AdditionalIssuers" />
    public string[] IssuerCertificates
    #if NETSTANDARD2_0
    { get; }
    #else
    => Array.Empty<string>();
    #endif

Option 2: Introduce a new interface

Downside: Adding it to ICertificateAuthorityConfiguration means it is "findable" in the IDE when implementing it into a class. A new interface is harder to find. Upside: No breaking changes.

Option 3: We avoid this feature... but only for standard 2.0

The functionality can be used through the options already - this is some inconvenience for those limited to standard 2.0, but allows better usage for most users, and all new users that have no restrictions in target frameworks.

Downside: It is not that funny to have different APIs for different target frameworks?

---

What do you think?

[natemcmaster]

I’m actually ok with the idea of dropping netstandard2.0. Here’s my thinking:

I added .NET Standard when this library began 5 years ago because, at the time, there was still some levels of interest in getting ASP.NET Core libraries to work on .NET Framework. I think it has been long enough now since https://github.com/aspnet/Announcements/issues/324 that I’m ok entirely dropping .NET Standard.

In fact, I’m okay jumping straight to net6. .NET Core 3.1 is now EOL, as is net5. Yes, this a breaking change, but also, I am okay not supporting old frameworks versions which even Microsoft doesn’t support anymore.

[Mafii]

@natemcmaster alright, good to hear & makes sense. I'll provide a PR when I find time!

[webprofusion-chrisc]

Btw, the root cause (pun!) is that the bouncy castle PFX builder needs a known intermediate or root to verify the chain from, but it turns out you don't actually need the root, just the issuer closest to the root.

See my certes fork here (Webprofusion.Certes) as used by Certify The Web. It's starting to diverge rather a lot from the original and I haven't proven there are no side effects of this approach yet, but the PFX does build without the root issuer being added: https://github.com/webprofusion/certes/commit/16dc9f7862fa8b2ef42e6e79a8a0b0a5ea11bae7#diff-3c1160ec9eab4ad6ed521d35d890a3cf0d4d1ab967db59fca986b06697970d3aL145