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

WindowsCryptographicException when using symlink as X509Certificate source #27826

Open
NatMarchand opened this issue Nov 6, 2018 · 15 comments
Open
Labels
area-System.Security enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@NatMarchand
Copy link

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
Copy link
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
Copy link
Contributor

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

@blowdart
Copy link
Contributor

blowdart commented Nov 6, 2018

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

@Tratcher
Copy link
Member

Tratcher commented Nov 6, 2018

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

@bartonjs
Copy link
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
Copy link
Member

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
Copy link
Contributor

blowdart commented Nov 6, 2018

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

@NatMarchand
Copy link
Author

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.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@erdtsieck
Copy link

Another workaround is doing a copy at startup.

# The copy is done, because wildcard_certificate.pfx is put into the container using docker secrets, which makes it a symlink. 
# Reading a certificate as a symlink is not supported at this moment: https://stackoverflow.com/q/43955181/1608705
# After doing a copy, the copied version is not a symlink anymore.
ENTRYPOINT (IF EXIST "c:\certificates\wildcard_certificate.pfx" (copy c:\certificates\wildcard_certificate.pfx c:\app\wildcard_certificate.pfx)) && dotnet webapplication.dll

My application runs in the "c:\app" folder and I put my "to be copied" certificates in "c:\certificates". At startup the certificate is copied to "c:\app", which my environment variables point to.

version: "3.7"
services:
  webapplication:
    image: ({CONTAINER_REGISTRY})/webapplication:({LABEL})
    environment:
      - ASPNETCORE_URLS=https://+;http://+
      - ASPNETCORE_HTTPS_PORT=443
      - ASPNETCORE_Kestrel__Certificates__Default__Path=wildcard_certificate.pfx
    secrets:
      - source: config_secrets
        target: C:/app/appsettings.json
      - source: wildcard_certificate_pfx
        target: c:\certificates\wildcard_certificate.pfx

@jhudsoncedaron
Copy link

Hit this in prod. Bug in Windows. It seems the only possible way of getting the Windows Crypto teams to address their bugs is to actually be willing to switch to OpenSSL if they don't.

@vcsjones
Copy link
Member

@jhudsoncedaron would it be possible for you to modify the code to read the contents of the file into a byte[] first? Such as in the original issue:

using var cert = new X509Certificate2(File.ReadAllBytes(file));

Or is this happening in a higher level, or otherwise unmodifiable, API for you?

@jhudsoncedaron
Copy link

jhudsoncedaron commented Jan 13, 2021

@vcsjones : Haven't you had the crashes resulting from reading the private key into a byte[] leak disk yet? We have. That writes a temp file in native code and maps it. The temp file survives if the process crashes. Also, %TEMP% may be unwritable on site.

Setting Epheremal key stops it from writing to disk, but that too is busted due to a different bug in the Windows Crypto API.

@jhudsoncedaron
Copy link

Apologies. new X509Certificate(byte[]) isn't the API affected by the leaks. new X509Certificate(byte[], SecureString) is. So some uses can use the workaround without issue and some can't.

@vcsjones
Copy link
Member

@jhudsoncedaron

You should be able to use new X509Certificate2(byte[], password, X509KeyStorageFlags.EphemeralKeySet) to avoid persisting the keys to disk and keeping them entirely in memory.

@jhudsoncedaron
Copy link

@vcsjones : #23749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Security enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

10 participants