Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Auth header is sent to HTTP dependencies #17580

Closed
CherryDT opened this issue Jul 3, 2017 · 10 comments
Closed

Auth header is sent to HTTP dependencies #17580

CherryDT opened this issue Jul 3, 2017 · 10 comments

Comments

@CherryDT
Copy link

CherryDT commented Jul 3, 2017

I'm opening this issue because:

npm is doing something I don't understand and think is wrong.

What's going wrong?

It appears that the user credentials configured in .npmrc (with always-auth) are also sent to HTTP dependencies. I don't think it makes sense, I expect those credentials to be sent to my default registry but definitely not to arbitrary HTTP URLs in dependencies. I think this can even be used to sniff credentials.

This behavior causes another issue, which is the reason I found it in the first place: I was unable to install a package from an Amazon S3 URL. The error was:

npm ERR! fetch failed https://s3.amazonaws.com/dotlabs-test/test/npmhttp/npmhttptest-1.0.0.tgz
npm WARN retry will retry, error on last attempt: Error: fetch failed with status code 400

After debugging a bit more, I saw that npm is sending an Authorization header with my user credentials, and this caused Amazon S3 to respond with a bad request error "Unsupported Authorization Type".

Note that it works fine when I create a .npmrc in the current directory (must be a package root) with the contents //myregistry.com/:always-auth=false but then I can't install any other packages anymore because the registry does need auth.

How can the CLI team reproduce the problem?

  • Set some user credentials using npm login
  • Set always-auth true
  • Try npm install https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz

Output with loglevel silly

