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

WindowsCryptographicException when using symlink as X509Certificate source #33273

Open
NatMarchand opened this Issue Nov 6, 2018 · 8 comments

Comments

Projects
None yet
6 participants
@NatMarchand

NatMarchand commented Nov 6, 2018

Hi there!
I'd like to use a symlink for constructing my X509Certificate. Currently I'm encoutering a Internal.Cryptography.CryptoThrowHelper+WindowsCryptographicException: Unspecified error
with the following code :

var file = "C:\\ProgramData\\Docker\\secrets\\link.pfx"; // symlink to a pfx file

var cert = new X509Certificate2(File.ReadAllBytes(file)); // <-- works well
Console.WriteLine(cert.ToString());

var cert2 = new X509Certificate2(file); // <-- exception thrown here
Console.WriteLine(cert2.ToString()); 

I know that this issue is already known (although I can't find the issue in this repo) as @bartonjs which seems to be responsible of the System.Security area already replied about it on stackoverflow (see here)

The work around suggested work BUT let me show you a use case that become much more complicated :

When working with AspNet Core using Kestrel in a Windows container orchestrated by Docker Swarm, I would love to use secrets for my SSL certificates (SSL Offloading on a proxy is not an option in my case). Unfortunately, Docker secrets are symbolic links. Therefore, to make it work I have to go from just declaring the secret and an environment variable (Kestrel__Certificates__Default__Path) to tuning Kestrel and load manually the certificate.

Do you plan to fix this ? My guess is, that's coming from CryptQueryObject in crypt32.

Just in case it's needed :

C:\ProgramData\Docker\secrets>dotnet --info
SDK .NET Core (reflétant tous les global.json) :
 Version:   2.1.500-preview-009335
 Commit:    f73e21a7d8

Environnement d'exécution :
 OS Name:     Windows
 OS Version:  10.0.17134
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.1.500-preview-009335\

Host (useful for support):
  Version: 2.1.5
  Commit:  290303f510

and

Internal.Cryptography.CryptoThrowHelper+WindowsCryptographicException: Unspecified error
   at Internal.Cryptography.Pal.CertificatePal.FromBlobOrFile(Byte[] rawData, String fileName, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
   at System.Security.Cryptography.X509Certificates.X509Certificate..ctor(String fileName, String password, X509KeyStorageFlags keyStorageFlags)
   at System.Security.Cryptography.X509Certificates.X509Certificate2..ctor(String fileName, String password)
@bartonjs

This comment has been minimized.

Member

bartonjs commented Nov 6, 2018

I know that this issue is already known (although I can't find the issue in this repo)

It's not a .NET (Framework or Core) issue, it's a Windows issue (as you surmised, we get an error out of CryptQueryObject).

Do you plan to fix this ?

There are currently no plans to address it, no. There's an amount of guesswork in recovery, which isn't clear that we would want in all cases.

@natemcmaster Would it make sense for Kestrel's "cert from file" path to do something like

try
{
    return new X509Certificate2(path);
}
catch (CryptographicException)
{
    if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
    {
        try
        {
            if ((File.GetAttributes(path) & FileSystemAttributes.ReparsePoint) != 0)
            {
                byte[] data = File.ReadAllBytes(path);
                return new X509Certificate2(data);
            }
         }
         catch
         {
         }
    }

    throw;
}

The difference in Kestrel's code from the cert PAL is you know that someone's doing it as part of process startup, and no "wait, are you trying to trick me?" questions really arise.

@natemcmaster

This comment has been minimized.

Member

natemcmaster commented Nov 6, 2018

@Tratcher @blowdart thoughts on adding this to Kestrel options binding? ☝️☝️

@blowdart

This comment has been minimized.

Contributor

blowdart commented Nov 6, 2018

Oh bad docker, naughty docker. Yea, seems ok though.

@Tratcher

This comment has been minimized.

Tratcher commented Nov 6, 2018

It's plausible, though it seems like it belongs in a lower level than Kestrel.

@bartonjs

This comment has been minimized.

Member

bartonjs commented Nov 6, 2018

@Tratcher My thinking for putting it in Kestrel is that you have the context to know that this is app startup; so there's probably no trickery around "ooh, I can get you to File.ReadAllBytes a 1.999GB file". I can't come up with a sane restriction (off the top of my head) for making that as a "generically, at any point in a runtime operation" change in our layer.

At the Windows layer they wouldn't need the intermediate copy, since they can still just pull from the file descriptor.

So both above us (with context) and below us (due to a different access model) can do this sensibly, but it's hard for my layer.

@Tratcher

This comment has been minimized.

Tratcher commented Nov 6, 2018

I can understand you don't want to change the default behavior but there's room for an intermediate API like X509Certificate2.LoadCertWithSimlinks(string path) that kestrel could call. There's no reason for that logic to be implemented directly in Kestrel.

@blowdart

This comment has been minimized.

Contributor

blowdart commented Nov 6, 2018

Or a flag on load from file, "FollowSymLinks"?

@NatMarchand

This comment has been minimized.

NatMarchand commented Nov 13, 2018

For people stuck on this issue (maybe I'm not alone trying to do this?), here is a workaround :
Instead of using the path of the secret such as "C:\\ProgramData\\Docker\\secrets\\<mySecretName>", it is possible to use the "internal" path "C:\\ProgramData\\Docker\\internal\\secrets\\<mySecretId>"
However, as explained in the doc here, it shouldn't be relied upon. Use with care.

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