caddy icon indicating copy to clipboard operation
caddy copied to clipboard

Support custom certificate validity period for certificate issuance

Open clauverjat opened this issue 7 months ago • 9 comments

What would you like to have changed?

It'd be great if users could configure the certificate validity period when requesting certificates from Certificate Authorities (CAs).

Why is this feature a useful, necessary, and/or important addition to this project?

The default validity period for certificates issued by most Public CAs, such as Let's Encrypt and ZeroSSL, is currently 90 days. However, there are scenarios where shorter certificate lifetimes are useful or required. A shorter validity period can minimize the window of exposure in case the cert's private key is compromised. I believe short certificate lifetimes are especially important since certificate revocation doesn't work (at least not reliably enough to rely on it for serious security on the web).

What alternatives are there, or what are you doing in the meantime to work around the lack of this feature?

The alternative for users that want certificates with shorter lifetime would be to generate the certificate out of Caddy and then manually load those certificates into Caddy. But doing so means that you don't benefit from Caddy "automatic HTTPS", this means that you'll need to manage the renewal of the certs for instance.

For my usecase, I've patched caddy, certmagic and acmez to add this feature when using an ACME issuer, but of course this requires me to compile my own custom version of caddy and then maintain it.

Context

For ACME certificate issuance, the desired certificate lifespan can be configured by setting the "notAfter" field in the order. Presently, popular CAs like Let's Encrypt and ZeroSSL do not support this feature. However, Google Public CA does offer this flexibility, and Caddy's own ACME server supports customizable validity periods (within bounds).

If you're interested, I'd be happy to share with you the modifications I've made to implement this feature. But please note that it's my first time working on the Caddy codebase, so my implementation might not be ideal.

clauverjat avatar Dec 15 '23 15:12 clauverjat

Sure -- it should be fairly trivial. Want to share your changes?

(Acmez should already support this by virtue of taking a CSR as input.)

mholt avatar Dec 15 '23 16:12 mholt

Indeed the change is mostly about reading the option from the Caddyfile and then passing it down the stack up to acmez. I don't think Acmez can achieve that already though. You need to set the "notAfter" field of the order object as described by the RFC ^1 (at least that's what I did), thus I had to add an argument to the ObtainCertificateUsingCSRSource function.

Here are my forks with the changes : https://github.com/mithril-security/caddy/tree/add-cert-lifetime-config https://github.com/mithril-security/certmagic/tree/add-cert-lifetime-config https://github.com/mithril-security/acmez/tree/add-cert-lifetime-config

Tests done

I tested the change using Caddy own ACME server.

localhost:9443 {
    acme_server {
        
    }
}

Test of a Caddyfile with global settings for cert lifetime

{
    acme_ca https://localhost:9443/acme/local/directory
    # global option to ask for certificate with a validity of 3 days
    cert_lifetime 3d
}

example.com {
}

Caddyfile with only site directive :

{
    acme_ca https://localhost:9443/acme/local/directory
}

example.com {
    tls {
        issuer acme {
            lifetime 2d
        }
    }
}

Caddyfile with both site and global set, to make sure site overrides global settings.

{
    acme_ca https://localhost:9443/acme/local/directory
    cert_lifetime 5d
}

example.com {
    tls {
        issuer acme {
            lifetime 2d
        }
    }
}

Limitations

The global "cert_lifetime" option that I added is only used by the ACMEIssuer. People could get confused that it does not apply to other Issuer. For instance it does not impact the internal issuer, which might be counterintuitive since the internal issuer has a "lifetime" option.

clauverjat avatar Dec 15 '23 17:12 clauverjat

Thanks for the patch info!

Looks about like what I expected, and it turns out I was wrong about CSR containing lifetime information. I coulda sworn it did but I guess not :man_shrugging:

I think overall the changes are the same idea as how I would do it, I'll get back to this soonish, I might take some time off for the holidays :innocent:

(The change to acmez would likely necessitate a v2)

mholt avatar Dec 16 '23 20:12 mholt

You're welcome.

Looks about like what I expected, and it turns out I was wrong about CSR containing lifetime information. I coulda sworn it did but I guess not 🤷‍♂️

Indeed, I get why you might have thought that, the certificate has a notAfter field so it's somewhat natural to think that the CSR also has one. I am not quite sure why they chose not to include this value in the CSR actually. Looking a little bit into it, it looks mostly incidental. The RFC that defines the CSR did not include that, so when they did ACME they had to provide that information outside the CSR, and they put it in the ACME order... why not ?

I think overall the changes are the same idea as how I would do it, I'll get back to this soonish, I might take some time off for the holidays 😇

Great, I will also be on vacation next week. Since this will likely require a change to acmez, do you have an idea of the time it will take before the feature is included in a release of caddy? There is no rush, but having an idea would be helpful on my side.

