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

WinRing0 get's blocked by Avast/AVs #984

Open
Rem0o opened this issue Feb 23, 2023 · 58 comments
Open

WinRing0 get's blocked by Avast/AVs #984

Rem0o opened this issue Feb 23, 2023 · 58 comments

Comments

@Rem0o
Copy link
Contributor

Rem0o commented Feb 23, 2023

There's a CVE for WinRing0 https://www.cvedetails.com/cve/CVE-2020-14979/
See also https://posts.specterops.io/cve-2020-14979-local-privilege-escalation-in-evga-precisionx1-cf63c6b95896

@PhyxionNL What do you think of the proposed changed in the driver
(replacing https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-iocreatedevice to https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdmsec/nf-wdmsec-wdmlibiocreatedevicesecure) to remove the potential privilege escalation exploit?

@PhyxionNL
Copy link
Collaborator

I'm aware - I have no problem with updating WinRing0, it would even make it possible to drop InpOut by building it with PHYSICAL_MEMORY_SUPPORT set. But the driver needs to be signed, which requires an EV Code Signing certificate. The driver also needs to pass HLK checks as kernel drivers needs to be signed by MS.

@Rem0o
Copy link
Contributor Author

Rem0o commented Feb 23, 2023

Is the current sys file here signed? Who built it?

@PhyxionNL
Copy link
Collaborator

Yes, it's signed (by Noriyuki MIYAZAKI from CrystalMark), otherwise it wouldn't have worked. I'm not sure it was signed by an EV certificate either, this wasn't mandatory in the past and Windows allows these older signed drivers to work in order not to break them, but new drivers first need to be signed by EV so you can upload them to MS' hardware dev center, which then signs the driver with their own certificate. New kernel mode drivers need to be signed by MS otherwise Windows rejects them.

@Rem0o
Copy link
Contributor Author

Rem0o commented Feb 25, 2023

Will do some testing with the WdmlibIoCreateDeviceSecure. I will compile the kernel driver with it, then disable secure drivers on my dev machine and see if functionalities and everything are maintained. If that works, might try to get the whole loop of EV signing and everything going. I don't mind paying for it.

@Rem0o
Copy link
Contributor Author

Rem0o commented Feb 25, 2023

Update:
image

Applied the changes suggested in the article with the "secure" version of the device object. Compiled that, put it in LHM, rebooted into no-sign kernel driver mode, and it just works.

@PhyxionNL
Copy link
Collaborator

PhyxionNL commented Feb 25, 2023

Nice work 👍 Please set _PHYSICAL_MEMORY_SUPPORT then as well so we can ditch InpOut too. BTW, the driver version in the repository here (1.2.0.5) isn't the latest version (1.3.0.x). WinRing0 isn't downloadable anymore on the openlibsys website, but there are some GitHub projects that have a newer version: https://github.com/QCute/WinRing0/releases, and this https://github.com/GermanAizek/WinRing0 (the project files here are GPL so you might need to create a new project yourself - all original files still seem to be under the original BSD license). This last project might have some other useful changes too (check history).
I'm attaching a 1.3.0 version I found on the web that seems legit (mirrored originally from sourceforge).

WinRing0_1_3_0.zip

@Rem0o
Copy link
Contributor Author

Rem0o commented Feb 25, 2023

Checked the sources for the zip you sent, and it still seems like 1.2.0.5
image

By the way for the _PHYSICAL_MEMORY_SUPPORT thing, you'll have to be more precise. I'm really not familiar with Windows kernel stuff. I will probably open a repo for this.

Also also, looked at the Extended validation Code signing stuff, and it seems like you have to be registered as an organization to be able to get the cert and renew it year after year. Is there any example of open source kernel drivers being used at large? If said association is myself, any usage of that driver will fall under my responsibility and it might get ugly with the legal stuff. Also, I'm no organization, just a solo dev.

@PhyxionNL
Copy link
Collaborator

Ah, maybe it's the same version then, a bit weird that that's not increased with 1.3.0 🤔 Might be worth comparing the files.
_PHYSICAL_MEMORY_SUPPORT is a preprocessor directive. You can add it VS project settings, or modify the source a bit to use the code between the ifdef.

I think you indeed need to be an organization for EV, and probably have that organization registered somewhere. I don't think a "GitHub organization" will suffice. You shouldn't have to renew if you don't need to resign the driver (it'll stay valid if you timestamp it and MS should do that for you). From a legal point of view the BSD license should take care of that (wiki).

@CrazyKidJack
Copy link

Also also, looked at the Extended validation Code signing stuff, and it seems like you have to be registered as an organization to be able to get the cert and renew it year after year. Is there any example of open source kernel drivers being used at large? If said association is myself, any usage of that driver will fall under my responsibility and it might get ugly with the legal stuff. Also, I'm no organization, just a solo dev.

@Rem0o
Not sure where you live but where I live I think I could start a sole proprietorship for free and get an EV cert that way. While starting the "company" would be free it is a bit time consuming so if there are other options, I'd rather those be pursued first... but if that is the only option, I'd be willing to do it if we could work something out for the EV cert.

I know I'm a rando... but if you are interested in chatting about it, you can send me an email here: fancontrolsevcert@gmail.com

@Rem0o
Copy link
Contributor Author

Rem0o commented Mar 14, 2023

@CrazyKidJack I don't think that would work either. Goal of EV signing is to avoid bogus entities. There is a whole process of verification, with address and everything.

@CrazyKidJack
Copy link

CrazyKidJack commented Mar 14, 2023

@CrazyKidJack I don't think that would work either. Goal of EV signing is to avoid bogus entities. There is a whole process of verification, with address and everything.

@Rem0o
I aware, and it would not be bogus. It would be a real sole proprietorship / real company. That company just would not produce anything, would not have have any expenses, and would not make any revenue.

For EV, there is no requirement that the organization be a successful company, just that it be a real organization that is officially registered with a governmental body (so that the CA can verify its existence and that you are the owner or authorized agent thereof).

@PatrickSchmidtSE
Copy link

PatrickSchmidtSE commented May 9, 2023

So there is a solution to the CVE but it cannot be applied, because the driver must be signed properly, is that correct? :( if so thats sad.

In another forum they said, that the whole signing process is not as complicated ( still some work and money)

"However, (this is all assuming you live in the US) I wanted to explicitly mention here that you don't have to be "incorporated" (which you corrected stated costs lots of money) in order to get an EV cert. You can get an EV cert as a sole proprietorship (which in many states in the US is free to create) and if that sole proprietorship does not have any expenses or revenue, then you also do not have to pay additional taxes (or even file any additional tax forms) in many US states.

You would probably have to "officially" register the sole proprietorship for the EV cert validation process, but even that is usually not required for a sole proprietorship... you'd only have to do it in this case for the validation process. The official registration might cost a small fee depending on where you live."

@PatrickSchmidtSE
Copy link

PatrickSchmidtSE commented May 11, 2023

BSD license

Did you set

Update: image

Applied the changes suggested in the article with the "secure" version of the device object. Compiled that, put it in LHM, rebooted into no-sign kernel driver mode, and it just works.

Did you set up a repo with those changes? Would be nice to test them for myself :)

EVGA who previously used WinRing0 seems to use an own driver now?

image
https://posts.specterops.io/cve-2020-14979-local-privilege-escalation-in-evga-precisionx1-cf63c6b95896

@Rem0o
Copy link
Contributor Author

Rem0o commented May 11, 2023

@SearchForTheCode
code is here https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/tree/master/WinRing0

Just modified that 1 line and compiled.

@PatrickSchmidtSE
Copy link

Ah nice, did the same , it seems to work but suddenly did not read any CPU temperature anymore. Need to dig into it.
But i can confirm that Avast Business is not blocking it anymore with that changes.

@mtaalas
Copy link

mtaalas commented Jun 5, 2023

It seems that this is the relevant to why this is happening?
https://nvd.nist.gov/vuln/detail/CVE-2020-14979

This has broken many other projects as well. But yeah, security is ever changing field and software just needs to keep up...

Thank you for your hard work everyone. You're awesome!

@namazso
Copy link
Contributor

namazso commented Jun 10, 2023

I have a (signed) driver that could be used as a replacement, however it'd require quite some changes to the code. Plus, I have the signing keys for the security features of the driver that prevent it being a WinRing0 v2, but that'd mean I'd need to review and sign all the changes affecting code that needs kernelmode features.

I'm not sure how comfortable maintainers here would be with that, so I might just end up with a fork.

@namazso
Copy link
Contributor

namazso commented Jun 10, 2023

If you're wondering about why signing and "why not just admin restrict the device", that's because anticheats would still hate that. Plus it'd go against multiple items on Microsoft's recommendations:

Do not production sign test code

Security checklist item #4: Do not production code sign development, testing, and manufacturing kernel driver code.

Kernel driver code that is used for development, testing, or manufacturing might include dangerous capabilities that pose a security risk. This dangerous code should never be signed with a certificate that is trusted by Windows. The correct mechanism for executing dangerous driver code is to disable UEFI Secure Boot, enable the BCD "TESTSIGNING", and sign the development, test, and manufacturing code using an untrusted certificate (for example, one generated by makecert.exe).

Code signed by a trusted Software Publishers Certificate (SPC) or Windows Hardware Quality Labs (WHQL) signature must not facilitate bypass of Windows code integrity and security technologies. Before code is signed by a trusted SPC or WHQL signature, first ensure it complies with guidance from Creating Reliable Kernel-Mode Drivers. In addition the code must not contain any dangerous behaviors, described below. For more information about driver signing, see Release driver signing later in this article.

Examples of dangerous behavior include the following:

  • Providing the ability to map arbitrary kernel, physical, or device memory to user mode.
  • Providing the ability to read or write arbitrary kernel, physical or device memory, including Port input/output (I/O).
  • Providing access to storage that bypasses Windows access control.
  • Providing the ability to modify hardware or firmware that the driver was not designed to manage.

@javs
Copy link
Contributor

javs commented Jun 10, 2023

@namazso This would be great IMO! Would your new driver be open source?

Also, if you are not planning on exposing arbitrary memory mappings, would that mean we would move some of the logic that is today in LHM to the kernel driver?

@namazso
Copy link
Contributor

namazso commented Jun 10, 2023

Would your new driver be open source?

Possibly, I haven't decided yet. The other option I'm considering is source-available, and some restrictions to make sure commercial entities don't just straight up copy it (and just sell alternative licensing to them).

Also, if you are not planning on exposing arbitrary memory mappings, would that mean we would move some of the logic that is today in LHM to the kernel driver?

It implements an interpreter that you can feed signed programs into.

@mtaalas
Copy link

mtaalas commented Jun 10, 2023

[...] some restrictions to make sure commercial entities don't just straight up copy it (and just sell alternative licensing to them).

This... 100% this. There are licenses that prevent commercial use of source code and/or if any part of that code is used, the resulting program must be open source as well.

But let's not start any longer discussion about that topic here as of now

@namazso
Copy link
Contributor

namazso commented Jun 10, 2023

There are licenses that prevent commercial use of source code

That's not open-source, that's source available - open source requires that there are no restrictions on field of use.

and/or if any part of that code is used, the resulting program must be open source as well.

Microsoft also has rules about what they sign. GPLv3 has clauses about giving up keys if signature enforcement is present (and therefore a no-go for Microsoft), so if I just licensed it as that, it'd be as useful as giving no rights at all, since OSS projects couldn't use it for its single purpose.

In the meantime, open-sourcing their fork might not be a big deal for these companies either, as all they'd want to change is the device path and the public key burned into the driver. So I wouldn't see a penny either from their very commercial usage.

So yeah, lots of considerations, which is why I didn't just dump it on GitHub already. But let's just stick to technology in this issue. It will be out in some shape or form, and certainly free as in beer to non-commercial users.

PS. I'm also available on mail and a bunch of chat platforms for more direct chat.

@namazso
Copy link
Contributor

namazso commented Jun 10, 2023

Decided to publish the source, for now without any extra rights given to anyone until I figure that stuff out.

Edit: Went with GPLv2 and an exception to allow FanControl and the likes to exist, but protect the freedom of the hardware-interfacing code.

@namazso
Copy link
Contributor

namazso commented Jun 11, 2023

Sorry for spamming this issue, but I'm just interested in any feedback.

I updated the license of PawnIO and released an example module in a new modules repo that should give an idea how they will look. This was already tested on a live version of the driver. The natives are not documented for now, nor is basically anything else, this is something I plan to address in the future.

Some tooling and libraries will be the next thing I'll release, followed by binary releases. These will most likely be proprietary (for some control over distribution format, enforcing uniform installations to avoid what happened to WinRing0), but otherwise identical to the released code.

It will come in 2 flavors:

  • Unrestricted (accepting any blobs), test signed.
  • Restricted (signed by me only), Microsoft signed.

The first can be used for development and testing, while the second for public releases.

@PhyxionNL
Copy link
Collaborator

There are various commercial apps using LHM, such as https://www.sentrysoftware.com/docs/hws-otel-collector/latest/connectors/LibreHardwareMonitor.html. I would gradly swap out WinRing0, but the driver's license mustn't be more restrictive than the project license. If you licensed it under MPL 2.0 then it would be under the same rules/restrictions as LHM. MPL 2.0 for example allows commercial usage without disclosing your entire source code. If we were to include your driver then LHM would pretty much be GPL licensed.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

If we were to include your driver then LHM would pretty much be GPL licensed.

That certainly isn't the intention! The exception on the GPLv2 is there for PawnIOLib which this way can be LGPL, which means anyone can link arbitrarily licensed LHM forks to it.

The idea is that you'd be distributing the LHM code under MPL, and ask users to install PawnIO, which is just the lib and the sys. This would be a slight change in behavior, however shouldn't affect licensing affairs at least.

Why do you think the exception is insufficient?

@PhyxionNL
Copy link
Collaborator

That would largely address LHM code licensing (LGPL is not 100% compatible with MPL 2.0 afaik) but not its usage. It's also not very convenient for portable users that they have to install PawnIO separately. One integrated package (so LHM would have to ship with PawnIO) that transparently installs it would be much easier for end users (similar to WinRing0).

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

LGPL is not 100% compatible with MPL 2.0 afaik

There are some nuances when combining actual code, but both licenses allow dynamic linking to arbitrarily licensed code, including fully proprietary. This symmetrically covers dynamic linking them to each other.

That would largely address LHM code licensing but not its usage.

Can you expand on this? I'm sure we can work out something for whatever use case there is out there that wouldn't work.

It's also not very convenient for portable users that they have to install PawnIO separately.

I'd rather see it as how Wireshark portable works - you can look at packet captures, but if you want to make a local capture you need to install NPcap. Same could be here except on a per-device basis.

One integrated package that transparently installs it would be much easier for end users (similar to WinRing0).

I'm not so sure. There's a constant stream of issues where multiple programs ship the same WinRing0 while registering it under different names and places. I'd like to fix that.

@PhyxionNL
Copy link
Collaborator

Can you expand on this?

With a separate installer to install the driver it's probably less of an issue, I was talking about integrating it into LHM similar to WinRing0.

I'd rather see it as how Wireshark portable works - you can look at packet captures, but if you want to make a local capture you need to install NPcap. Same could be here except on a per-device basis.

Wireshark is a very specialized tool that's probably not used a lot by novice users. LHM only has a portable edition, and FanControl is also just a portable edition. Surely users can install a separate driver, but I can already see tons of reports about missing sensors. LHM without WinRing0 will produce virtually no sensors and there's not really anything useful in the program. You'd basically have to add a big message that a separate driver install is required. I don't think that's as convenient as it could be.

I'd like to fix that.

Same! But I would also like to get it done right 🙂

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

LHM only has a portable edition, and FanControl is also just a portable edition.

Well, I'd argue about whether registering a system-wide service is a "portable" thing to do. Running kernel drivers from user-writable directories is also a bad idea in general, it should be done from an admin-only dir (like program files).

If you want to do it right, you'll end up with what's commonly called an installer. Just silent at most.

I can already see tons of reports about missing sensors. You'd basically have to add a big message that a separate driver install is required.

I was thinking more like a warning popup on startup like "Blah blah, PawnIO is required for many sensors. Start installer?" that should reduce user friction. Especially if the installer is bundled.

For LHMLib it can just be marked as a "breaking change". Considering a few bits would need to be rewritten, that's a pretty good idea in general too. It also would be a breaking change anyway, as people who had WinRing0 installed by other software would need to run it as admin now, while the with the vulnerable version it just worked.


The only solution other than an installer I can see is some convoluted "is another copy of me already running" on startup and "am I the last one using this copy" on shutdown logic. Plus copying to a system directory on start and deleting when needed. Which is very overcomplicated and error-prone.

@PhyxionNL
Copy link
Collaborator

The only solution other than an installer I can see is some convoluted "is another copy of me already running" on startup and "am I the last one using this copy" on shutdown logic. Plus copying to a system directory on start and deleting when needed. Which is very overcomplicated and error-prone.

That's pretty much what WinRing0 is doing, it's certainly not a perfect solution when multiple apps are using it. I guess a message that a driver install is required could be OK, we can experiment with it. It would then be useful to include a function in PawnIOLib to check if the driver is installed and which version is installed so we can take action on that.
Would PawnIO also allow us to get rid of InpOut? I'd be nice to only use one driver. A PR with PawnIO integration would be appreciated, it'd probably be easier for you 🙂. Oh, your PawnIO.Releases repo isn't available (or private).

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

It would then be useful to include a function in PawnIOLib to check if the driver is installed and which version is installed so we can take action on that.

Indeed. The idea was to just test for PawnIOLib's presence.

Would PawnIO also allow us to get rid of InpOut? I'd be nice to only use one driver.

Yes. These are the primitives it provides to pawn modules: https://github.com/namazso/PawnIO.Modules/blob/main/include/native.inc

Now, since these are very powerful, I obviously won't sign rootkit-tier code like winring0, but adding the bare minimum of code for ensuring you're talking to the right device and doing the talking should be fairly easy. Tests can be done with the unrestricted driver, and signed blobs can be used with the properly signed one.

A PR with PawnIO integration would be appreciated, it'd probably be easier for you 🙂.

Well, moving the reading into pawn module(s) would take a community effort, but I can do a single device easily. Since I have a Ryzen CPU, might just go with porting that.

Oh, your PawnIO.Releases repo isn't available (or private).

Yes, I've been slowly publishing the various components as I got them cleaned up and ready. I still need to add some finishing touches to that one.

@PhyxionNL
Copy link
Collaborator

PhyxionNL commented Jun 13, 2023

There's nothing using WinRing0 directly, everything is wrapped inside Ring0, so if the methods therein are reimplemented with PawnIO then effectively the entire library would immediately work with the new driver. The only exception is AMD Ryzen SMU and Gigabyte LPC that's also using InpOut32.
It might be worth checking to see what's necessary to implement in the modules in Ring0 class. Most of the code there is just wrapping and installing the driver, the main code is https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/blob/master/LibreHardwareMonitorLib/Hardware/Ring0.cs#L450-L546

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

if the methods therein are reimplemented with PawnIO then effectively the entire library would immediately work with the new driver.

Yeah, unfortunately as I mentioned earlier, doing such thing goes directly against Microsoft's recommendations and would probably land us on their blocklist like WinRing0 and its copies.

Which is why PawnIO implements a VM, and requires blobs signed by me. So the idea is to move a minimal amount of checks and reading into Pawn modules (like checking if we're running on Intel before writing Intel MSRs), making me sign that, and just load that signed blob in LHM, and query that. Basically mini drivers.

@PhyxionNL
Copy link
Collaborator

I think those drivers ended up there because they're actually vulnerable, not because of the ability to read and write artibrary data.

I'm not sure how useful PawnIO would be otherwise, as it would:

  • Probably require quite some modules.
  • Rely on you to build and sign each module. That's OK if this is a one-time thing, but if a new device/component needs to be added, it probably wouldn't work with the existing module, so we'd back to square one.
  • Probably be too difficult for the community to implement all these different modules to get the same functionality as-is.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

I think those drivers ended up there because they're actually vulnerable, not because of the ability to read and write artibrary data.

That's varying. The Admin-only RTCore64 used to be there for like half a year. It was removed recently, but it might be put back in the future. Basically no one really likes these kind of drivers because often they're often abused for admin -> kernel. Even though that's supposedly not a security boundary (although Microsoft always patches such exploits in their own code), malware frequently ships and uses them, so they have awful reputation.

I'm not sure how useful PawnIO would be otherwise

Well, that is unfortunate, but understandable. I might just go ahead with a fork instead then (trying to make this not sound like a threat, we just have different incentives, like you making sure vital dependencies don't die, and me trying not to get my driver blacklisted).

I also didn't like certain architectural choices here anyways (like requiring admin, instead of splitting into service (admin) + client (user)).

Probably be too difficult for the community to implement all these different modules to get the same functionality as-is.

Most modules would be a series of like 3 io reads and stuffing it into the out array. It takes a bit of time, but certainly not skill. I guess OSS projects don't exactly have so much of that either.

@Rem0o
Copy link
Contributor Author

Rem0o commented Jun 13, 2023

Well, that is unfortunate, but understandable. I might just go ahead with a fork instead then

@namazso I followed the conversation along seeing where it would end up. The alternative proposed could be promising, but in any case I think we would need to see a working prototype (in a branch/fork) anyway with everything working to see if it looks viable to go ahead and port everything away from WinRing0 in the master branch here. I'm curious as to exactly how it would look, as if I understand correctly, every "arbitrary" memory mapping written right now in Code as Ring0.Read( address ) / Ring0.Write( address ) would need to be hardcoded in a small module, which would then need to be signed, and packaged/integrated into the app.

I also didn't like certain architectural choices here anyways (like requiring admin, instead of splitting into service (admin) + client (user)).

That's just for the winform LHM app itself, which I see as a functional/useful demo app for LHMLib. It just so happens to be a simple 1 process application that is "portable". I can very much see a LHMService folder being added here in the future, for any client to be built on top with gRPC/REST API/IPC or whatever. I don't see how the LHM client/app is relevant to this conversation.

@PhyxionNL
Copy link
Collaborator

Most modules would be a series of like 3 io reads and stuffing it into the out array. It takes a bit of time, but certainly not skill. I guess OSS projects don't exactly have so much of that either.

It's not just writing the modules, it's also relying on you to sign and build them. That's doesn't have to be a huge problem if the modules are flexible enough to not require that for every change. Take this recent commit for example, 701e8ad, this added some ReadByte's for the Ring0 driver. This is the kind of flexibility that should be possible to achieve without rewriting the modules. Most of the PRs that add new sensors are from users that have a system with a sensor and they need to be able to try their changes out. If every change they make requires a rewritten module, it's not feasible for either party.

What about some other ways to secure the driver? Such as checking if the calling process has a trusted certificate. Most AVs/FW also show that data, such as:
image

That all said, I'm more than happy to take a look at a PR and see what's what. I don't know what you have exactly in mind, but if we'd end up keeping WinRing0 in addition to the PawnIO driver then we just have yet another driver.

I don't see how the LHM client/app is relevant to this conversation.

Exactly.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

Btw, about the two driver thing - you could use a better (but still vulnerable) driver to get rid of one:

https://crystalmark.info/download/archive/CrystalCPUID/CrystalCPUID415Src.zip
https://crystalmark.info/download/archive/CrystalCPUID/CrystalCPUID415x64.zip

I also have a copy of OpenLibSys 1.0.0.2, but it's missing source and can't find it on github.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

@Rem0o

The alternative proposed could be promising, but in any case I think we would need to see a working prototype (in a branch/fork)

I'll probably work on that in the following weeks.

every "arbitrary" memory mapping written right now in Code as Ring0.Read( address ) / Ring0.Write( address ) would need to be hardcoded in a small module, which would then need to be signed, and packaged/integrated into the app.

That is correct.

I don't see how the LHM client/app is relevant to this conversation.

Just mentioned another thing I wanted to change if I went forward with a fork, not that relevant.

@PhyxionNL

users that have a system with a sensor and they need to be able to try their changes out.

That's perfectly doable with the test signed version - it requires no signatures on the modules.

Such as checking if the calling process has a trusted certificate.

Unfortunately that's largely useless (and incorrect in the AV). Another app can easily just start a suspended copy and inject. Multiple HW vendors implemented such checks, and they're all frequently circumvented.

That all said, I'm more than happy to take a look at a PR and see what's what.

Sure thing. I could also do an usermode reduced PawnIO implementation on top of WinRing0, in case that could help anything (like upstreaming the changes but keeping WinRing0 as a backend / keeping it optionally)

@PhyxionNL
Copy link
Collaborator

That's perfectly doable with the test signed version - it requires no signatures on the modules.

Presumably you would need to disable driver signature enforcement in Windows then? Even so, we'd still end up relying on you only to sign the modules so we'd always have to wait for you to resign, etc. I don't think that's feasible.

Unfortunately that's largely useless (and incorrect in the AV). Another app can easily just start a suspended copy and inject. Multiple HW vendors implemented such checks, and they're all frequently circumvented.

Do you have any other idea in mind? Surely something a little bit more practical than this should be possible.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

Presumably you would need to disable driver signature enforcement in Windows then?

Yeah

Even so, we'd still end up relying on you only to sign the modules so we'd always have to wait for you to resign, etc. I don't think that's feasible.

Well, yes.

Do you have any other idea in mind? Surely something a little bit more practical than this should be possible.

I ended up with this solution after exhausting practically everything else to lock down the arbitrary access functions to not provide practically kernel code execution. There isn't much ways to ensure integrity of a usermode process. Windows has a signature enforcement process mitigation, but that's for Microsoft signatures only.


Anyway, I think an installed PawnIO first, WinRing0 (or CrystalSysInfo rather) backed UM implementation fallback could be a doable way. It would also solve the installer annoyances - people can just opt in to use it to avoid vulnerable drivers, but it also works without. I also don't mind going full MPL 2.0 in such an implementation.

This would let us code-share at the very least, while providing a not-worse-than-now fallback.

@PhyxionNL
Copy link
Collaborator

Microsoft's response to the vulnerability of WinRing0 was that the driver had to be locked down to administrators with IoCreateDeviceSecure, so I guess that's sufficient enough for them? With administrator access you can pretty much already do whatever you want anyway (or at least circumvent restrictions).

Otherwise, with PawnIO, basically the entire LPC would have to be rewritten in modules (along with several other classes using Ring0). I really don't see that happening.

For this to be viable I think we'd have to look into a way that's largely compatible with the project.

@namazso
Copy link
Contributor

namazso commented Jun 13, 2023

I see, OK. I'll look into actually porting things, so that I can have a better sense of just how complicated all of it would be.

@namazso
Copy link
Contributor

namazso commented Jun 14, 2023

Well, I "ported" Ryzen SMU and it indeed looks more like a rewrite at this point. It was slightly more challenging because my SMU is not actually supported by LHM (Raphael, 0x540104) and I just added some made up size (0x1000) when testing, but just the porting part itself took multiple hours.

I still need to integrate it in LHM, since so far I just compared the floats it read to make sure it works at least.

@PhyxionNL
Copy link
Collaborator

That's indeed what I suspected. For the entire project I don't see a complete rewrite happening. I'd love to get rid of WinRing0 with a proper replacement, but it should be somewhat feasible to integrate ☺️

Looking at the guidelines, I think what should be possible is something similar to the example here:
https://learn.microsoft.com/en-us/windows-hardware/drivers/driversecurity/driver-security-dev-best-practices#providing-the-ability-to-read-and-write-msrs
So no free for all access but only access to specific ranges. That'd be a lot easier to integrate in the existing code as a large part is just fixed addresses (or fixed address + a read offset between 0 and 255 for example). What's then very dynamic, we can look into adding some specific modules for. What do you think?

@namazso
Copy link
Contributor

namazso commented Jun 15, 2023

Yes, that's pretty much my intention. I just need to be able to constrain it enough that the worst you can do is a crash / unstable hardware / etc, and not a kernel compromise.

I wanted to get Ryzen SMU out of the way for that sole reason - because it needs physmem r/w at an unknown location. I'm thinking of relaxing it to allow up to a page of reading from the base, so that I can just throw out the size matching code.

I'd love to provide the least restrictions that are still "safe". A crash is not useful to an attacker that can already just call NtRaiseHardError to crash the system.

@PhyxionNL
Copy link
Collaborator

Sounds good. It might be a good idea to also separate the read/write ranges as more ranges are being read than written. Obviously some writes still need to happen as it would otherwise be impossible to control fans 🙂

@alberts8
Copy link

@namazso
I guess that msr and mmio register reading and writing on intel cpu's would be doable that way as well. Would it be necessary to limit which registers can be used or would it be enough to restrict such a module to only have access to the msr's, pci configuration and the whole mmio register range.

@namazso
Copy link
Contributor

namazso commented Jun 16, 2023

Tough question, I'll see when I get there, it really depends on what we might accidentally hit with a range. For example for MSRs reading can at most leak KASLR addresses, but arbitrary write is enough for complete kernel takeover. There's multiple ways even, you can overwrite KERNEL_GS_BASE to run on user stack, or overwrite LSTAR so it points past the swapgs...

In the meantime, I got something done with LpcIO: https://github.com/namazso/PawnIO.Modules/blob/main/LpcIO.p

It didn't seem too dangerous to just give full access to the Super I/O, but I did move the enter/exit and detection logic inside so that I can handle Gigabyte EC later.

Seems to work for my Nuvotron NCT6686D as far as detection goes. I should probably spend some time with integration and getting some distribution package of PawnIO going, but I might as well run into all the bugs in my driver before a public release.

@PatrickSchmidtSE
Copy link

PatrickSchmidtSE commented Jun 21, 2023

Update: image

Applied the changes suggested in the article with the "secure" version of the device object. Compiled that, put it in LHM, rebooted into no-sign kernel driver mode, and it just works.

I also tried this code for testing purposes. And while all CPU related stuff seem to work, i got problems on the mainboard parts. Voltages and Mainboard Temperatures are not detected any more as sub hardware. I only applied those changes and booted in the non-signed kernel driver mode. CPU Temp works like normal.

Any explanation why this could be?

EDIT: Forget it, for everyone interested, the problem was with the KMDF version build. The system i tested had an older KMDF version than the build process used

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

No branches or pull requests

8 participants