npm info it worked if it ends with ok
npm verb cli [ '/usr/local/bin/node',
npm verb cli   '/usr/local/bin/npm',
npm verb cli   'i',
npm verb cli   '-s',
npm verb cli   'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm verb cli   '--loglevel=silly' ]
npm info using npm@4.1.2
npm info using node@v7.6.0
npm sill loadCurrentTree Starting
npm sill install loadCurrentTree
npm sill install readLocalPackageData
npm sill fetchPackageMetaData https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm sill fetchOtherPackageData https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm sill cache add args [ 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill cache add   null ]
npm verb cache add spec https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm sill cache add parsed spec Result {
npm sill cache add   raw: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill cache add   scope: null,
npm sill cache add   escapedName: null,
npm sill cache add   name: null,
npm sill cache add   rawSpec: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill cache add   spec: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill cache add   type: 'remote' }
npm sill mapToRegistry name https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm sill mapToRegistry using default registry
npm sill mapToRegistry registry http://[REDACTED]/
npm sill mapToRegistry data Result {
npm sill mapToRegistry   raw: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill mapToRegistry   scope: null,
npm sill mapToRegistry   escapedName: null,
npm sill mapToRegistry   name: null,
npm sill mapToRegistry   rawSpec: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill mapToRegistry   spec: 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm sill mapToRegistry   type: 'remote' }
npm sill mapToRegistry uri https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm verb scopeAuth alwaysAuth set for http://[REDACTED]/
npm verb addRemoteTarball https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz not in flight; adding
npm verb addRemoteTarball [ 'https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz',
npm verb addRemoteTarball   null ]
npm info retry fetch attempt 1 at 8:12:16 AM
npm info attempt registry request try #1 at 8:12:17 AM
npm http fetch GET https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm verb request id b36d1d3997acfd6a
npm http fetch 400 https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm ERR! fetch failed https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm WARN retry will retry, error on last attempt: Error: fetch failed with status code 400
npm info retry fetch attempt 2 at 8:12:28 AM
npm info attempt registry request try #1 at 8:12:28 AM
npm http fetch GET https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm http fetch 400 https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm ERR! fetch failed https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm WARN retry will retry, error on last attempt: Error: fetch failed with status code 400
npm info retry fetch attempt 3 at 8:13:33 AM
npm info attempt registry request try #1 at 8:13:33 AM
npm http fetch GET https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm http fetch 400 https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm ERR! fetch failed https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz
npm sill fetchPackageMetaData Error: fetch failed with status code 400
npm sill fetchPackageMetaData     at Request.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/npm-registry-client/lib/fetch.js:58:14)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill fetchPackageMetaData     at Request.emit (events.js:189:7)
npm sill fetchPackageMetaData     at Request.onRequestResponse (/usr/local/lib/node_modules/npm/node_modules/request/request.js:986:10)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill fetchPackageMetaData     at ClientRequest.emit (events.js:189:7)
npm sill fetchPackageMetaData     at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:522:21)
npm sill fetchPackageMetaData     at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)
npm sill fetchPackageMetaData     at TLSSocket.socketOnData (_http_client.js:411:20)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill fetchPackageMetaData  error for https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz Error: fetch failed with status code 400
npm sill fetchPackageMetaData     at Request.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/npm-registry-client/lib/fetch.js:58:14)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill fetchPackageMetaData     at Request.emit (events.js:189:7)
npm sill fetchPackageMetaData     at Request.onRequestResponse (/usr/local/lib/node_modules/npm/node_modules/request/request.js:986:10)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill fetchPackageMetaData     at ClientRequest.emit (events.js:189:7)
npm sill fetchPackageMetaData     at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:522:21)
npm sill fetchPackageMetaData     at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)
npm sill fetchPackageMetaData     at TLSSocket.socketOnData (_http_client.js:411:20)
npm sill fetchPackageMetaData     at emitOne (events.js:96:13)
npm sill rollbackFailedOptional Starting
npm sill rollbackFailedOptional Finishing
npm sill runTopLevelLifecycles Finishing
npm sill install printInstalled
npm verb stack Error: fetch failed with status code 400
npm verb stack     at Request.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/npm-registry-client/lib/fetch.js:58:14)
npm verb stack     at emitOne (events.js:96:13)
npm verb stack     at Request.emit (events.js:189:7)
npm verb stack     at Request.onRequestResponse (/usr/local/lib/node_modules/npm/node_modules/request/request.js:986:10)
npm verb stack     at emitOne (events.js:96:13)
npm verb stack     at ClientRequest.emit (events.js:189:7)
npm verb stack     at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:522:21)
npm verb stack     at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)
npm verb stack     at TLSSocket.socketOnData (_http_client.js:411:20)
npm verb stack     at emitOne (events.js:96:13)
npm verb cwd /mnt/c/Users/david/Temp/npmhttptest
npm ERR! Linux 4.4.0-43-Microsoft
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "i" "-s" "https://s3.amazonaws.com/dotlabs-test/test/npmhttp/testpkg-1.0.0.tgz" "--loglevel=silly"
npm ERR! node v7.6.0
npm ERR! npm  v4.1.2

npm ERR! fetch failed with status code 400
npm ERR! 
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>
npm verb exit [ 1, true ]

npm ERR! Please include the following file with any support request:
npm ERR!     /mnt/c/Users/david/Temp/npmhttptest/npm-debug.log

supporting information:

  • npm -v prints: 4.1.2
  • node -v prints: v7.6.0
  • npm config get registry prints: Our company registry
  • Windows, OS X/macOS, or Linux?: WSL
@zkat zkat added the bug label Jul 4, 2017
@zkat
Copy link
Contributor

zkat commented Jul 4, 2017

@CherryDT does this happen with npm@5?

@akdor1154
Copy link

akdor1154 commented Jul 4, 2017

With npm@5 and OP's setup instructions, npm sends
authorization: Bearer <a GUID, I guess an npm auth token>
.

Test server:

import express = require('express');
const app = express();
app.use((req, res) => {
	console.log(`${req.method} ${req.url}`);
	console.dir(req.headers);
	res.sendStatus(404);
})
app.listen(8080);