A few thoughts: I thought back on the changes that I have suggested. The implementation I did was kind of the minimal work needed to make it work. I think a possible improvement would be take a notAfter date in acmez library instead of a validity period. It would make for purer interface (in the functional sense, because the function won't depend on the system clock) and be closer to what is sent to the ACME server. Of course, the caddyfile would still take a validity period. Also in acmez if one needs to do a new version, one might also add the notBefore date as an argument, I don't have a usecase for it, but it adds symmetry and it would avoid another new version if one needed it in the future.

clauverjat avatar Dec 21 '23 14:12 clauverjat

We need this feature, especially in browsers using the chromium kernel, the SSL certificate is always cached, and you will see the browser's certificate expiration error message every few days.

fangzhengjin avatar Dec 25 '23 05:12 fangzhengjin

If we can control how far in advance the certificate is reissued before it expires, we may be able to avoid the error message caused by chromium's cached certificate.

fangzhengjin avatar Dec 25 '23 05:12 fangzhengjin

Yeah, I'm all for this, but I need to figure out how I want to do the acmez patch, whether it's a v2 or I try to track down users of acmez and if it only impacts a few people (i don't think many people are using it), maybe we just make the change in v1 so that those users will continue getting patches without changing their source code.

mholt avatar Dec 30 '23 18:12 mholt

Hi @fangzhengjin and happy new year to everyone !

If I understand correctly what you want : controlling how far in advance a certificate is reissued before it expired, then this issue won't solve it. This issue is about adding a parameter to control how long the certificate should be valid. If you shorten (like I want for my usecase) the lifetime of your certificate, you'll likely worsen the issue that might happen with certificate caching. It looks like you actually want to modify the "renewal window ratio" i.e. how long before a certificate's expiration to try renewing it, as a function of its total lifetime. You can have more information about this parameter looking into the caddy source code, https://github.com/caddyserver/caddy/blob/8a50f191bf3e4e60fdd51753f54604735f9787e4/modules/caddytls/automation.go#L126 Also it seems the RenewalWindowRatio can already be configured using Caddy JSON config (but not the Caddyfile?).

I might have misunderstood something but anyway I wanted to point that out so that people won't get confused by that discussion

clauverjat avatar Jan 02 '24 10:01 clauverjat

Yeah, I'm all for this, but I need to figure out how I want to do the acmez patch, whether it's a v2 or I try to track down users of acmez and if it only impacts a few people (i don't think many people are using it), maybe we just make the change in v1 so that those users will continue getting patches without changing their source code.

@mholt I'm one of the users in some very small (personal, pet) projects and we use it too in some Smallstep stuff. The change in acmez looks small enough to be able to fix quickly when just updating the dependency, so personally I wouldn't worry too much about that.

That said, maybe it's nicer if instead of being able to provide the single validity time as an argument, the argument becomes a variadic argument, taking a func(o *acme.Order) error, or something like that. Depending on the guarantees you want to provide to users of the package, that may need to be gated with some additional validation logic. For example, being able to operate on the *acme.Order means more things could be overridden, potentially erroneously. That maybe needs to be blocked from happening. One way could be as follows:

type OrderOpts struct {NBF, NAF}
type OrderOption func(o *OrderOpts) 
func WithDuration(d time.duration) func(o *OrderOpts) {
    return func(o *OrderOpts) {
      o.NAF: time.Now().Add(d)
    }
}

func (c *Client) ObtainCertificateUsingCSRSource(ctx context.Context, account acme.Account, identifiers []acme.Identifier, source CSRSource, opts ...OrderOption) ([]acme.Certificate, error) {
    oo := &OrderOpts{
        NBF: defaultNBF,
        NAF: defaultNAF,
    }
    for _, applyTo := range(opts) {
        applyTo(oo)
    }
    
    order := &acme.Order{nbf: oo.NBF, naf: oo.NAF}
}

hslatman avatar Jan 02 '24 13:01 hslatman

Just to update anyone following over here in this repo, this is currently being worked on in 2 fronts:

  • ACMEz v2: https://github.com/mholt/acmez/pull/23
  • ZeroSSL API: https://github.com/caddyserver/zerossl (their API allows configurable cert lifetimes but their ACME endpoint does not)

Once ACMEz v2 lands, we can update CertMagic, and Caddy, accordingly.

mholt avatar Mar 07 '24 01:03 mholt

@clauverjat I've updated ACMEz and CertMagic to support this, now it just needs to be added to Caddy. Would you like to submit a PR for your Caddy patch? (But with the latest CertMagic/ACMEz APIs? I haven't made a release yet for CertMagic, so use master.) If not I will come up with something, probably similar. But I'd like for you to get credit.

mholt avatar Apr 18 '24 22:04 mholt

Thanks @mholt. I just submitted a PR with an updated patch that uses CertMagic master.

clauverjat avatar Apr 19 '24 17:04 clauverjat