Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/crypto/acme: allow setting client KID directly #46303

Closed
Stanwise opened this issue May 21, 2021 · 5 comments
Closed

x/crypto/acme: allow setting client KID directly #46303

Stanwise opened this issue May 21, 2021 · 5 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
Proposals (old)
Milestone
Backlog

Comments

@Stanwise
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

N/A

What did you do?

I have a service that uses acme.Client for communication with an RFC8555 CA. For technical reasons I need to reconstruct the acme.Client multiple times during the full flow.

Current implementation works overall, but I'd like to be able to specify account KID (equal to Account.URI) explicitly, instead of having the library always call the CA's account registration endpoint to obtain kid to cache: https://github.com/golang/crypto/blob/c07d793c2f9aacf728fe68cbd7acd73adbd04159/acme/acme.go#L152

What did you expect to see?

I'd like to be able to create the client like this:

&acme.Client{
    Key:          signer,
    KID:          accountURI,
    DirectoryURL: directoryUrl,
    HTTPClient:   &http.Client{...}
},

And have the kid field populated from it , e.g.:

  • KID field could be a string, only used in the first request to pre-populate the private mutex-protected kid value.
  • The following snippet could be added to accountKID function between lines 151 and 152
if c.KID != "" {	
  c.kid = c.KID
  return c.kid
}

What did you see instead?

I have to specify the client like this:

&acme.Client{
    Key:          signer,
    DirectoryURL: directoryUrl,
    HTTPClient: &http.Client{...}
},

And then the library calls the CA's account registration endpoint to obtain kid to cache. This results in a lot of unnecessary calls to the CA.
@gopherbot gopherbot added this to the Proposal milestone May 21, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) May 26, 2021
@gopherbot
Copy link

Change https://golang.org/cl/354697 mentions this issue: acme: expose Client KID field

@rolandshoemaker
Copy link
Member

(sent a CL for this, but just to articulate my thoughts here) This seems reasonable to me. I don't think adding a duplicate public field is worth the extra complexity, seems fine to just expose the existing kid field.

@rsc
Copy link
Contributor

rsc commented Oct 13, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 13, 2021
@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Oct 20, 2021
@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Oct 27, 2021
@rsc
Copy link
Contributor

rsc commented Oct 27, 2021

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

@rsc rsc changed the title proposal: x/crypto/acme: allow setting client KID directly x/crypto/acme: allow setting client KID directly Oct 27, 2021
@rsc rsc modified the milestones: Proposal, Backlog Oct 27, 2021
owenthereal pushed a commit to owenthereal/upterm.crypto that referenced this issue Mar 5, 2022
@rolandshoemaker @owenthereal
acme: expose Client KID field
034a349 
Expose the previously private KID field of the Client type. This allows
callers which have locally cached their key identity to avoid needing
to make a call to the ACME service every time they construct a new
client.

Fixes golang/go#46303

Change-Id: I219167c5b941f56a2028c4bc253ff56386845549
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/354697
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
iamacarpet pushed a commit to affordablemobiles/xcrypto that referenced this issue Aug 2, 2022
@rolandshoemaker @iamacarpet
acme: expose Client KID field
5f7b488 
Expose the previously private KID field of the Client type. This allows
callers which have locally cached their key identity to avoid needing
to make a call to the ACME service every time they construct a new
client.

Fixes golang/go#46303

Change-Id: I219167c5b941f56a2028c4bc253ff56386845549
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/354697
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jan 31, 2023
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@rolandshoemaker
acme: expose Client KID field
3f66a77 
Expose the previously private KID field of the Client type. This allows
callers which have locally cached their key identity to avoid needing
to make a call to the ACME service every time they construct a new
client.

Fixes golang/go#46303

Change-Id: I219167c5b941f56a2028c4bc253ff56386845549
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/354697
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@rolandshoemaker
acme: expose Client KID field
d877807 
Expose the previously private KID field of the Client type. This allows
callers which have locally cached their key identity to avoid needing
to make a call to the ACME service every time they construct a new
client.

Fixes golang/go#46303

Change-Id: I219167c5b941f56a2028c4bc253ff56386845549
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/354697
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
No open projects
Proposals (old)
Accepted
Milestone
Backlog
Development

No branches or pull requests
4 participants
@rsc @Stanwise @rolandshoemaker @gopherbot