[Question] Security of the Encrypted data

[Angelelz]

If I use this library to encrypt text and write the output to a txt file, could another executable be able to decrypt the text using the static method NCryptDecrypt from this same library, without the need to use Windows Hello Authentication?
Maybe for somebody the answer to this question is clear from the code, but I'm just learning, and I do it in my spare time. Thanks.

[SeppPenner]

This is a really good question... I hope that this is impossible because this would break the whole system...

Honestly, I haven't tried that yet... I will check if this is possible.

EDIT: It might take some time, I'm quite busy with some other projects at the moment...

[SeppPenner]

I will try this under https://github.com/SeppPenner/WindowsHelloTest.

[SeppPenner]

For me, it wasn't possible to get this to work without the need to identify again :)

Check the project under https://github.com/SeppPenner/WindowsHelloTest. You can run https://github.com/SeppPenner/WindowsHelloTest/blob/master/WindowsHelloTest.1Only.sln to encrypt the data into a file named test.dat in the solution folder. The project under https://github.com/SeppPenner/WindowsHelloTest/blob/master/WindowsHelloTest.2Only.sln can be run to decrypt the data again.

As I said, I always needed to re-identify myself when running the WindowsHelloTest2 project.

[SeppPenner]

@Angelelz Feel free to contact me again if you are able to somehow undergo this mechanism or if you have further questions.

[Angelelz]

Thank you for taking the time to answer this question! I will fork that repository and work on it myself, and if needed, share my findings with you in this thread. I might even use it in my WinHelloUnlock plugin to provide it in .plgx file.

[SeppPenner]

Thank you for taking the time to answer this question! I will fork that repository and work on it myself, and if needed, share my findings with you in this thread.

Sure, no problem :)

[Angelelz]

It happened exactly as I was afraid. It's easy to break the encryption if you use WindowsHello Library as code files instead of Nuget package, so that you can change some private static methods to public (or internal) like NCryptDecrypt and some others.
I changed some of the logic in the Test2 to prove the point.
I also changed the test1 a little to encrypt text instead.
Please take a look at my fork and test it yourself, maybe we find a solution to this.

[SeppPenner]

Ah okay. If you use this as a class and not from the nuget library, this might be. Well, I don't necessarily think that this is a security issue because it's used in a project you can control?

[Angelelz]

The problem is, if it's used in a project I cannot control, I still can decrypt the data from that project using a simple console app like the one in my fork. Which I think defeats its purpose. With that in mind, any data saved "encrypted" with this library might as well be written in plain text! I think that is a real concern.

But the hope is not lost, please check out the latest KeePassWinHello version, in which they sing a key with Windows Hello Biometry and that is used to encrypt data, I've been trying to understand it and verify it for the last couple of days. LMK what you think

[SeppPenner]

The problem is, if it's used in a project I cannot control, I still can decrypt the data from that project using a simple console app like the one in my fork.

Have you tried to setup two different projects (in different locations) both having the same WinHelloProvider class and the same settings? I'm not sure how the API determines if you need to re-authenticate or not exactly...

But the hope is not lost, please check out the latest KeePassWinHello version, in which they sing a key with Windows Hello Biometry and that is used to encrypt data

I'm not sure if this solves the problem. If there is a key and this key is available somewhere, another programm can easily get access to the key as well. (As far as I understood it now).

[Angelelz]

Yes, I've done a lot of research and now I have a better understanding of all of this. The problem is that this library just uses the default Windows Passport key to encrypt the data, and that key is obviously available for the user. To make the Windows Hello Security prompt appear, they just set a property to that key called NcryptPinCacheIsGestureRequiredProperty to 1. That just makes the Windows Security Prompt appear but it does nothing to the key, it is still available to the user to use for decryption. The real decryption is made by NCryptDecrypt method.

In the latest version of KeePassWinHello they now create a custom persistent key that is signed with Windows Hello. To use that key, you actually have to use Windows Hello Security Prompt because it's required by the key to decrypt data.

I still managed to find a couple of problems with their implementation that I can exploit using a console app. It is a private project because of its security nature but I added you and KeePassWinHello developers as collaborators for you all to check out.

[SeppPenner]

That just makes the Windows Security Prompt appear but it does nothing to the key, it is still available to the user to use for decryption. The real decryption is made by NCryptDecrypt method.

Is it possible to get the key without the prompt in this scenario? Ah, you mean, they just set the parameter NcryptPinCacheIsGestureRequiredProperty before they do something else, so if someone just removes this parameter check and uses the functionality like I (or you in your test project) have done, they could use the key again easily?

In the latest version of KeePassWinHello they now create a custom persistent key that is signed with Windows Hello. To use that key, you actually have to use Windows Hello Security Prompt because it's required by the key to decrypt data.

Ok, that sounds better.

I still managed to find a couple of problems with their implementation that I can exploit using a console app. It is a private project because of its security nature but I added you and KeePassWinHello developers as collaborators for you all to check out.

Mhm, bad :D I will have to check this as well...

[Angelelz]

