src: add WDAC integration (Windows) #54364

rdw-msft · 2024-08-14T00:35:14Z

Add calls to Windows Defender Application Control to enforce integrity of .js, .json, .node files.

Motivation

In the past I've spoken to @RafaelGSS and the Node security work group about code integrity. We feel like it's an important defense in depth feature and the removal of the --experimental-policy feature gives us room to use OS-level code integrity features. This is a first pass at a CI implementation that cooperates with the OS.

These additions add an additional layer of defense against malicious code execution on a system that is enforcing code integrity. Code Integrity enforcement mitigates the risk of malicious code modifications after signing-time. For example, they can prevent an arbitrary file-write vulnerability from turning into an RCE. These additional checks are only made if the OS in code integrity enforcement mode and has explicitly set a configuration value in their code integrity policy to have Node enforce CI.

Windows Defender Application Control (WDAC)

Official documentation: https://learn.microsoft.com/en-us/windows/security/application-security/application-control/windows-defender-application-control/wdac-and-applocker-overview

WDAC is the Windows code integrity subsystem. It enforces code integrity using digital signatures. This is straightforward for DLLs and EXEs, since they're loaded using well known entry points in the OS. However, dynamic runtimes (interpreters like Node.js or Python) make it difficult to determine whether a file being read will be executed. In order to assist dynamic runtimes, WDAC provides an interface for runtimes to call with the code they intent to execute, and WDAC will determine if the runtime is allowed to execute the code based off signature information. WDAC (wldp.dll) exposes various methods for runtimes to ask the OS questions about the code integrity policy on the system. These changes leverage three interfaces WldpCanExecuteFile, WldpGetApplicationSettingBoolean, and WldpQuerySecurityPolicy.

WldpCanExecuteFile - given a file handle, determines if the file is allowed to be executed from WDAC policy.

WldpGetApplicationSettingBoolean - Queries WDAC policy for an application-defined setting with a boolean value. This can be thought of as a more secure setting store.

WldpQuerySecurityPolicy - WdlpGetApplicationSettingBoolean is not available on all versions of Windows, so this method provides fallback behavior to query WDAC policy settings.

High-level implementation

At startup, Node will query WDAC policy to see if it should enter integrity enforcement mode. If the policy provider Node.js has set the policy setting EnforceCodeIntegrity to TRUE, then Node will call WldpCanExecuteFile when a .js, .json, or .node file is loaded using require. If this setting is not enabled, the call to WldpCanExecuteFile will not be made and execution will continue as normal.

Additionally, if EnforceCodeIntegrity is set to TRUE, we disable features that allow arbitrary code from being executed by Node (e.g. the "-e" command line option and the REPL) when the system is enforcing code integrity.

Signing files

Signatures are stored in catalog file, .cat. This catalog file can be thought of as a collection of filenames and their associated hashes. A Node application author can generate a catalog using the Powershell command New-FileCatalog documentation.

PS> New-FileCatalog -CatalogFilePath ./MyApplicationCatalog.cat -Path ./MyApplicationRelease/

The catalog then can be signed with a certificate trusted by WDAC policy using Powershell with Set-AuthenticodeSignature documentation or signtool.exe

The signed catalog can then be installed on the system MyApplication is being deployed to.

Codecov Report

Attention: Patch coverage is 90.24390% with 8 lines in your changes missing coverage. Please review.

Project coverage is 87.31%. Comparing base (880c446) to head (a7fef80).
Report is 67 commits behind head on main.

Files	Patch %	Lines
lib/internal/main/eval_string.js	72.72%	3 Missing ⚠️
src/node_code_integrity.cc	86.36%	3 Missing ⚠️
lib/code_integrity.js	92.85%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54364      +/-   ##
==========================================
+ Coverage   87.09%   87.31%   +0.22%     
==========================================
  Files         648      650       +2     
  Lines      182216   182441     +225     
  Branches    34965    34997      +32     
