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

Credentials from configured source used for a non-configured source when the URL is similar #3565

Open
6 tasks done
pauby opened this issue Nov 15, 2024 · 0 comments · May be fixed by #3568
Open
6 tasks done

Credentials from configured source used for a non-configured source when the URL is similar #3565

pauby opened this issue Nov 15, 2024 · 0 comments · May be fixed by #3568
Assignees
@vexx32
Labels
3 - Review Bug Customer Security
Milestone
2.4.1

Comments

@pauby
Copy link
Member

pauby commented Nov 15, 2024 •
edited
Loading

Checklist

  • I confirm there are no unresolved issues reported on the Chocolatey Status page.
  • I have verified this is the correct repository for opening this issue.
  • I have verified no other issues exist related to my problem.
  • I have verified this is not an issue for a specific package.
  • I have verified this issue is not security related.
  • I confirm I am using official, and not unofficial, or modified, Chocolatey products.

What You Are Seeing?

When performing package operations against a source, Chocolatey CLI may choose to use credentials stored against a different source.

What is Expected?

If credentials are not provided, Chocolatey CLI should not use credentials stored against a different source.

How Did You Get This To Happen?

  1. Install Chocolatey on a machine
    Set-ExecutionPolicy Unrestricted Process -Force; irm https://ch0.co/go | iex
  2. Install Sonatype Nexus Repository ('Nexus') on a machine (note: also installing NexuShell)
    choco install nexus-repository nexushell --confirm
    a. Log in to Nexus
    Connect-NexusServer -Hostname LocalHost -Credential ([PSCredential]::new('admin', (Get-Content C:\ProgramData\sonatype-work\nexus3\admin.password | ConvertTo-SecureString -Force -AsPlainText)))
    b. Create ChocolateyInternal NuGet repository
    New-NexusRepository -Name ChocolateyInternal -Format nuget -Type hosted -DeploymentPolicy Allow
    c. Create AdminOnly NuGet repository
    New-NexusRepository -Name AdminOnly -Format nuget -Type hosted -DeploymentPolicy Allow
    d. Push a package to the AdminOnly repository
    $ApiKey = (Get-NexusNuGetApiKey -Credential ([PSCredential]::new('admin', (Get-Content C:\ProgramData\sonatype-work\nexus3\admin.password | ConvertTo-SecureString -Force -AsPlainText)))).apiKey; choco push $env:ChocolateyInstall\lib\nexus-repository\nexus-repository.nupkg --source http://localhost:8081/repository/adminOnly/index.json --api-key="$ApiKey"
    e. Turn off anonymous access (last, because otherwise you can't push the package)
    Set-NexusAnonymousAuth -Disabled
  3. Add the ChocolateyInternal repository to the Chocolatey install, including the valid username and password you are using for Nexus.
    choco source add -n=ChocoInternal --source=http://localhost:8081/repository/ChocolateyInternal/index.json -U=admin -P="$(Get-Content C:\ProgramData\sonatype-work\nexus3\admin.password)"
  4. choco search --source $AdminOnlyFeedUrl, where the AdminOnlyFeedUrl is the URL of the AdminOnly repository on Nexus, without specifying credentials.
    choco search --source http://localhost:8081/repository/adminOnly/index.json

At the end of the configuration, the server should be configured as follows:

  • Nexus installed
  • Anonymous authentication turned off, an account ready to be used
  • Two sources created
  • A package pushed to AdminOnly
  • The ChocolateyInternal authenticated source added to Chocolatey CLI
  • The AdminOnly source not added to Chocolatey CLI

System Details

  • Operating System: Windows Server 2022
  • Windows PowerShell Version: 5.1.20348.2849
  • Chocolatey CLI Version: 2.4.0
  • Chocolatey Licensed Extension version: 6.2.1 (but seemingly N/A)
  • Chocolatey License type (Professional / Business / ?): Business
  • Terminal/Emulator: bare PowerShell.exe

Installed Packages

N/A

Output Log

See GitLab issue.

Additional Context
@pauby pauby added this to the 2.4.1 milestone Nov 15, 2024
vexx32 added a commit to vexx32/choco that referenced this issue Nov 15, 2024
@vexx32
(chocolatey#3565) Match full source URL for credential lookup
09b3f6d 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri.Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.
@gep13 gep13 changed the title Credentials from configured NuGet v3 source used for a non-configured source when the URL is similar Credentials from configured source used for a non-configured source when the URL is similar Nov 18, 2024
@gep13 gep13 mentioned this issue Nov 18, 2024
Open
6 tasks
@vexx32 vexx32 self-assigned this Nov 18, 2024
vexx32 added a commit to vexx32/choco that referenced this issue Nov 18, 2024
@vexx32
(chocolatey#3565) Match full source URL for credential lookup
8496e81 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri.Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.
vexx32 added a commit to vexx32/choco that referenced this issue Nov 18, 2024
@vexx32
(chocolatey#3565) Rework NugetCredentialProvider
47e12d5 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri. Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.

Additionally, we try to ensure that if a user has named a specific
source, rather than themselves providing a URL at the command line, we
prioritise finding that in the already-configured sources and use that
source if the URL matches the current URL that NuGet requires a
credential for.
vexx32 added a commit to vexx32/choco that referenced this issue Nov 18, 2024
@vexx32
(chocolatey#3565) Rework NugetCredentialProvider
c073d57 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri. Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.

Additionally, we try to ensure that if a user has named a specific
source, rather than themselves providing a URL at the command line, we
prioritise finding that in the already-configured sources and use that
source if the URL matches the current URL that NuGet requires a
credential for.
vexx32 added a commit to vexx32/choco that referenced this issue Nov 18, 2024
@vexx32
(chocolatey#3565) PARTIAL: Add unit tests for ChocolateyNugetCredenti…
35b71e5 
…alProvider
vexx32 added a commit to vexx32/choco that referenced this issue Nov 19, 2024
@vexx32
(chocolatey#3565) Add unit tests for ChocolateyNugetCredentialProvider
5c4a716 
These tests ensure that the use cases we expect to handle in the
credential provider are appropriately handled according to our
expectations, based on the user-provided input and the transformed input
that is left in configuration.Sources once the credential provider
typically gets queried.
@vexx32 vexx32 linked a pull request Nov 19, 2024 that will close this issue
Open
10 tasks
vexx32 added a commit to vexx32/choco that referenced this issue Nov 19, 2024
@vexx32
(chocolatey#3565) Rework NugetCredentialProvider
f098482 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri. Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.

Additionally, we try to ensure that if a user has named a specific
source, rather than themselves providing a URL at the command line, we
prioritise finding that in the already-configured sources and use that
source if the URL matches the current URL that NuGet requires a
credential for.
vexx32 added a commit to vexx32/choco that referenced this issue Nov 19, 2024
@vexx32
(chocolatey#3565) Add unit tests for ChocolateyNugetCredentialProvider
db44247 
These tests ensure that the use cases we expect to handle in the
credential provider are appropriately handled according to our
expectations, based on the user-provided input and the transformed input
that is left in configuration.Sources once the credential provider
typically gets queried.
corbob pushed a commit to vexx32/choco that referenced this issue Nov 20, 2024
@vexx32 @corbob
(chocolatey#3565) Rework NugetCredentialProvider
f307edd 
Previously we looked up any available sources in the config by the
hostname, before falling back to trying an exact match if we had
collisions.

This still allowed credentials to be reused in situations where we don't
actually know if they're applicable; many repository servers will
support different credentials for individual repositories, so we cannot
and should not assume that credentials for one repository will actually
match another repository, nor that users want the credentials to be
shared for both.

It also led to the possibility of users storing one repository first,
and then later specifying a different repository on the same server, and
choco would try to use the stored credentials for the first repository
for the explicitly-entered URL which is nowhere in config.

Instead, we should only match the whole URL (which can be done with
Uri. Equals to ensure that we match hostnames case-insensitively, but
routes case-sensitively), and expect users to provide credentials if
they provide a URL that is not explicitly in the sources.

Additionally, we try to ensure that if a user has named a specific
source, rather than themselves providing a URL at the command line, we
prioritise finding that in the already-configured sources and use that
source if the URL matches the current URL that NuGet requires a
credential for.
corbob pushed a commit to vexx32/choco that referenced this issue Nov 20, 2024
@vexx32 @corbob
(chocolatey#3565) Add unit tests for ChocolateyNugetCredentialProvider
d2ac771 
These tests ensure that the use cases we expect to handle in the
credential provider are appropriately handled according to our
expectations, based on the user-provided input and the transformed input
that is left in configuration.Sources once the credential provider
typically gets queried.
corbob added a commit to vexx32/choco that referenced this issue Nov 21, 2024
@corbob
(chocolatey#3565) Add Pester tests for Credential Provider
090ed11 
Add Pester tests to ensure we don't inadvertently bleed configured
credentials into scenarios where they should not be used.
corbob added a commit to vexx32/choco that referenced this issue Nov 21, 2024
@corbob
(chocolatey#3565) Add Pester tests for Credential Provider
1353106 
Add Pester tests to ensure we don't inadvertently bleed configured
credentials into scenarios where they should not be used.
corbob added a commit to vexx32/choco that referenced this issue Nov 21, 2024
@corbob
(chocolatey#3565) Remove query string from Pester tests
36a736b 
The query string used by some commands seems to be ever so slightly
different depending on where they are run. This removes the query string
from the tests as these tests are not concerned with the entire message,
just the initial part of the message.
corbob added a commit to vexx32/choco that referenced this issue Nov 21, 2024
@corbob
(chocolatey#3565) Address review comments
7a0b627
corbob added a commit to corbob/choco that referenced this issue Nov 22, 2024
@corbob
.x: Auto stash before cherry pick of "(chocolatey#3565) Remove query …
8a64fbc 
…string from Pester tests"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vexx32 vexx32

Labels
3 - Review Bug Customer Security
Projects
None yet
Milestone
2.4.1
Development

Successfully merging a pull request may close this issue.

(#3565 #2421) Avoid credential bleed from saved sources with the same hostname vexx32/choco
2 participants
@pauby @vexx32