Skip to content

Commit

Permalink
crypto: change default check(Host|Email) behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
jkleinsc committed Apr 28, 2022
1 parent 6445fa0 commit 1f1eb23
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/node/.patches
Expand Up @@ -42,3 +42,4 @@ process_fix_hang_after_note_exit_3521.patch
feat_add_uv_loop_interrupt_on_io_change_option_to_uv_loop_configure.patch
fix_preserve_proper_method_names_as-is_in_error_stack.patch
macos_avoid_posix_spawnp_cwd_bug_3597.patch
crypto_change_default_check_host_email_behavior.patch
99 changes: 99 additions & 0 deletions patches/node/crypto_change_default_check_host_email_behavior.patch
@@ -0,0 +1,99 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Wed, 19 Jan 2022 19:05:53 +0000
Subject: crypto: change default check(Host|Email) behavior

This changes the default behavior of the X509Certificate functions
checkHost and checkEmail to match the default behavior of OpenSSL's
X509_check_host and X509_check_email functions, respectively, which
is also what RFC 2818 mandates for HTTPS.

Refs: https://github.com/nodejs/node/pull/36804
Refs: https://github.com/nodejs/node/pull/41569

PR-URL: https://github.com/nodejs/node/pull/41600
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
(cherry picked from commit 18365d8ee6a5fdeb8b46d1a0ec9b954d61ebca7e)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index dff4142fd4a5e29c744b2e849b66b65088d4002b..175d89f54acb991d8f36230aacbfd0b46168a5dd 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -2511,14 +2511,14 @@ changes:

Checks whether the certificate matches the given email address.

+If the `'subject'` option is undefined or set to `'default'`, the certificate
+subject is only considered if the subject alternative name extension either does
+not exist or does not contain any email addresses.
+
If the `'subject'` option is set to `'always'` and if the subject alternative
name extension either does not exist or does not contain a matching email
address, the certificate subject is considered.

-If the `'subject'` option is set to `'default'`, the certificate subject is only
-considered if the subject alternative name extension either does not exist or
-does not contain any email addresses.
-
If the `'subject'` option is set to `'never'`, the certificate subject is never
considered, even if the certificate contains no subject alternative names.

@@ -2535,7 +2535,7 @@ changes:
* `name` {string}
* `options` {Object}
* `subject` {string} `'default'`, `'always'`, or `'never'`.
- **Default:** `'always'`.
+ **Default:** `'default'`.
* `wildcards` {boolean} **Default:** `true`.
* `partialWildcards` {boolean} **Default:** `true`.
* `multiLabelWildcards` {boolean} **Default:** `false`.
@@ -2551,15 +2551,15 @@ or it might contain wildcards (e.g., `*.example.com`). Because host name
comparisons are case-insensitive, the returned subject name might also differ
from the given `name` in capitalization.

+If the `'subject'` option is undefined or set to `'default'`, the certificate
+subject is only considered if the subject alternative name extension either does
+not exist or does not contain any DNS names. This behavior is consistent with
+[RFC 2818][] ("HTTP Over TLS").
+
If the `'subject'` option is set to `'always'` and if the subject alternative
name extension either does not exist or does not contain a matching DNS name,
the certificate subject is considered.

-If the `'subject'` option is set to `'default'`, the certificate subject is only
-considered if the subject alternative name extension either does not exist or
-does not contain any DNS names. This behavior is consistent with [RFC 2818][]
-("HTTP Over TLS").
-
If the `'subject'` option is set to `'never'`, the certificate subject is never
considered, even if the certificate contains no subject alternative names.

diff --git a/lib/internal/crypto/x509.js b/lib/internal/crypto/x509.js
index cd20f6868da72ccfa65fabdcb572b3e3f2e8a8a5..e7098d17da6aacf4912fcca001702e3f43689548 100644
--- a/lib/internal/crypto/x509.js
+++ b/lib/internal/crypto/x509.js
@@ -65,8 +65,7 @@ function isX509Certificate(value) {
function getFlags(options = {}) {
validateObject(options, 'options');
const {
- // TODO(tniessen): change the default to 'default'
- subject = 'always', // Can be 'default', 'always', or 'never'
+ subject = 'default', // Can be 'default', 'always', or 'never'
wildcards = true,
partialWildcards = true,
multiLabelWildcards = false,
diff --git a/test/parallel/test-x509-escaping.js b/test/parallel/test-x509-escaping.js
index 99418e4c0bf21c26d5ba0ad9d617419abc625593..4e88d324cd3d5abe6b056d32ea40969531d31639 100644
--- a/test/parallel/test-x509-escaping.js
+++ b/test/parallel/test-x509-escaping.js
@@ -425,7 +425,7 @@ const { hasOpenSSL3 } = common;
assert.strictEqual(certX509.subjectAltName, 'DNS:evil.example.com');

// The newer X509Certificate API allows customizing this behavior:
- assert.strictEqual(certX509.checkHost(servername), servername);
+ assert.strictEqual(certX509.checkHost(servername), undefined);
assert.strictEqual(certX509.checkHost(servername, { subject: 'default' }),
undefined);
assert.strictEqual(certX509.checkHost(servername, { subject: 'always' }),

0 comments on commit 1f1eb23

Please sign in to comment.