==========================================
+ Hits       158704   159305     +601     
+ Misses      16785    16398     -387     
- Partials     6727     6738      +11

Files	Coverage Δ
lib/internal/errors.js	`94.45% <100.00%> (+0.01%)`	⬆️
lib/internal/modules/cjs/loader.js	`92.99% <100.00%> (+0.06%)`	⬆️
src/node_binding.cc	`83.66% <ø> (ø)`
src/node_external_reference.h	`100.00% <ø> (ø)`
lib/code_integrity.js	`92.85% <92.85%> (ø)`
lib/internal/main/eval_string.js	`70.42% <72.72%> (+0.42%)`	⬆️
src/node_code_integrity.cc	`86.36% <86.36%> (ø)`

... and 71 files with indirect coverage changes

RafaelGSS

We've discussed on the security-wg and the next steps are:

Creating documentation
Improving the test coverage

Then we can go the a proper code review. Meanwhile, I'm tagging @nodejs/tsc for awareness.

This is a Windows specific feature that will enabled by a node key in Windows system dictionary. So, it shouldn't be considered a semver-major or a specific Node.js API.

Currently, Windows should be the only environment that makes those checks available via API. We are still evaluating the status of OSX and Linux support for that. However, it's fair to assume this PR will only implement such guarantees for Windows users.

I recommend watching our meeting as we've discussed the usage and how this is turned-on in Windows systems (https://www.youtube.com/watch?v=w4zzH-otKNI)

l0kod · 2024-09-11T14:24:05Z

FYI, we are working on a similar feature for Linux. The goal is the same but the implementation and API are different. Linux already provides access control systems and this new feature (previously open+O_MAYEXEC, now execveat+AT_CHECK) is the missing piece to control script execution the same way other kind of execution can already be controlled. This article gives a good overview: https://lwn.net/Articles/982085/

I plan to send a new patch series in a few weeks. Please let me know what you think.

l0kod · 2024-09-11T15:33:16Z

Cc @tiran, @zooba

rdw-msft · 2024-10-29T17:30:55Z

Hey @RafaelGSS , I've added more documentation about code integrity mode. Please let me know if it looks adequate, or if you'd like me to explain more.

I also added doc/api/wdac-manifest.xml. This is the manifest we spoke about in the last meeting I attended. It declares the WDAC Application settings that NodeJS will query for. It should be hosted in an accessible place so when system admins author their Windows Code Integrity policy they know what settings are available to them, and it also adds some type-checking for the policy writer.

I'll plan on attending the next security WG meeting to discuss in more detail if needed. Thanks

jasnell · 2024-11-05T23:28:34Z

doc/api/wdac-manifest.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="utf-8"?>


A comment in this document explaining what this is and why it is here would be helpful.

jasnell · 2024-11-05T23:28:41Z

doc/api/wdac-manifest.xml

+<AppManifest Id="NodeJS" xmlns="urn:schemas-microsoft-com:windows-defender-application-control">
+  <SettingDefinition Name="EnforceCodeIntegrity" Type="Boolean" IgnoreAuditPolicies="false"/>
+  <SettingDefinition Name="DisableInterpretiveMode" Type="Boolean" IgnoreAuditPolicies="false"/>
+</AppManifest>


Suggested change

</AppManifest>

</AppManifest>

jasnell · 2024-11-05T23:29:51Z

lib/internal/code_integrity.js

@@ -0,0 +1,41 @@
+'use strict';


A comment here explaining a bit about what this is would be helpful. Also, if this feature is limited to windows, comments indicating so should be added as well as possible asserts to ensure it is only running on windows.

I think this file should throw if required by a non-windows environment.

jasnell · 2024-11-05T23:33:01Z

src/node_code_integrity.cc

+static void IsFileTrustedBySystemCodeIntegrityPolicy(
+  const FunctionCallbackInfo<Value>& args) {
+    args.GetReturnValue().Set(true);
+}


