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

cmd/go: .netrc credentials are not forward to GOPROXY if they do not contain password #40215

Open
marwan-at-work opened this issue Jul 14, 2020 · 7 comments

Comments

@marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Jul 14, 2020

Go currently allows a user to forward credentials to a GOPROXY using a .netrc file. However, if I specify my .netrc file as follows:

machine myproxy.com login mytoken

Go will silently fail and will not send the credentials to myproxy.com unless I explicitly put machine myproxy.com login myuser password mytoken.

In other words, I must have both "login" and "password" values.

On the other hand, machine github.com login mytoken works just fine for VCS authentication (since this is handled by git and libcurl directly and not by Go which they do allow you to specify only the "login" without a "password")

To reproduce:

  1. Create a .netrc file in your home directory with the following line machine localhost:3000 login myToken
  2. Create a TLS server with trusted certs that run on localhost:3000 and would just log out the basic auth. Example here
  3. Create a temporary module in an empty dir: go mod init tmp
  4. From that dir, run GOPATH=/tmp/empty/dir GOPROXY=https://localhost:3000,proxy.golang.org go get github.com/pkg/errors
  5. Note your server logs, the basic auth is missing.
  6. Repeat the same steps as above, but add a password to your netrc config: machine localhost:3000 login myToken password myPassword`
  7. Note how the auth headers are now being passed.

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

$ go version
go version go1.14.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

Extracted from #40189 (see #40189 (comment))

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Jul 14, 2020

cc @bcmills @matloob

Thanks for opening this.

What's the correct behavior here? Should we report an error for a netrc file like this (probably only when we would contact the machine in question)? Or should we send a request with a username and no password? The former seems more correct to me, but we should be consistent with other systems.

@marwan-at-work
Copy link
Contributor Author

@marwan-at-work marwan-at-work commented Jul 14, 2020

An addition I forgot to include: this is the line where it appends to the netrc lines that will be forwarded to the GOPROXY, note that it checks that both a login/password must exist:

if l.machine != "" && l.login != "" && l.password != "" {

@bcmills
Copy link
Member

@bcmills bcmills commented Jul 20, 2020

@marwan-at-work, how would you expect the request to be encoded? (As an HTTPS Basic Auth request with the empty string as the password, or something else?) RFC 7617 isn't exactly clear on the matter...

@marwan-at-work
Copy link
Contributor Author

@marwan-at-work marwan-at-work commented Jul 20, 2020

As an HTTPS Basic Auth request with the empty string as the password

@bcmills at least for GitHub, that is I believe what happens. To test this out, you can use a GitHub token to get your profile information like so:

curl -v https://<GitHub Token>@api.github.com/user

From the verbose output, you can see the Authorization header which then you can decode its base64 value and see that it looks like this:

"<token>:"

So the token is basically the basic auth user, and there is no password after the :.

Finally, you can confirm that the GitHub API was happy with this and it successfully returns your profile info based on that token.

To be honest, I don't know if this behavior is intentional, accidental, or just unique to GitHub. I just noticed that the .netrc file has always worked against GitHub like this and was surprised to see that Go didn't follow the same rules when forwarding credentials to a GOPROXY and had to dig into the code to find out why.

Thanks!

@bcmills
Copy link
Member

@bcmills bcmills commented Jul 21, 2020

I'd like more authoritative confirmation than “GitHub does it”, but if someone can find such a confirmation that seems like a reasonable approach to me.

@bcmills bcmills added the help wanted label Jul 21, 2020
@salewski
Copy link

@salewski salewski commented Aug 2, 2020

I'd be interested to hear something more authoritative, as well, but after surveying the behaviors of other tools these are the main options I see (not all mutually exclusive):

  • Ignore .netrc records that do not provide both login and password fields (current go get behavior; is like wget(1) and (effectively) httpie(1))

  • Fallback to some other configuration to obtain the password (like s-nail(1) and fetchmail(1))

  • Prompt the user for the missing password (like ftp(1), s-nail(1), and fetchmail(1))

  • Treat the missing password field in .netrc as meaning "empty password" for HTTP (like curl(1))

  • Error out (like python3 -m netrc)

There are several different things to consider:

  1. Whether or not it is even legit for a machine record in a .netrc file to be missing a password field (it is legit -- despite what python3 -m netrc says :-)

  2. How a given .netrc file parser recognizes an empty/missing password field (unfortunately, this varies; notes below).

  3. How the application using the .netrc entry behaves in the presence of a missing password field (our main topic here).

    • ftp(1) and fetchmail(1) prompt the user for a password.

    • For HTTP, curl(1) and wget(1) do not prompt the user.

    • The httpie(1) program is using the built-in Python 3 netrc module under the hood, so it (internally) chokes on a .netrc file with a missing password field; it effectively disregards the .netrc file altogether.

  4. How an empty password is encoded in an HTTP request. Here, curl(1), wget(1), and httpie(1) all do the same thing: HTTP Basic Auth, with the value <user-id>: base64-encoded. (There are knobs to obtain different behaviors, too.)

  5. How GitHub handles "personal access tokens" when presented via HTTP Basic Auth. It turns out that GitHub will accept the credentials in all of the following forms (I just tested):

        <legit-user-id>:<token>
        <bogus-user-id>:<token>
        <token>:<token>
        <token>:
    

    Wild stuff. Such behavior probably is not representative. I would have expected only the first to have been accepted.

  6. Whether the proxy server authentication would accept the token in the <user-id> portion of the credentials while doing Basic Auth, even if go get were to have sent it. I doubt most systems would be as flexible as (5), and if dealing with an access token they might require Bearer Auth (or something else), anyway.

When I started writing this, my preference was leaning in the direction of the curl(1) behavior, in part because its parsing of the .netrc file seems more sane than that of the other HTTP tools I looked at. But taking a step back, it now seems to me that the HTTP tools collectively are the oddballs in the world of .netrc parsing and semantics.

The .netrc file format does not provide an unambiguous way to differentiate between a password value that has not been specified and one that should have the empty string as its value. But from what I can tell, the non-HTTP tools all treat an omitted password keyword and value pair as meaning "missing" or "not specified". They then fall back to some other means of acquiring a password value, such as consulting other (non-.netrc) types of configuration, or by prompting the user.

The HTTP tools, OTOH, seem to provide competing interpretations of what may or may not represent an empty password in the .netrc file:

  • curl(1) treats an omitted password field/value pair as "empty string";

  • wget(1) ignores a .netrc record unless the password keyword is present, and then treats the literal "" as an empty password (eww);

  • httpie(1) (like wget) doesn't recognize a .netrc record that does not have a password keyword, but if one is present it interprets the value (like curl) literally ("" means two-double-quote chars).

Even though they agree about how to represent an empty password via HTTP Basic Auth, they do not agree amongst themselves what constitutes an empty or missing password in the .netrc file. It is only when we get to the point at which an empty password has been decided upon that they agree on how to encode that for HTTP Basic Auth.

Others might be able to offer counter examples, but considering what appears to me to be the historical (relative) consistency in the interpretation of the absence of the password and value field as meaning "not specified", I think that interpretation should hold for go get, as well.

However, if an empty string password is then obtained by some other means, I think go get should use it and send the authorization header (rather than requiring a non-empty string to do so, which is the current behavior).

Notes on the above:

  • Regarding (1), there is not only the behavior of existing tools (some of them long-standing) as noted above, but also widespread agreement in the documentation of the tools that an omitted password field means "not present" (including curl). I wasn't able to find any documentation that suggested that an omitted password field might mean "empty password". For example, the GNU Inetutils docs about the .netrc file admit the possibility of the login and password fields being omitted with the language qualifying their effect: "If this token is present, ...".

  • Regarding (2), I've seen four different behaviors, but the most sane is simply for the password keyword and value to both be omitted. Wget "needs to be tricked" into using an empty password from a .netrc file:

        machine fake4.example.com login whatev4 password ""
    

    All the other parsers I've looked at (correctly, IMHO) treat that value as two sequential double-quote chars rather than the empty string.

    If it appears as the last line in the .netrc file, curl will accept the following and treat the password as empty:

        machine fake4.example.com login whatev4 password
    
  • Regarding (5), I was intrigued by @marwan-at-work's description above about

        machine github.com login mytoken
    

    working at all because it would mean GitHub is accepting the personal access token in the <user-id> field of the Basic Auth credentials. I had to test it out to see for myself, and was surprised that so many variations were accepted, as long as the <token> appeared in one or both fields. It looks to me like a weird hybrid between Basic and Bearer token auth, but hey, it works.

    A somewhat more tame Bearer-ish Basic Auth scheme is documented to work with Databricks. The main difference being that the <user-id> is always the string constant token. Just mentioning it as another data point that is not GitHub.

  • Regarding (6), I am just speculating, but I would expect that the .netrc entry would need to be something like the following even if go get was willing to pass the empty password:

        machine myproxy.com login token password mytoken
    

    I'd be curious to hear otherwise, but I would expect most services accepting a token while using Basic Auth would still require that the token be passed as the <password> field, and would not accept it if passed as the <user-id>.

One last note: I should also clarify my apparently contradictory comments above about about how httpid(1) effectively ignores the .netrc file, but then proceed to describe how it behaves once it has an empty password. The two behaviors were tested independently; to get the tool to provide Basic Auth creds with an empty password on the wire I needed to embed the <user-id> portion in the URL provided on the command line:

    $ http --verbose --ignore-netrc 'http://someone@fake1.example.com:7777/'
@bcmills
Copy link
Member

@bcmills bcmills commented Aug 3, 2020

@salewski, note that cmd/go intentionally does not prompt for a password interactively at the moment (see also #26232), so treating passwordless entries as “password not specified” today would be equivalent to ignoring those entries entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.