search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.7k stars 17.49k forks source link

x/crypto/acme: add AccountKeyRollover #42516

Closed dandragona-dev closed 2 years ago

dandragona-dev commented 3 years ago

RFC8555's Account Key Rollover is not yet supported in the acme package. This is a desirable RFC8555 feature that is supported by Let's Encrypt, and so CAs depending on this library may wish to also implement this feature.

The public API for this could be something like:

// AccountKeyRollover attempts to transition a client's account key to a new key.
// If the new key already belongs to an account registered with the CA then it will return the existing
// account's account URL (AKA the 'kid').
// Otherwise returns "", nil on success, and "", err for other error types.
// On successful key rollovers the client's Key field is updated with 'newKey'.
// https://tools.ietf.org/html/rfc8555#section-7.3.5
func (c *Client) AccountKeyRollover(ctx context.Context, newKey crypto.Signer) (string, error) {}
rsc commented 3 years ago

/cc @rolandshoemaker

rolandshoemaker commented 3 years ago

Seems reasonable. I'm not entirely sure there is much value in having the KID as a return value. I think we can probably just return an error and include the existing KID as part of the error message.

rsc commented 3 years ago

OK, so I am seeing:

func (c *Client) AccountKeyRollover(ctx context.Context, newKey crypto.Signer) error

where there may be a specific KeyAlreadyRegisteredError returned containing the existing KID. Do I have that right? Thanks.

dandragona-dev commented 3 years ago

That would work for my needs. I just wanted to make sure we have access to the existing KID for that case.

rolandshoemaker commented 3 years ago

That would work for my needs. I just wanted to make sure we have access to the existing KID for that case.

What is your use case for wanting the KID? I can see why you might want it during registration, but I'm not sure I see the value in having it during key rollover as you already have an account. Are you going to switch to the account using the key you wanted to rollover to?

dandragona-dev commented 3 years ago

I wanted to be able to provide some more context for the user about the error that occurred, although I do understand your line of reasoning. Having the KID be returned through an error is perfectly suitable rather than having it in the API though. Since this is a proposal for the API I will leave the final decision to you whether you want to return the KID in the error message or not.

Thanks!

rsc commented 3 years ago

Based on the discussion above, this https://github.com/golang/go/issues/42516#issuecomment-741954048 seems like a likely accept.

rsc commented 3 years ago

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

gopherbot commented 2 years ago

Change https://go.dev/cl/400274 mentions this issue: acme: add AccountKeyRollover