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: Don't omit authorization error returned in challenge URL #37340

Closed
andrewlouisx opened this issue Feb 20, 2020 · 4 comments
Closed

x/crypto/acme: Don't omit authorization error returned in challenge URL #37340

andrewlouisx opened this issue Feb 20, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@andrewlouisx
Copy link

andrewlouisx commented Feb 20, 2020 •
edited

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

$ go version
go1.13.1 darwin/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="off"

GOARCH="amd64"

GOBIN=""

GOCACHE="/Users/alouis/Library/Caches/go-build"

GOENV="/Users/alouis/Library/Application Support/go/env"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GOOS="darwin"

GOPROXY="https://proxy.golang.org,direct"

GOROOT="/usr/local/go"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"

GCCGO="gccgo"

AR="ar"

CC="clang"

CXX="clang++"

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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gf/xt8nljp959z9gyd_qdvr68gc0000gp/T/go-build456374909=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  • Call WaitAuthorization on a challenge url.
  • The challenge URL returned by Lets Encrypt showed that the auth fails due to the presence of a CAA record; the response was a 200 that looked like this:
{
  "type": "dns-01",
  "status": "invalid",
  "error": {
    "type": "urn:ietf:params:acme:error:caa",
    "detail": "CAA record for domain.com prevents issuance",
    "status": 403
  },
  "url": "https://acme-v02.api.letsencrypt.org/acme/chall-v3/xxx/xxx",
  "token": "xxx",
  "validationRecord": [
    {
      "hostname": "domain.com"
    }
  ]
}
  • WaitAuthorization unmarshals this response into a wireAuthz type; which expects errors to be nested under the challenges field.
  • However, this outer error in the response is omitted as we do this unmarshal, and we lose reference to it. Eventually, WaitAuthorization returns this odd looking error acme: authorization error for :
  • There also might not be an Identifier field in the response, we should not return an error message assuming its presence.

What did you expect to see?

acme: authorization error: 403 urn:ietf:params:acme:error:caa: CAA record for domain.com prevents issuance

What did you see instead?

acme: authorization error for :

Related PR

Link to a proposed fix over here: golang/crypto#121
@gopherbot gopherbot added this to the Unreleased milestone Feb 20, 2020
@andrewlouisx andrewlouisx mentioned this issue Feb 20, 2020
Closed
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 20, 2020
@toothrot
Copy link
Contributor

/cc @FiloSottile @x1ddos

@andrewlouisx
Copy link
Author

@toothrot I see the NeedsInvestigation label, let me know how I can help there

@andrewlouisx
Copy link
Author

Here's an example of a challenge url formatted this way, for which https://acme-v01.api.letsencrypt.org/acme/chall-v3/3078101936/5w1LuQ we will return this malformed error. You should be able to reproduce by calling WaitAuthorization on this url

This isn't only the case where the domain has a CAA record by the way, I'm just using this one since it's easy to replicate.

There's another challenge link I've seen that returns a 202 and looks like the following. There are likely to be other cases as well.

{
  "type": "dns-01",
  "status": "invalid",
  "error": {
    "type": "urn:acme:error:dns",
    "detail": "DNS problem: NXDOMAIN looking up TXT for _acme-challenge.<domain> - check that a DNS record exists for this domain",
    "status": 400
  },
  "uri": "https://acme-v01.api.letsencrypt.org/acme/chall-v3/xxx",
  "token": "xxx"
}

cc @FiloSottile @x1ddos @toothrot

@gopherbot
Copy link

Change https://golang.org/cl/220343 mentions this issue: acme: Make WaitAuthorization return authorization errors consistently

@golang golang locked and limited conversation to collaborators Mar 2, 2021
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@FiloSottile
acme: make WaitAuthorization return authorization errors consistently
8d75d03 
Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964b4a680b135301695ccd56cff88a8ffb26
GitHub-Pull-Request: golang/crypto#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@c-expert-zigbee
acme: make WaitAuthorization return authorization errors consistently
bd5829d 
Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964b4a680b135301695ccd56cff88a8ffb26
GitHub-Pull-Request: golang/crypto#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@c-expert-zigbee
acme: make WaitAuthorization return authorization errors consistently
d3fc7e3 
Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964b4a680b135301695ccd56cff88a8ffb26
GitHub-Pull-Request: golang/crypto#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@FiloSottile
acme: make WaitAuthorization return authorization errors consistently
a5dd37f 
Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964b4a680b135301695ccd56cff88a8ffb26
GitHub-Pull-Request: golang/crypto#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@FiloSottile
acme: make WaitAuthorization return authorization errors consistently
b2f3e06 
Fixes golang/go#37340

Change-Id: I19c4f150b8607ad4a1613cf97ad3362f4b779d7c
GitHub-Last-Rev: 4215964
GitHub-Pull-Request: golang#121
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220343
Reviewed-by: Filippo Valsorda <filippo@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 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

acme: make WaitAuthorization return authorization errors consistently andrewlouisx/crypto
3 participants
@toothrot @andrewlouisx @gopherbot