proposal: x/crypto/acme: support the ACME Renewal Information standard in Client

[jmhodges]

(This could be viewed as a preliminary ticket because, while Let's Encrypt supports the API in question and there are multiple ACME clients supporting it, the standard hasn't been finalized at time of creation. Recent events described below made me want to write this out.)

It would be nice for x/crypto/acme to give consumers of it a convenient way of knowing when their certificates need renewal especially when their CAs have revoked those certificates early.

One of the outcomes of the recent Let's Encrypt outage was a recognition that many ACME clients aren't renewing certificates that were revoked by the CA. @agwa wrote a very nice piece on the outage, the renewal problem, and how ACME clients might solve it.

In that post, he mentions that Let's Encrypt currently supports the still-under-development ACME Renewal Information (ARI) standard. That standard and API allows ACME clients to hear from the CA that they need to renew a certificate they previously made. That renewal information also, helpfully, gives guidance on when to attempt a renewal even during normal operation.

It would be nice for x/crypto/acme to support the ARI standard.

One version of this work would be to:

1. add a new method to acme.Client called GetRenewalInfo accepting a x509 certificate that it extract the necessary CertID from, and returning a RenewalInformation struct with the suggested liveness window and explanatory URL
2. add a new method to acme.Client called UpdateRenewalInfo accepting a x509 certificate (a more minimal version would not include this method as its not required for correct operation of GetRenewalInfo)

This ticket includes a proposed API below. The method and type names below are meant only as reasonable first attempts at naming and others might have other better names.

The word "CertID" has specific meaning and importance here. The ARI APIs takes the certificate's "DER-encoded CertID ASN.1 sequence" (link). See section 4.1.1 of RFC6960 for the definition of "CertID". A CertID is a combination of what are currently multiple fields in crypto/x509.Certificate. That means GetRenewalInfo and UpdateRenewalInfo, unlike RevokeCert, needs to parse the x509 certificate given to it.

While GetRenewalInfo and UpdateRenewalInfo could take some CertID bytes or struct, that combination of data and encoding is a bit more complicated than we could expect users to re-create well. So, we're left with the choice of taking a []byte of the certificate that is parsed under the hood, or a *x509.Certificate.

A []byte for the cert seems preferable to a *x509.Certificate. First, because the []byte type fits the pattern set by RevokeCert. Second, a []byte would also likely result in less code written by most consumers as most don't care about the individual bytes inside a cert and would be passing it directly from their storage layer. Third, the []byte also avoids them having to learn how to parse x509 well. However, specifying the full x509.Certificate type might better convey what's being requested from the consumer and avoid this library from having to inform the consumer when their x509 certificate is ill-formed. On balance, the []byte seems better, but I'm open to hearing I'm wrong.

One last note: GetRenewalInfo doesn't require its request payload to be signed with the correct account key, but UpdateRenewalInfo does.

type SuggestedWindow struct {
  Start time.Time
  End time.Time
}

type RenewalInformation struct {
  SuggestedWindow SuggestedWindow
  ExplanationURL string
} 

func (c * Client) GetRenewalInfo(ctx context.Context, cert []byte) (*RenewalInformation, error)

func (c * Client) UpdateRenewalInfo(ctx context.Context, cert []byte) (error)

[ianlancetaylor]

CC @golang/security

[FiloSottile]

ARI is great, and we should definitely support it. What worries me is that the draft is not final and we already had one round of draft-to-RFC API churn in x/crypto/acme. Any sense how long that might take? If it's years I guess we'd have to bite the bullet.

I'd skip UpdateRenewalInfo. It's not necessary for most uses, and feels more likely to change than GetRenewalInfo. Also, I had to look at the spec to learn what it does. (Tells the server if the certificate has been renewed, and the bool SHOULD always be true, which is kinda weird.)

Nit, I wouldn't hoist SuggestedWindow out as its own type myself.

[AGWA]

Agree with @FiloSottile's points. Concretely, I would propose this API:

type RenewalInformation struct {
        WindowStart    time.Time
        WindowEnd      time.Time
        ExplanationURL string
} 

func (*Client) GetRenewalInfo(ctx context.Context, cert []byte) (*RenewalInformation, error)

I don't have a sense of how long it's going to take to finalize ARI, but I think it's likely that however it ends up, the above API will still work.

[noncombatant]

At Tailscale we have a fork of x/crypto (and a plan to upstream anything that upstream will accept). Into that fork, @awly and I added support for ARI as described in the current Internet-Draft.

Our API shape is close to what you all propose here. I do agree that it would be simpler, code-wise, to flatten the RenewalInformation struct, but that would require the JSON response as specified to be flattened, too. As specified, hoisting SuggestedWindow lets us use json.Unmarshal directly, which is nice.

Ideally, when ARI is finalized, we can all harmonize on an implementation in upstream x/crypto.

[awly]

The draft is now submitted to IESG for publication, so it is unlikely to change at this point (cc @aarongable). We have a working implementation in https://github.com/tailscale/golang-x-crypto/commit/f0b76a10a08e5ae339838273506f59b90e071559 and https://github.com/tailscale/golang-x-crypto/commit/3fde5e568aa421f641805f385e599b6e5a40e837 that has been used in Tailscale for over a year now.

The API is:

type Directory struct {
    ... // existing fields
    RenewalInfoURL string
}

// FetchRenewalInfo retrieves the RenewalInfo from Directory.RenewalInfoURL.
func (c *Client) FetchRenewalInfo(ctx context.Context, leaf []byte) (*RenewalInfo, error)

// RenewalInfoWindow describes the time frame during which the ACME client
// should attempt to renew, using the ACME Renewal Info Extension.
type RenewalInfoWindow struct {
    Start time.Time `json:"start"`
    End   time.Time `json:"end"`
}

// RenewalInfo describes the suggested renewal window for a given certificate,
// returned from an ACME server, using the ACME Renewal Info Extension.
type RenewalInfo struct {
    SuggestedWindow RenewalInfoWindow `json:"suggestedWindow"`
    ExplanationURL  string            `json:"explanationURL"`
}

If everyone is happy with the API above and draft stability, I'm happy to send a CL with the combined changes.