@stamoern
Copy link

@zkat, I explored this bug and found the commit
Maybe it's better to revert those changes and leave a condition:

// Do not send credentials to hosts different from registry host
const shouldAuth = auth && url.parse(uri).host === url.parse(registry).host

@zkat zkat added support npm5 and removed bug labels Jul 28, 2017
@zkat
Copy link
Contributor

zkat commented Jul 28, 2017

always-auth is an option intended for when you need to make sure to send authorization headers even when your tarballs are hosted on a separate server. You shouldn't need this option at all if you have a registry that you're authorizing with which then points to s3 -- if those dependencies aren't working when you install without always-auth, then something is pretty funky but I'd need a lot more details in that case.

As described, everything I'm seeing is that this is working as designed and intended.

@stamoern
Copy link

I have also doubted, that it is a bug. Because otherwise always-auth becomes a redundant configuration parameter. Apparently there no need in this parameter in this situation

@CherryDT
Copy link
Author

CherryDT commented Jul 30, 2017

@zkat @Withw I'm not sure about that - according to the documentation, always-auth sends the authorization also in GET requests when accessing the registry.

Now, for me that's needed because my registry is a Sinopia server for internal use which of course requires credentials to access packages, because those packages should be accessible only to us. So far so good.

However, it appears that the credentials are also sent when accessing raw package URLs in the dependencies list (not the registry), which is what this issue is about, and which I still believe is wrong and dangerous (because third party packages may sneak in rogue package URLs in their dependencies which sniff your credentials, and this can even work when --ignore-scripts is passed which would prevent other forms of attacks during npm install) and does break S3 URLs because no authorization header is allowed to be sent in that case.

always-auth is an option intended for when you need to make sure to send authorization headers even when your tarballs are hosted on a separate server.

I'm not sure what you mean by that - the documentation for always-auth says "Force npm to always require authentication when accessing the registry, even for GET requests." which is exactly what I want it to do.

You shouldn't need this option at all if you have a registry that you're authorizing with which then points to s3

Yes, but I'm not using a registry which points to S3. I have a dependency like this: {"dependencies": {"my-cool-package": "https://s3.amazonaws.com/some-bucket/my-cool-package.tgz"}}. No registry involved, so I believe that no credentials should be sent.

Because otherwise always-auth becomes a redundant configuration parameter. Apparently there no need in this parameter in this situation

I'm not sure I can follow. After applying the fix for this issue, this configuration would still control whether credentials are sent for GET requests to the registry, right?

@stamoern
Copy link

stamoern commented Jul 31, 2017

@CherryDT If you remove always-auth (set it to false) the condition in this line will prevent sending auth header to host different from your registry (Amazon S3 host in your case) But header still will be sent to your registry (if you are logged in).
Perhaps it will be better to fix docs.

@zkat Correct me if I'm wrong

@CherryDT
Copy link
Author

@Withw Strange. It appears you are right, I tested it again with npm@5 and it appears that indeed without always-auth the registry still works. However I'm sure it wasn't always like that - we used always-auth in the first place because without it, our registry would deny the access...

Does this mean that the flag changed its meaning over time and no longer controls whether the username and password are sent to the registry on GET requests, but instead control whether they are sent elsewhere (contrary to what's written in the docs)? I want to make sure I understand correctly what's going on.

@stamoern
Copy link

@CherryDT Take a look at this commit
Before the commit always-auth should have been set to send header with username and password, regardless from url and registry hosts (see line)
I guess it was a fix that didn't get into the docs

@npm-robot
Copy link

We're closing this support issue as it has gone three days without activity. The npm CLI team itself does not provide support via this issue tracker, but we are happy when users help each other here. In our experience once a support issue goes dormant it's unlikely to get further activity. If you're still having problems, you may be better served by joining WeAllJS and asking your question in its #npm channel. The npm CLI team is all present there, but there are also lots of other folks who can help you too.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants