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.

Sign up for GitHub

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: support subproblems #38978

Closed
mbaynton opened this issue May 10, 2020 · 5 comments · May be fixed by golang/crypto#138
Closed

x/crypto/acme: support subproblems #38978

mbaynton opened this issue May 10, 2020 · 5 comments · May be fixed by golang/crypto#138
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Milestone
Unreleased

Comments

@mbaynton
Copy link

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

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mpantheon/.cache/go-build"
GOENV="/home/mpantheon/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/mpantheon/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build487582068=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Called AuthorizeOrder to request a new ACMEv2 Order with identifiers rejected by the CA

What did you expect to see?

An acme.Error populated with RFC 8555 subproblems

What did you see instead?

An acme.Error with no subproblems available.

This limits the information clients can provide to users and prevents automated corrections to the invalid Order, such as by removing the the rejected identifiers and requesting a new Order.
@gopherbot gopherbot added this to the Unreleased milestone May 10, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/233165 mentions this issue: x/crypto/acme: add support for subproblems

@mbaynton mbaynton mentioned this issue May 10, 2020
Open
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 11, 2020
@toothrot
Copy link
Contributor

/cc @FiloSottile @x1ddos

@rolandshoemaker rolandshoemaker changed the title x/crypto/acme: Unmarshal RFC 8555 subproblems proposal: x/crypto/acme: support subproblems Mar 30, 2021
@rolandshoemaker
Copy link
Member

rolandshoemaker commented Mar 30, 2021 •
edited
Loading

Since this changes an API, marking it as a proposal.

CL 233165 proposes the following API addition/change:

type Subproblem struct {
	Type        string
	Detail      string
	Instance    string
	Identifier  *AuthzID
}

func (sp Subproblem) String() string

type Error struct {
	...
	Subproblems []Subproblem
}

@rsc rsc mentioned this issue Apr 1, 2021
Open
@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

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

@rsc
Copy link
Contributor

rsc commented Apr 14, 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: support subproblems x/crypto/acme: support subproblems Apr 14, 2021
@golang golang locked and limited conversation to collaborators Apr 21, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@rolandshoemaker
acme: add support for subproblems
1a5fe5c 
Add support for RFC 8555 subproblems. The type naming is real bike-shed
territory, but I think I've mostly matched the existing style of the
package. In a similar vein the format of how to print subproblems
when stringing an acme.Error is up for debate (it could just be
completely ignored, and require clients to inspect Error.Subproblems
themselves).

Fixes golang/go#38978

Change-Id: Ice803079bab621ae9410de79e7e75e11c1af21b6
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/233165
Trust: Roland Shoemaker <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@rolandshoemaker
acme: add support for subproblems
c0b353a 
Add support for RFC 8555 subproblems. The type naming is real bike-shed
territory, but I think I've mostly matched the existing style of the
package. In a similar vein the format of how to print subproblems
when stringing an acme.Error is up for debate (it could just be
completely ignored, and require clients to inspect Error.Subproblems
themselves).

Fixes golang/go#38978

Change-Id: Ice803079bab621ae9410de79e7e75e11c1af21b6
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/233165
Trust: Roland Shoemaker <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal Proposal-Accepted Proposal-FinalCommentPeriod
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

Unmarshal subproblems from problem details mbaynton/crypto
5 participants
@toothrot @rsc @mbaynton @rolandshoemaker @gopherbot