Skip to content

Commit

Permalink
Revert "crypto: add extensions property to X509Certificate"
Browse files Browse the repository at this point in the history
This reverts commit 7dd4190.
  • Loading branch information
mertcanaltin committed Jul 15, 2023
1 parent 7dd4190 commit 11d9028
Show file tree
Hide file tree
Showing 17 changed files with 141 additions and 61 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,8 @@ For information about the governance of the Node.js project, see
**Anatoli Papirovski** <<[email protected]>> (he/him)
* [AshCripps](https://github.com/AshCripps) -
**Ash Cripps** <<[email protected]>>
* [atlowChemi](https://github.com/atlowChemi) -
**Chemi Atlow** <<[email protected]>> (he/him)
* [Ayase-252](https://github.com/Ayase-252) -
**Qingyu Deng** <<[email protected]>>
* [bengl](https://github.com/bengl) -
Expand Down
4 changes: 2 additions & 2 deletions deps/nghttp2/lib/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,8 @@ LIBNGHTTP3_LIBS = @LIBNGHTTP3_LIBS@
LIBNGTCP2_CFLAGS = @LIBNGTCP2_CFLAGS@
LIBNGTCP2_CRYPTO_BORINGSSL_CFLAGS = @LIBNGTCP2_CRYPTO_BORINGSSL_CFLAGS@
LIBNGTCP2_CRYPTO_BORINGSSL_LIBS = @LIBNGTCP2_CRYPTO_BORINGSSL_LIBS@
LIBNGTCP2_CRYPTO_OPENSSL_CFLAGS = @LIBNGTCP2_CRYPTO_OPENSSL_CFLAGS@
LIBNGTCP2_CRYPTO_OPENSSL_LIBS = @LIBNGTCP2_CRYPTO_OPENSSL_LIBS@
LIBNGTCP2_CRYPTO_QUICTLS_CFLAGS = @LIBNGTCP2_CRYPTO_QUICTLS_CFLAGS@
LIBNGTCP2_CRYPTO_QUICTLS_LIBS = @LIBNGTCP2_CRYPTO_QUICTLS_LIBS@
LIBNGTCP2_LIBS = @LIBNGTCP2_LIBS@
LIBOBJS = @LIBOBJS@
LIBS = @LIBS@
Expand Down
4 changes: 2 additions & 2 deletions deps/nghttp2/lib/includes/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ LIBNGHTTP3_LIBS = @LIBNGHTTP3_LIBS@
LIBNGTCP2_CFLAGS = @LIBNGTCP2_CFLAGS@
LIBNGTCP2_CRYPTO_BORINGSSL_CFLAGS = @LIBNGTCP2_CRYPTO_BORINGSSL_CFLAGS@
LIBNGTCP2_CRYPTO_BORINGSSL_LIBS = @LIBNGTCP2_CRYPTO_BORINGSSL_LIBS@
LIBNGTCP2_CRYPTO_OPENSSL_CFLAGS = @LIBNGTCP2_CRYPTO_OPENSSL_CFLAGS@
LIBNGTCP2_CRYPTO_OPENSSL_LIBS = @LIBNGTCP2_CRYPTO_OPENSSL_LIBS@
LIBNGTCP2_CRYPTO_QUICTLS_CFLAGS = @LIBNGTCP2_CRYPTO_QUICTLS_CFLAGS@
LIBNGTCP2_CRYPTO_QUICTLS_LIBS = @LIBNGTCP2_CRYPTO_QUICTLS_LIBS@
LIBNGTCP2_LIBS = @LIBNGTCP2_LIBS@
LIBOBJS = @LIBOBJS@
LIBS = @LIBS@
Expand Down
4 changes: 2 additions & 2 deletions deps/nghttp2/lib/includes/nghttp2/nghttp2ver.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
* @macro
* Version number of the nghttp2 library release
*/
#define NGHTTP2_VERSION "1.53.0"
#define NGHTTP2_VERSION "1.55.0"

/**
* @macro
* Numerical representation of the version number of the nghttp2 library
* release. This is a 24 bit number with 8 bits for major number, 8 bits
* for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
*/
#define NGHTTP2_VERSION_NUM 0x013500
#define NGHTTP2_VERSION_NUM 0x013700

#endif /* NGHTTP2VER_H */
6 changes: 6 additions & 0 deletions doc/contributing/collaborator-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,12 @@ For pull requests from first-time contributors, be
[welcoming](#welcoming-first-time-contributors). Also, verify that their git
settings are to their liking.

If a pull request contains more than one commit, it can be landed either by
squashing into one commit or by rebasing all the commits, or a mix of the two.
Generally, a collaborator should land a pull request by squashing. If a pull
request has more than one self-contained subsystem commits, a collaborator
may land it as several commits.

All commits should be self-contained, meaning every commit should pass all
tests. This makes it much easier when bisecting to find a breaking change.

Expand Down
6 changes: 3 additions & 3 deletions doc/contributing/maintaining/maintaining-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ This a list of all the dependencies:
* [libuv 1.46.0][]
* [llhttp 8.1.0][]
* [minimatch 9.0.3][]
* [nghttp2 1.53.0][]
* [nghttp2 1.55.0][]
* [nghttp3 0.7.0][]
* [ngtcp2 0.8.1][]
* [npm 9.6.7][]
Expand Down Expand Up @@ -223,7 +223,7 @@ See [maintaining-http][] for more informations.
The [minimatch](https://github.com/isaacs/minimatch) dependency is a
minimal matching utility.

### nghttp2 1.53.0
### nghttp2 1.55.0

The [nghttp2](https://github.com/nghttp2/nghttp2) dependency is a C library
implementing HTTP/2 protocol.
Expand Down Expand Up @@ -338,7 +338,7 @@ performance improvements not currently available in standard zlib.
[maintaining-openssl]: ./maintaining-openssl.md
[maintaining-web-assembly]: ./maintaining-web-assembly.md
[minimatch 9.0.3]: #minimatch-903
[nghttp2 1.53.0]: #nghttp2-1530
[nghttp2 1.55.0]: #nghttp2-1550
[nghttp3 0.7.0]: #nghttp3-070
[ngtcp2 0.8.1]: #ngtcp2-081
[npm 9.6.7]: #npm-967
Expand Down
7 changes: 5 additions & 2 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function fork(modulePath, args = [], options) {
if (options != null) {
validateObject(options, 'options');
}
options = { ...options, shell: false };
options = { __proto__: null, ...options, shell: false };
options.execPath = options.execPath || process.execPath;
validateArgumentNullCheck(options.execPath, 'options.execPath');

Expand Down Expand Up @@ -196,7 +196,7 @@ function normalizeExecArgs(command, options, callback) {
}

// Make a shallow copy so we don't clobber the user's options object.
options = { ...options };
options = { __proto__: null, ...options };
options.shell = typeof options.shell === 'string' ? options.shell : true;

return {
Expand Down Expand Up @@ -329,6 +329,7 @@ function execFile(file, args, options, callback) {
({ file, args, options, callback } = normalizeExecFileArgs(file, args, options, callback));

options = {
__proto__: null,
encoding: 'utf8',
timeout: 0,
maxBuffer: MAX_BUFFER,
Expand Down Expand Up @@ -703,6 +704,7 @@ function normalizeSpawnArguments(file, args, options) {

return {
// Make a shallow copy so we don't clobber the user's options object.
__proto__: null,
...options,
args,
cwd,
Expand Down Expand Up @@ -828,6 +830,7 @@ function spawn(file, args, options) {
*/
function spawnSync(file, args, options) {
options = {
__proto__: null,
maxBuffer: MAX_BUFFER,
...normalizeSpawnArguments(file, args, options),
};
Expand Down
10 changes: 0 additions & 10 deletions lib/internal/crypto/x509.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ class X509Certificate {
fingerprint512: this.fingerprint512,
keyUsage: this.keyUsage,
serialNumber: this.serialNumber,
extensions: this.extensions,
}, opts)}`;
}

Expand Down Expand Up @@ -266,15 +265,6 @@ class X509Certificate {
return value;
}

get extensions() {
let value = this[kInternalState].get('extensions');
if (value === undefined) {
value = this[kHandle].extensions();
this[kInternalState].set('extensions', value);
}
return value;
}

get raw() {
let value = this[kInternalState].get('raw');
if (value === undefined) {
Expand Down
8 changes: 8 additions & 0 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ const {
SideEffectFreeRegExpPrototypeSymbolReplace,
} = require('internal/util');

const {
markTransferMode,
} = require('internal/worker/js_transferable');

const {
codes: {
ERR_ARG_NOT_ITERABLE,
Expand Down Expand Up @@ -326,6 +330,8 @@ class URLSearchParams {
// Default parameter is necessary to keep URLSearchParams.length === 0 in
// accordance with Web IDL spec.
constructor(init = undefined) {
markTransferMode(this, false, false);

if (init == null) {
// Do nothing
} else if (typeof init === 'object' || typeof init === 'function') {
Expand Down Expand Up @@ -761,6 +767,8 @@ class URL {
#searchParams;

constructor(input, base = undefined) {
markTransferMode(this, false, false);

if (arguments.length === 0) {
throw new ERR_MISSING_ARGS('url');
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ function getFunctionBase(value, constructor, tag) {
function identicalSequenceRange(a, b) {
for (let i = 0; i < a.length - 3; i++) {
// Find the first entry of b that matches the current entry of a.
const pos = b.indexOf(a[i]);
const pos = ArrayPrototypeIndexOf(b, a[i]);
if (pos !== -1) {
const rest = b.length - pos;
if (rest > 3) {
Expand Down
3 changes: 2 additions & 1 deletion src/debug_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
V(NGTCP2_DEBUG) \
V(SEA) \
V(WASI) \
V(MKSNAPSHOT)
V(MKSNAPSHOT) \
V(PERMISSION_MODEL)

enum class DebugCategory : unsigned int {
#define V(name) name,
Expand Down
47 changes: 47 additions & 0 deletions src/permission/fs_permission.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "fs_permission.h"
#include "base_object-inl.h"
#include "debug_utils-inl.h"
#include "util.h"
#include "v8.h"

Expand Down Expand Up @@ -72,6 +73,46 @@ namespace node {

namespace permission {

void PrintTree(const FSPermission::RadixTree::Node* node, size_t spaces = 0) {
std::string whitespace(spaces, ' ');

if (node == nullptr) {
return;
}
if (node->wildcard_child != nullptr) {
per_process::Debug(DebugCategory::PERMISSION_MODEL,
"%s Wildcard: %s\n",
whitespace,
node->prefix);
} else {
per_process::Debug(DebugCategory::PERMISSION_MODEL,
"%s Prefix: %s\n",
whitespace,
node->prefix);
if (node->children.size()) {
size_t child = 0;
for (const auto& pair : node->children) {
++child;
per_process::Debug(DebugCategory::PERMISSION_MODEL,
"%s Child(%s): %s\n",
whitespace,
child,
std::string(1, pair.first));
PrintTree(pair.second, spaces + 2);
}
per_process::Debug(DebugCategory::PERMISSION_MODEL,
"%s End of tree - child(%s)\n",
whitespace,
child);
} else {
per_process::Debug(DebugCategory::PERMISSION_MODEL,
"%s End of tree: %s\n",
whitespace,
node->prefix);
}
}
}

// allow = '*'
// allow = '/tmp/,/home/example.js'
void FSPermission::Apply(const std::string& allow, PermissionScope scope) {
Expand Down Expand Up @@ -175,6 +216,12 @@ void FSPermission::RadixTree::Insert(const std::string& path) {
parent_node_prefix_len = i;
}
}

if (UNLIKELY(per_process::enabled_debug_list.enabled(
DebugCategory::PERMISSION_MODEL))) {
per_process::Debug(DebugCategory::PERMISSION_MODEL, "Inserting %s\n", path);
PrintTree(root_node_);
}
}

} // namespace permission
Expand Down
2 changes: 0 additions & 2 deletions src/permission/fs_permission.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ class FSPermission final : public PermissionBase {
void Apply(const std::string& allow, PermissionScope scope) override;
bool is_granted(PermissionScope perm, const std::string_view& param) override;

// For debugging purposes, use the gist function to print the whole tree
// https://gist.github.com/RafaelGSS/5b4f09c559a54f53f9b7c8c030744d19
struct RadixTree {
struct Node {
std::string prefix;
Expand Down
59 changes: 59 additions & 0 deletions test/parallel/test-child-process-prototype-tampering.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import * as common from '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import { EOL } from 'node:os';
import { strictEqual } from 'node:assert';
import cp from 'node:child_process';

// TODO(LiviaMedeiros): test on different platforms
if (!common.isLinux)
common.skip();

const expectedCWD = process.cwd();
const expectedUID = process.getuid();

for (const tamperedCwd of ['', '/tmp', '/not/existing/malicious/path', 42n]) {
Object.prototype.cwd = tamperedCwd;

cp.exec('pwd', common.mustSucceed((out) => {
strictEqual(`${out}`, `${expectedCWD}${EOL}`);
}));
strictEqual(`${cp.execSync('pwd')}`, `${expectedCWD}${EOL}`);
cp.execFile('pwd', common.mustSucceed((out) => {
strictEqual(`${out}`, `${expectedCWD}${EOL}`);
}));
strictEqual(`${cp.execFileSync('pwd')}`, `${expectedCWD}${EOL}`);
cp.spawn('pwd').stdout.on('data', common.mustCall((out) => {
strictEqual(`${out}`, `${expectedCWD}${EOL}`);
}));
strictEqual(`${cp.spawnSync('pwd').stdout}`, `${expectedCWD}${EOL}`);

delete Object.prototype.cwd;
}

for (const tamperedUID of [0, 1, 999, 1000, 0n, 'gwak']) {
Object.prototype.uid = tamperedUID;

cp.exec('id -u', common.mustSucceed((out) => {
strictEqual(`${out}`, `${expectedUID}${EOL}`);
}));
strictEqual(`${cp.execSync('id -u')}`, `${expectedUID}${EOL}`);
cp.execFile('id', ['-u'], common.mustSucceed((out) => {
strictEqual(`${out}`, `${expectedUID}${EOL}`);
}));
strictEqual(`${cp.execFileSync('id', ['-u'])}`, `${expectedUID}${EOL}`);
cp.spawn('id', ['-u']).stdout.on('data', common.mustCall((out) => {
strictEqual(`${out}`, `${expectedUID}${EOL}`);
}));
strictEqual(`${cp.spawnSync('id', ['-u']).stdout}`, `${expectedUID}${EOL}`);

delete Object.prototype.uid;
}

{
Object.prototype.execPath = '/not/existing/malicious/path';

// Does not throw ENOENT
cp.fork(fixtures.path('empty.js'));

delete Object.prototype.execPath;
}
2 changes: 1 addition & 1 deletion test/parallel/test-string-decoder.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ assert.throws(

if (common.enoughTestMem) {
assert.throws(
() => new StringDecoder().write(Buffer.alloc(0x1fffffe8 + 1).fill('a')),
() => new StringDecoder().write(Buffer.alloc((process.arch === 'ia32' ? 0x18ffffe8 : 0x1fffffe8) + 1).fill('a')),
{
code: 'ERR_STRING_TOO_LONG',
}
Expand Down
32 changes: 0 additions & 32 deletions test/parallel/test-x509-escaping.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,35 +502,3 @@ const { hasOpenSSL3 } = common;
}, common.mustCall());
}));
}

// certificateExtensions test
{
const pem = fixtures.readSync('x509-certificate-extensions.pem', 'utf8');

const cert = new X509Certificate(pem);
const expectedExtensions = {
'1.3.6.1.5.5.7.1.1': Buffer.from('test').toString('base64'),
'1.3.6.1.5.5.7.1.2': 'http://example.com/',
};

assert.deepStrictEqual(cert.certificateExtensions, expectedExtensions);

const serverKey = fixtures.readSync('x509-certificate-extensions-key.pem', 'utf8');

const server = tls.createServer({
key: serverKey,
cert: pem,
}, common.mustCall((conn) => {
conn.destroy();
server.close();
})).listen(common.mustCall(() => {
const { port } = server.address();
tls.connect(port, {
ca: pem,
servername: 'example.com',
checkServerIdentity: (peerCert) => {
assert.deepStrictEqual(peerCert.certificateExtensions, expectedExtensions);
},
}, common.mustCall());
}));
}
4 changes: 1 addition & 3 deletions test/wpt/status/url.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"fail": {
"note": "We are faking location with a URL object for the sake of the testharness and it has searchParams.",
"expected": [
"searchParams on location object",
"URL: no structured serialize/deserialize support",
"URLSearchParams: no structured serialize/deserialize support"
"searchParams on location object"
]
}
},
Expand Down

0 comments on commit 11d9028

Please sign in to comment.