Skip to content

x/crypto/acme: Directory.AuthzURL isn't checked for pre-auth support, in client.Authorize() #40839

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

Closed
nathanaelle opened this issue Aug 17, 2020 · 5 comments
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted.
Milestone

Comments

@nathanaelle
Copy link

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

go version go1.14.7 darwin/amd64

Does this issue reproduce with the latest release?

yes, with any release of crypto/acme with ACMEv2 support

What did you do?

_, err := lestencryptStagingV2client.Authorize("example.com")

What did you expect to see?

{directotyURL} doesn't support pre-authorization flow

What did you see instead?

Post "": unsupported protocol scheme ""

this obscure error is returned because AuthzURL is empty for letsencrypt staging v2, and AuthzURL is not tested before calling POST.

@gopherbot gopherbot added this to the Unreleased milestone Aug 17, 2020
@dmitshur dmitshur changed the title x/crypto/acme : Directory.AuthzURL isn't checked for pre-auth support, in client.Authorize() x/crypto/acme: Directory.AuthzURL isn't checked for pre-auth support, in client.Authorize() Aug 18, 2020
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 18, 2020
@dmitshur
Copy link
Contributor

/cc @FiloSottile @x1ddos per owners.

@x1ddos
Copy link

x1ddos commented Aug 18, 2020

// AuthzURL is used to initiate identifier pre-authorization flow.
// Empty string indicates the flow is unsupported by the CA.
AuthzURL string

https://godoc.org/golang.org/x/crypto/acme#Directory

@nathanaelle
Copy link
Author

@x1ddos ,

in this file : https://github.com/golang/crypto/blob/123391ffb6de/acme/acme.go#L466 , you can see :

if _, err := c.Discover(ctx); err != nil {
		return nil, err
	}

there is a discovery made, but … nothing is done with the directory, and the next instruction is to do a POST to AuthzURL.

my point is :
AuthzURL may be checked here and return an error if AuthzURL is empty to avoid this kind or error Post "": unsupported protocol scheme "" .

@nathanaelle
Copy link
Author

This issue is about Dev Experience , not cryptography, not RTFM.

I already read the manual and I read also the RFC, how can @x1ddos may guess that ?
if I ignore that in ACMEv1 Pre-auth was mandatory but not in ACMEv2, how possible did I know that this is an issue since the ACMEv2 ?

the Dev Experience issue is Authorize() can return a meaningfull error if AuthzURL is empty.

the error {directotyURL} doesn't support pre-authorization flow carry more meaning than Post "": unsupported protocol scheme ""

but Post "": unsupported protocol scheme "" is returned by post().
so Authorize() need to check if AuthzURL is empty, to return a meaningfull error.

go is not a one-person side-project.
Replying with only a manual citation, seems at least a bit aggressive.
And, if it is possible to establish that aggressive reply was made without any effort to understand the issue, that may become a conduct issue.

English is not my native language, and I live with a brain injury (traumatic injury) that make those kind of social interaction really difficult in any language.

I don't ask any favor, I just expect a reply compatible with https://golang.org/conduct


 These are the values to which people in the Go community (“Gophers”) should aspire.

    Be friendly and welcoming
    Be patient
        Remember that people have varying communication styles and that not everyone is using their native language. (Meaning and tone can be lost in translation.) 
    Be thoughtful
        Productive communication requires effort. Think about how your words will be interpreted.
        Remember that sometimes it is best to refrain entirely from commenting. 
    Be respectful
        In particular, respect differences of opinion. 
    Be charitable
        Interpret the arguments of others in good faith, do not seek to disagree.
        When we do disagree, try to understand why. 
    Avoid destructive behavior:
        Derailing: stay on topic; if you want to talk about something else, start a new conversation.
        Unconstructive criticism: don't merely decry the current state of affairs; offer—or at least solicit—suggestions as to how things may be improved.
        Snarking (pithy, unproductive, sniping comments)
        Discussing potentially offensive or sensitive issues; this all too often leads to unnecessary conflict.
        Microaggressions: brief and commonplace verbal, behavioral and environmental indignities that communicate hostile, derogatory or negative slights and insults to a person or group. 

People are complicated. You should expect to be misunderstood and to misunderstand others; when this inevitably occurs, resist the urge to be defensive or assign blame. Try not to take offense where no offense was intended. Give people the benefit of the doubt. Even if the intent was to provoke, do not rise to it. It is the responsibility of all parties to de-escalate conflict when it arises. 

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/661675 mentions this issue: acme: return error from pre-authorization when unsupported

@dmitshur dmitshur added FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted.
Projects
None yet
Development

No branches or pull requests

4 participants