These could probably just be implemented in JavaScript so avoid the extraneous JS->C++ boundary call.

jasnell · 2024-11-05T23:33:34Z

src/node_code_integrity.h

+
+#include <Windows.h>
+
+// {0xb5367df1,0xcbac,0x11cf,{0x95,0xca,0x00,0x80,0x5f,0x48,0xa1,0x92}}


It's not clear what this comment is.

jasnell · 2024-11-05T23:34:20Z

src/node_code_integrity.h

+#define WLDP_HOST_OTHER \
+    {0x626cbec3, 0xe1fa, 0x4227, \
+    {0x98, 0x0, 0xed, 0x21, 0x2, 0x74, 0xcf, 0x7c}};
+


A comment with a link to the relevant win32 API docs would be helpful for reviewers and future contributors.

jasnell · 2024-11-05T23:35:47Z

src/node_code_integrity.h

+#ifndef SRC_NODE_CODE_INTEGRITY_H_
+#define SRC_NODE_CODE_INTEGRITY_H_
+
+#ifdef _WIN32


Given that this is Windows only, I would generally prefer that this entire internal binding only be compiled on windows, with the non-functional stubs implemented solely in javascript on the other platforms.

jasnell · 2024-11-05T23:36:27Z

test/parallel/test-code-integrity.js

+'use strict';
+
+// Flags: --expose-internals


Nit:

Suggested change

'use strict';

// Flags: --expose-internals

// Flags: --expose-internals

'use strict';

RafaelGSS

I’m concerned about synchronous calls blocking the event loop and ensuring compatibility with the Windows API across all Node.js versions supported on Windows

doc/api/code_integrity.md

RafaelGSS · 2024-11-08T17:50:34Z

doc/api/code_integrity.md

+## Code Integrity on Linux
+
+Code integrity on Linux is not yet implemented. Plans for implementation will
+be made once the necessary APIs on Linux have been upstreamed.


We might want to link to the original issue

RafaelGSS · 2024-11-08T17:50:54Z

doc/api/errors.md

@@ -789,6 +789,18 @@ changes:
 There was an attempt to use a `MessagePort` instance in a closed
 state, usually after `.close()` has been called.

+<a id="ERR_CODE_INTEGRITY_BLOCKED"></a>
+


Same stability index applies. Stability: 1.1

RafaelGSS · 2024-11-08T17:54:50Z

lib/internal/modules/cjs/loader.js