Is it possible to get the key without the prompt in this scenario?

Exactly!

so if someone just removes this parameter check and uses the functionality like I (or you in your test project) have done, they could use the key again easily?

Yes.

You can check out some more details in the issue I opened in my console app for KeePassWinHello developers.

[Angelelz]

Well, as you can see they fixed the issue. If you want, I can start helping you update this library with persistent key feature and maybe even credential storage too. I would do it on my own, but I prefer to help you because I don't know how to build a nuget package!

[SeppPenner]

If you want, I can start helping you update this library with persistent key feature and maybe even credential storage too.

That would be a good idea. You have a better understanding of the issue and the implementation now, I guess...

I would do it on my own, but I prefer to help you because I don't know how to build a nuget package!

Well, you couldn't upload a nuget package with the same id either way (It's linked to the login I assume), but generating one is actually quite easy now. Do you use Visual Studio 2019? When you right-click on a project and go to Settings and to Package afterwards, you should be able to add your nuget package details and it will be created on build of the project if you check Generate nuget package on build. You can have a look at https://github.com/NuGet/docs.microsoft.com-nuget/blob/master/docs/quickstart/create-and-publish-a-package-using-visual-studio.md.

[Angelelz]

Do you use Visual Studio 2019?

I use VS 2017, should I upgrade?
I hope that is not an issue because I already started working on this library.

[SeppPenner]

I use VS 2017, should I upgrade?

You don't need to. I can update this nuget package easily after your changes are merged. I'm just not sure if this already works with the 2017 version.

[Angelelz]

Pushing PersistentKeyFeature
Error encountered while pushing branch to the remote repository: Git failed with a fatal error.
unable to access 'https://github.com/SeppPenner/WindowsHello/': The requested URL returned error: 403
Pushing to https://github.com/SeppPenner/WindowsHello

Trying to push into a new branch on this repo I'm getting this error. I guess I don't have privileges to write. Should I just fork this and create PRs from there?

Also, I'm not familiar with the VS's test functionality, when I start debugging WindowsHello.Tests what am I expecting? A console appears briefly and exits immediately?

[SeppPenner]

Trying to push into a new branch on this repo I'm getting this error. I guess I don't have privileges to write. Should I just fork this and create PRs from there?

Yeah, that's true. Can you fork this repository and create a pull request? This would be the easiest way. (Fork my repository, push something to your forked one, e.g. https://github.com/Angelelz/WindowsHello/ and create a pull request on github.com afterwards). That's how I do it and it's the easiest way I guess.

Also, I'm not familiar with the VS's test functionality, when I start debugging WindowsHello.Tests what am I expecting? A console appears briefly and exits immediately?

Do you run this project via the Play button (F5)? You should run it with the test explorer (There must be something like Test --> Windows --> Test explorer, I don't know the exact path because my settings are in German 😃). And yes, the test is not really good yet. It just covers one test case.

By the way, thank you already for your time with this issue 👍 I would probably need longer to integrate this into the project.

[SeppPenner]

Just for your information (I know, this is long fixed), but I have created a security advisory for the users here: GHSA-wvpv-ffcv-r6cw

[NicoleG25]

Just for your information (I know, this is long fixed), but I have created a security advisory for the users here: GHSA-wvpv-ffcv-r6cw

Hi !
Could you point me to the commit fixing the issue? :)

Thanks in advance !

[SeppPenner]

@NicoleG25 Basically, it's this one: a90c0cc. However, due to formatting changes, there have been a lot of unnecessary changes I reverted later... That's why it looks more complex than it actually is...

So, the interesting file is WinHelloProvider.cs. I have put the differences into another repository under SeppPenner/WindowsHelloChanges@e931dbc#diff-b09675fc9b649ff2689c77dd2d2a9ba5 with just two commits. I hope this makes it a bit easier to understand (Still, the code was formatted during the rework, so it's still a bit of a mess).

[SeppPenner]

New reference to Github advisory repo: GHSA-wvpv-ffcv-r6cw

[danergo]

Hi @SeppPenner, @Angelelz:

Would you mind summarizing within a few words on how 1.0.4 fixes this issue? I know it's done in a90c000, but still it's too huge to analyze.

Is the data now being saved into credential storage? As far as I can see from the readme, it is, please confirm. :)

Thank you very much!
I was looking for quite a long time ago to similar solution.

[SeppPenner]

Well, the main changes were done in WindowsHello/WindowsHello/, file WinHelloProvider.cs.

And as a quote from @Angelelz:

The problem is that this library just uses the default Windows Passport key to encrypt the data, and that key is obviously available for the user. To make the Windows Hello Security prompt appear, they just set a property to that key called NcryptPinCacheIsGestureRequiredProperty to 1. That just makes the Windows Security Prompt appear but it does nothing to the key, it is still available to the user to use for decryption. The real decryption is made by NCryptDecrypt method.

In the latest version of KeePassWinHello they now create a custom persistent key that is signed with Windows Hello. To use that key, you actually have to use Windows Hello Security Prompt because it's required by the key to decrypt data.