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

CoreCLR restore needs to ignore feeds with encryption #2942

Closed
billwert opened this Issue Jun 9, 2016 · 7 comments

Comments

Projects
None yet
6 participants
@billwert

billwert commented Jun 9, 2016

When a user adds a Nuget feed so they can push to it, VSTS will instruct them to run a command like this:
nuget sources add -name "TestFeedForBug" -source https://thiswouldbemyfeed -username "username" -password "anencryptedpassword"

This adds the feed to the global nuget.config, along with key information in the tag.

When that is present, restoring running on CoreCLR fails. This is exposed through the CLI, which means VS with the .NET Core SDK installed will fail to restore .NET Core projects.

Instead of just failing, on CoreCLR Nuget should ignore feeds which have credentials like this.

Repro:

  1. Get the .NET CLI SDK (RC2 reproduces it, as does latest.)
  2. mkdir res
  3. Copy the config at the end of the issue into nuget.config in this folder
  4. dotnet new
  5. dotnet restore --packages here

--packages is important for the repro as if everything can be satisfied by your cache you won't hit the correct codepath.

The config XML below has fake values, but still hits the correct codepath. The top of the stack you should see is also below (you'll first hit a failure because the URL is fake, which can be ignored.)

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <packageSources>
    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" />
    <add key="TestFeedForBug" value="https://thiswouldbemyfeed" />
  </packageSources>
  <packageSourceCredentials>
    <TestFeedForBug>
      <add key="Username" value="username" />
      <add key="Password" value="averylongstringofencryptedchars" />
    </TestFeedForBug>
  </packageSourceCredentials>
</configuration>
0:010> !clrstack
OS Thread Id: 0xcac (10)
        Child SP               IP Call Site
000000c3005fcd28 00007ffc2d071f28 [HelperMethodFrame: 000000c3005fcd28] 
000000c3005fce20 00007ffc144dc38f NuGet.Configuration.EncryptionUtility.DecryptString(System.String)
000000c3005fce50 00007ffc144d4c69 NuGet.Configuration.PackageSourceCredential.get_Password()
000000c3005fceb0 00007ffc13dd0787 NuGet.Protocol.HttpSource+d__33.MoveNext()
000000c3005fcf00 00007ffbb4f6bc7b System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[NuGet.Protocol.HttpSource+d__33, NuGet.Protocol.Core.v3]](d__33 ByRef) [f:\dd\ndp\clr\src\mscorlib\src\System\Runtime\CompilerServices\AsyncMethodBuilder.cs @ 323]
000000c3005fcf60 00007ffc13dbc9a8 NuGet.Protocol.HttpSource.UpdateHttpClientAsync()
0:010> !pe
Exception object: 000002804ec2d5c8
Exception type:   System.NotSupportedException
Message:          Specified method is not supported.
InnerException:   <none>
StackTrace (generated):
<none>
StackTraceString: <none>
HResult: 80131515
@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Jun 9, 2016

What we are going to do short term-

Change the error message to say:

Password decryption is not supported on this platform (windows/dotnetcore). The following feed uses an encrypted password: 'https://msasg.pkgs.visualstudio.com/DefaultCollection/_packaging/PrototypeDotNetCoreSupport/nuget/v3/index.json'. You can use a clear text password as a workaround, or pass it on the commandline

yishaigalatzer commented Jun 9, 2016

What we are going to do short term-

Change the error message to say:

Password decryption is not supported on this platform (windows/dotnetcore). The following feed uses an encrypted password: 'https://msasg.pkgs.visualstudio.com/DefaultCollection/_packaging/PrototypeDotNetCoreSupport/nuget/v3/index.json'. You can use a clear text password as a workaround, or pass it on the commandline

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Jun 9, 2016

This issue will only cover the story for the error message. #1851 tracks the full support

yishaigalatzer commented Jun 9, 2016

This issue will only cover the story for the error message. #1851 tracks the full support

@richlander

This comment has been minimized.

Show comment
Hide comment
@richlander

richlander Jun 9, 2016

Please use proper brand strings:

  • Windows
  • .NET Core

richlander commented Jun 9, 2016

Please use proper brand strings:

  • Windows
  • .NET Core
@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Jun 10, 2016

Contributor

please keep up until you port this change into b2.

Contributor

rrelyea commented Jun 10, 2016

please keep up until you port this change into b2.

@rrelyea rrelyea reopened this Jun 10, 2016

@alpaix

This comment has been minimized.

Show comment
Hide comment
@alpaix

alpaix Jun 10, 2016

Please correct the message before merging. Interactive password is not supported on .NET Core. Encrypted passwords are not supported on all platforms not just Windows.

alpaix commented Jun 10, 2016

Please correct the message before merging. Interactive password is not supported on .NET Core. Encrypted passwords are not supported on all platforms not just Windows.

@alpaix

This comment has been minimized.

Show comment
Hide comment
@alpaix

alpaix Jun 10, 2016

I'd suggest this message format instead

Password decryption is not supported on .NET Core for this platform. The following feed uses an encrypted password: '{0}'. You can use a clear text password as a workaround.

alpaix commented Jun 10, 2016

I'd suggest this message format instead

Password decryption is not supported on .NET Core for this platform. The following feed uses an encrypted password: '{0}'. You can use a clear text password as a workaround.

@zhili1208

This comment has been minimized.

Show comment
Hide comment
@zhili1208

zhili1208 Jun 10, 2016

Contributor

Correct the message and checked into dev and 3.5.0-beta2

Contributor

zhili1208 commented Jun 10, 2016

Correct the message and checked into dev and 3.5.0-beta2

@zhili1208 zhili1208 closed this Jun 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment