Re: [ros-dev] [ros-diffs] 02/08: [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a user-mode hard-error BSOD.

[Alex Ionescu]

Is there a point to this blatant behavior change?

Best regards,
Alex Ionescu

On Sun, Apr 1, 2018 at 3:04 PM, Hermès Bélusca-Maïto <
[e-mail]> wrote:
> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=
> f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
>
> commit f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
> Author:     Hermès Bélusca-Maïto [e-mail]
> AuthorDate: Sun Apr 1 14:46:19 2018 +0200
> Commit:     Hermès Bélusca-Maïto [e-mail]
> CommitDate: Sun Apr 1 22:39:31 2018 +0200
> [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a
> user-mode hard-error BSOD.
>
> ntoskrnl/ex/harderr.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ntoskrnl/ex/harderr.c b/ntoskrnl/ex/harderr.c
> index 84f409a1bb..a5200e3e74 100644
> --- a/ntoskrnl/ex/harderr.c
> +++ b/ntoskrnl/ex/harderr.c
> @@ -132,8 +132,18 @@ ExpRaiseHardError(IN NTSTATUS ErrorStatus,
>      /* Check if this error will shutdown the system */
>      if (ValidResponseOptions == OptionShutdownSystem)
>      {
>    /* Check for privilege */
>    if (!SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
>    /*
>     * Check if we have the privileges.
>     *
>     * NOTE: In addition to the Shutdown privilege we also check
>
> whether
>     * the caller has the Tcb privilege. The purpose is to allow only
>     * SYSTEM processes to "shutdown" the system on hard errors (BSOD)
>     * while forbidding regular processes to do so. This behaviour
>
> differs
>     * from Windows, where any user-mode process, as soon as it has
>
> the
>     * Shutdown privilege, can trigger a hard-error BSOD.
>     */
>    if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode) ||
>        !SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
>    {
>        /* No rights */
>        *Response = ResponseNotHandled;

[M. Ziggyesque]

While this has application in protecting a system from rogue or trojan apps, supposedly a shutdown privilege would be something granted manually by a SYSTEM level process that is trusted, so that if the system is not recoverable a graceful shutdown can be attempted with minimal process switching; which might make things worse. If that can be bypassed so a virus could exploit it, that's the API I'd look to change, not so much at the point of this diff. Also, the code is double calling the ShutdownPrivelege check where only the first appears necessary, unless there's a significant chance of a task switch.

------ Original message------
From: Alex Ionescu
Date: Sun, Apr 1, 2018 10:22 PM
To: ReactOS Development List;Hermès Bélusca-Maïto;
Cc: Linda Wang;
Subject:Re: [ros-dev] [ros-diffs] 02/08: [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a user-mode hard-error BSOD.

Is there a point to this blatant behavior change?

Best regards,
Alex Ionescu

On Sun, Apr 1, 2018 at 3:04 PM, Hermès Bélusca-Maïto <[e-mail]:[e-mail]> wrote:
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f0729b30bb79d6f538cf2b...https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.reactos.org%2F%3Fp%3Dreactos.git%3Ba%3Dcommitdiff%3Bh%3Df0729b30bb79d6f538cf2b9578ff8ebe7989f8d3&data=02%7C01%7C%7C284bcff3fef645cd57f508d598408a05%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636582325322486573&sdata=9NuDzfRL08cyls9qq%2F2p5gfPAQGtOLd58Ubd896u%2Fbw%3D&reserved=0

commit f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
Author:     Hermès Bélusca-Maïto <[e-mail]:[e-mail]>
AuthorDate: Sun Apr 1 14:46:19 2018 +0200
Commit:     Hermès Bélusca-Maïto <[e-mail]:[e-mail]>
CommitDate: Sun Apr 1 22:39:31 2018 +0200

[NTOSKRNL] Forbid processes without the Tcb prvilege to perform a user-mode hard-error BSOD.
---
 ntoskrnl/ex/harderr.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ntoskrnl/ex/harderr.c b/ntoskrnl/ex/harderr.c
index 84f409a1bb..a5200e3e74 100644
--- a/ntoskrnl/ex/harderr.c
+++ b/ntoskrnl/ex/harderr.c
@@ -132,8 +132,18 @@ ExpRaiseHardError(IN NTSTATUS ErrorStatus,
     /* Check if this error will shutdown the system */
     if (ValidResponseOptions == OptionShutdownSystem)
     {
-        /* Check for privilege */
-        if (!SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
+        /*
+         * Check if we have the privileges.
+         *
+         * NOTE: In addition to the Shutdown privilege we also check whether
+         * the caller has the Tcb privilege. The purpose is to allow only
+         * SYSTEM processes to "shutdown" the system on hard errors (BSOD)
+         * while forbidding regular processes to do so. This behaviour differs
+         * from Windows, where any user-mode process, as soon as it has the
+         * Shutdown privilege, can trigger a hard-error BSOD.
+         */
+        if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode) ||
+            !SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
         {
             /* No rights */
             *Response = ResponseNotHandled;

[Hermès BÉLUSCA-MAÏTO]

Yes, to only allow programs that REALLY REALLY REALLY REALLY ….. need to do so to trigger the hard-error “shutdown” BSOD from user-mode to do so, and these programs would better be only those that run only in SYSTEM rights, and more exactly these include CSRSS, WINLOGON and SMSS when something very bad happen to them.

I would not appreciate, for example, that when I run a program under a not-so privileged account (like, some random user account) that has just the shutdown privilege to shut the computer down properly, that this program suddently “BSODS” my machine.

To these programs, I say “f$ck these!”

Regards,

Hermès

De : Ros-dev [mailto:[e-mail]] De la part de Alex Ionescu
Envoyé : lundi 2 avril 2018 04:20
À : ReactOS Development List; Hermès Bélusca-Maïto
Cc : Linda Wang
Objet : Re: [ros-dev] [ros-diffs] 02/08: [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a user-mode hard-error BSOD.

Is there a point to this blatant behavior change?

Best regards,
Alex Ionescu

On Sun, Apr 1, 2018 at 3:04 PM, Hermès Bélusca-Maïto [e-mail] wrote:

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f0729b30bb79d6f538cf2b...

commit f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
Author:     Hermès Bélusca-Maïto [e-mail]
AuthorDate: Sun Apr 1 14:46:19 2018 +0200
Commit:     Hermès Bélusca-Maïto [e-mail]
CommitDate: Sun Apr 1 22:39:31 2018 +0200

[NTOSKRNL] Forbid processes without the Tcb prvilege to perform a user-mode hard-error BSOD.
---
 ntoskrnl/ex/harderr.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/ntoskrnl/ex/harderr.c b/ntoskrnl/ex/harderr.c
index 84f409a1bb..a5200e3e74 100644
--- a/ntoskrnl/ex/harderr.c
+++ b/ntoskrnl/ex/harderr.c
@@ -132,8 +132,18 @@ ExpRaiseHardError(IN NTSTATUS ErrorStatus,
     /* Check if this error will shutdown the system */
     if (ValidResponseOptions == OptionShutdownSystem)
     {
-        /* Check for privilege */
-        if (!SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
+        /*
+         * Check if we have the privileges.
+         *
+         * NOTE: In addition to the Shutdown privilege we also check whether
+         * the caller has the Tcb privilege. The purpose is to allow only
+         * SYSTEM processes to "shutdown" the system on hard errors (BSOD)
+         * while forbidding regular processes to do so. This behaviour differs
+         * from Windows, where any user-mode process, as soon as it has the
+         * Shutdown privilege, can trigger a hard-error BSOD.
+         */
+        if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode) ||
+            !SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
         {
             /* No rights */
             *Response = ResponseNotHandled;