@@ -1669,6 +1672,11 @@ Module._extensions['.js'] = function(module, filename) {
  // If already analyzed the source, then it will be cached.
  const content = getMaybeCachedSource(module, filename);

+  const isAllowedToExecute = ci.isAllowedToExecuteFile(filename);


This might be better placed inside Module._load (search for permission.isEnabled() on this file).

We shouldn't even call this function for non-windows environments.

RafaelGSS · 2024-11-08T17:55:39Z

src/node_code_integrity.cc

+
+namespace codeintegrity {
+
+#ifdef _WIN32


Does it work on all supported Windows environments?

WldpCanExecuteFile, WldpQueryApplicationSettingBoolean methods are implemented on Windows Server and Windows Client.

Is there a list of NodeJS supported environments I can check against?

https://github.com/nodejs/node/blob/main/BUILDING.md#platform-list

What is the minimum version of Windows supported by Node?

Wldp.dll was added in Windows 8, however the APIs we're using to enforce file signatures were added in Windows 11 Build 22621

The APIs we're using:
WldpCanExecuteFile, checks if the file meets code integrity policy (e.g. hash is unchanged), is available in Windows 11 Build 22621+
WldpGetApplicationSettingBoolean, queries the security policy to see if a configuration setting is set, this is the flag we use to determine if we should be enforcing code integrity with calls to WldpCanExecuteFile, is also in Win11.
WldpQuerySecurityPolicy, is the fallback method we use if WldpGetApplicationSettingBoolean is not available. This allows backward compatibility to early Windows 10 builds.

We can assume that if we can't get a handle on Wldp.dll, the system is too old to support code integrity, so execution can just continue.

If we're on a system with WldpQuerySecurityPolicy, and the Node.js EnforceCodeIntegrity setting is enabled, but we can't load WldpCanExecuteFile, we should probably halt execution. I can add this behavior in the next iteration if it seems reasonable.

cc: @nodejs/build @nodejs/platform-windows

RafaelGSS · 2024-11-08T17:57:32Z

src/node_code_integrity.cc

+  }
+
+  HMODULE wldp_module = LoadLibraryExA(
+      "wldp.dll",


Is it a default dll on Windows? Can't it be subject of hijack?

I remember that we faced an vulnerability in the past where Node.js was reading the openssl.conf from working_dir instead of absolute openssl path leading to some vulnerabilities. Could you double check if that's the case?

This is a default DLL, the OS grabs a handle on this DLL early in the boot process to prevent tampering.

We can also assume that if this DLL doesn't exist on a system, it is too old to support code integrity features.

RafaelGSS · 2024-11-08T17:58:26Z

src/node_code_integrity.cc

+
+  const GUID wldp_host_other = WLDP_HOST_OTHER;
+  WLDP_EXECUTION_POLICY result;
+  HRESULT hr = WldpCanExecuteFile(


Are those calls synchronous right? It seems to block the event loop on every check making the system useless from a nodejs perspective

Yes. These calls are synchronous, and there are no async versions. Is there a way to address this without the platform adding an async version?

This call will only be made on systems that have opted into code integrity enforcement. I can reach out to the Windows platform team and see if there's opportunity to add an async version if this is an issue.

At the moment, is it acceptable to say there will be performance hits when using this feature?

I think it will make this feature hard to use considering Node.js constraints, we can measure but I suspect it will make things quite slow

Add calls to Windows Defender Application Control to enforce integrity of .js, .json, .node files.

anonrig · 2024-11-18T21:05:32Z

lib/internal/code_integrity.js

@@ -0,0 +1,41 @@
+'use strict';


I think this file should throw if required by a non-windows environment.

anonrig · 2024-11-18T21:06:04Z

lib/internal/modules/cjs/loader.js

@@ -1669,6 +1672,11 @@ Module._extensions['.js'] = function(module, filename) {
  // If already analyzed the source, then it will be cached.
  const content = getMaybeCachedSource(module, filename);

+  const isAllowedToExecute = ci.isAllowedToExecuteFile(filename);


We shouldn't even call this function for non-windows environments.

anonrig · 2024-11-18T21:06:50Z

src/node_code_integrity.cc

+namespace codeintegrity {
+
+#ifdef _WIN32
+static bool isWldpInitialized = false;


We should avoid using global variables, and use a snapshottable API.

anonrig · 2024-11-18T21:07:33Z

src/node_code_integrity.cc

+using v8::Object;
+using v8::Value;
+
+namespace codeintegrity {


Suggested change

namespace codeintegrity {

namespace code_integrity {

anonrig · 2024-11-18T21:08:08Z

src/node_code_integrity.cc

+}  // namespace codeintegrity
+}  // namespace node
+
+NODE_BINDING_CONTEXT_AWARE_INTERNAL(code_integrity,


Why do we even create this binding context for non-windows environments?

anonrig · 2024-11-18T21:08:36Z

src/node_code_integrity.cc

+static void IsInterpretiveModeDisabled(
+  const FunctionCallbackInfo<Value>& args) {
+    args.GetReturnValue().Set(false);
+}


There is no need for this function, since it only returns a constant boolean value.

Suggested change

static void IsInterpretiveModeDisabled(

const FunctionCallbackInfo<Value>& args) {

args.GetReturnValue().Set(false);

}

anonrig · 2024-11-18T21:09:23Z

tsconfig.json

@@ -34,6 +34,7 @@
      "buffer": ["./lib/buffer.js"],
      "child_process": ["./lib/child_process.js"],
      "cluster": ["./lib/cluster.js"],
+      "codeintegrity": ["./lib/codeintegrity.js"],


we should also update the necessary files in typings folder for the internal APIs.

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 14, 2024

RafaelGSS added semver-minor PRs that contain new features and should be released in the next minor version. security-wg-agenda labels Aug 14, 2024

mcollina reviewed Aug 14, 2024

View reviewed changes

rdw-msft mentioned this pull request Aug 21, 2024

Discuss WDAC (code integrity on Windows) nodejs/security-wg#1364

Open

mhdawson mentioned this pull request Aug 26, 2024

Node.js Security team Meeting 2024-08-29 nodejs/security-wg#1365

Closed

RafaelGSS requested changes Aug 29, 2024

View reviewed changes

mhdawson mentioned this pull request Sep 9, 2024

Node.js Security team Meeting 2024-09-12 nodejs/security-wg#1375

Closed

mhdawson mentioned this pull request Sep 23, 2024

Node.js Security team Meeting 2024-09-26 nodejs/security-wg#1382

Closed

l0kod mentioned this pull request Sep 27, 2024

Script execution control on Linux nodejs/security-wg#1388

Open

mhdawson mentioned this pull request Oct 8, 2024

Node.js Security team Meeting 2024-10-10 nodejs/security-wg#1389

Closed

rdw-msft force-pushed the wldp-integration branch from 7cfafd5 to e8e5e7a Compare October 10, 2024 17:16

rdw-msft force-pushed the wldp-integration branch from e8e5e7a to 18b9645 Compare October 29, 2024 17:26

mhdawson mentioned this pull request Nov 4, 2024

Node.js Security team Meeting 2024-11-07 nodejs/security-wg#1398

Closed

jasnell reviewed Nov 5, 2024

View reviewed changes

RafaelGSS requested changes Nov 8, 2024

View reviewed changes

rdw-msft added 11 commits November 18, 2024 10:43

src: add WDAC integration (Windows)

cd2ff28

Add calls to Windows Defender Application Control to enforce integrity of .js, .json, .node files.

appease linter

b5bd33f

use LF instead of CRLF

9935443

fix expected exception

ac351a2

move code_integrity to internal module, fix formatting

164edc5

add DisableInteractiveMode code integrity setting

156f2c9

add more code integrity docs

9942e9d

clarify docs, add active development status

851a07a

move stub calls for Linux/Darwin into js

2fb34de

fix expose internals flag location

e1e41e7

remove trailing whitespace

8765dc6

rdw-msft force-pushed the wldp-integration branch from 18b9645 to 8765dc6 Compare November 18, 2024 18:57

anonrig requested changes Nov 18, 2024

View reviewed changes

		@@ -198,6 +199,8 @@ const onRequire = getLazy(() => tracingChannel('module.require'));

		const relativeResolveCache = { __proto__: null };

		const ci = require('code_integrity');


		#include <Windows.h>

		// {0xb5367df1,0xcbac,0x11cf,{0x95,0xca,0x00,0x80,0x5f,0x48,0xa1,0x92}}

src: add WDAC integration (Windows) #54364

Are you sure you want to change the base?

src: add WDAC integration (Windows) #54364

Conversation

rdw-msft commented Aug 14, 2024

Motivation

Windows Defender Application Control (WDAC)

High-level implementation

Signing files

Other Questions

nodejs-github-bot commented Aug 14, 2024

anonrig commented Aug 14, 2024

rdw-msft commented Aug 14, 2024

targos commented Aug 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 19, 2024

Codecov Report

RafaelGSS left a comment

Choose a reason for hiding this comment

l0kod commented Sep 11, 2024

l0kod commented Sep 11, 2024

rdw-msft commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafaelGSS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafaelGSS Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafaelGSS Nov 11, 2024 •

edited

Loading