fixup! address comments, use node_errors, add pskIdentityHint error test

doc/api/errors.md

@@ -1857,6 +1857,11 @@ vector for denial-of-service attacks.
 An attempt was made to issue Server Name Indication from a TLS server-side
 socket, which is only valid from a client.
 
+<a id="ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED"></a>
+### ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED
+
+Failed to set PSK identity hint. Hint may be too long.
+
 <a id="ERR_TRACE_EVENTS_CATEGORY_REQUIRED"></a>
 ### ERR_TRACE_EVENTS_CATEGORY_REQUIRED
 

doc/api/tls.md

@@ -145,11 +145,11 @@ ciphers can be retrieved via `openssl ciphers -v 'PSK'`. All TLS 1.3
 ciphers are eligible for PSK but currently only those that use SHA256 digest are
 supported they can be retrieved via `openssl ciphers -v -s -tls1_3 -psk`.
 
-According to the [RFC 4279][] PSK identities up to 128 bytes in length, and
+According to the [RFC 4279][], PSK identities up to 128 bytes in length and
 PSKs up to 64 bytes in length must be supported. As of OpenSSL 1.1.0
 maximum identity size is 128 bytes, and maximum PSK length is 256 bytes.
 
-Current implementation doesn't support asynchronous PSK callbacks due to the
+The current implementation doesn't support asynchronous PSK callbacks due to the
 limitations of the underlying OpenSSL API.
 
 ### Client-initiated renegotiation attack mitigation
@@ -1659,14 +1659,15 @@ changes:
     If the return value is `null` the negotiation process will stop and an
     "unknown_psk_identity" alert message will be sent to the other party.
     If the server wishes to hide the fact that the PSK identity was not known,
-    callback must provide some random data as `psk` to make the connection fail
-    with "decrypt_error" before negotiation is finished.
+    the callback must provide some random data as `psk` to make the connection
+    fail with "decrypt_error" before negotiation is finished.
     PSK ciphers are disabled by default, and using TLS-PSK thus
     requires explicitly specifying a cipher suite with the `ciphers` option.
     More information can be found in the [RFC 4279][].
   * `pskIdentityHint` {string} optional hint to send to a client to help
     with selecting the identity during TLS-PSK negotiation. Will be ignored
-    in TLS 1.3.
+    in TLS 1.3. Upon failing to set pskIdentityHint `'tlsClientError'` will be
+    emitted with `'ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED'` code.
   * ...: Any [`tls.createSecureContext()`][] option can be provided. For
     servers, the identity options (`pfx`, `key`/`cert` or `pskCallback`)
     are usually required.

src/env.h

@@ -165,7 +165,7 @@ constexpr size_t kFsStatsBufferLength =
   V(no_message_symbol, "no_message_symbol")                                    \
   V(oninit_symbol, "oninit")                                                   \
   V(owner_symbol, "owner")                                                     \
-  V(onpskexchange_symbol, "onpskexchange")
+  V(onpskexchange_symbol, "onpskexchange")                                     \
 
 // Strings are per-isolate primitives but Environment proxies them
 // for the sake of convenience.  Strings should be ASCII-only.
@@ -327,7 +327,6 @@ constexpr size_t kFsStatsBufferLength =
   V(priority_string, "priority")                                               \
   V(process_string, "process")                                                 \
   V(promise_string, "promise")                                                 \
-  V(psk_identity_hint_error, "Failed to set PSK identity hint")                \
   V(psk_string, "psk")                                                         \
   V(pubkey_string, "pubkey")                                                   \
   V(query_string, "query")                                                     \

src/node_errors.h

@@ -58,6 +58,7 @@ void PrintErrorString(const char* format, ...);
   V(ERR_STRING_TOO_LONG, Error)                                              \
   V(ERR_TLS_INVALID_PROTOCOL_METHOD, TypeError)                              \
   V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, TypeError)              \
+  V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, Error)                              \
 
 #define V(code, type)                                                         \
   inline v8::Local<v8::Value> code(v8::Isolate* isolate,                      \
@@ -101,6 +102,7 @@ void PrintErrorString(const char* format, ...);
     "Script execution was interrupted by `SIGINT`")                          \
   V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER,                         \
     "Cannot serialize externalized SharedArrayBuffer")                       \
+  V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, "Failed to set PSK identity hint")  \
 
 #define V(code, message)                                                     \
   inline v8::Local<v8::Value> code(v8::Isolate* isolate) {                   \

src/tls_wrap.cc

@@ -28,6 +28,7 @@
 #include "node_crypto_bio.h"  // NodeBIO
 // ClientHelloParser
 #include "node_crypto_clienthello-inl.h"
+#include "node_errors.h"
 #include "stream_base-inl.h"
 #include "util-inl.h"
 
@@ -1087,12 +1088,13 @@ void TLSWrap::SetPskIdentityHint(const FunctionCallbackInfo<Value>& args) {
   CHECK_NOT_NULL(p->ssl_);
 
   Environment* env = p->env();
+  Isolate* isolate = env->isolate();
 
   CHECK(args[0]->IsString());
-  node::Utf8Value hint(env->isolate(), args[0].As<String>());
+  node::Utf8Value hint(isolate, args[0].As<String>());
 
   if (!SSL_use_psk_identity_hint(p->ssl_.get(), *hint)) {
-    Local<Value> err = Exception::Error(env->psk_identity_hint_error());
+    Local<Value> err = node::ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED(isolate);
     p->MakeCallback(env->onerror_string(), 1, &err);
   }
 }
@@ -1123,8 +1125,7 @@ unsigned int TLSWrap::PskServerCallback(SSL* s,
   if (!maybe_identity_str.ToLocal(&identity_str)) return 0;
 
   // Make sure there are no utf8 replacement symbols.
-  const v8::String::Utf8Value& identity_utf8 =
-      v8::String::Utf8Value(isolate, identity_str);
+  v8::String::Utf8Value identity_utf8(isolate, identity_str);
   if (strcmp(*identity_utf8, identity) != 0) return 0;
 
   Local<Value> argv[] = {identity_str,

test/parallel/test-tls-psk-errors.js

@@ -0,0 +1,32 @@
+'use strict';
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const tls = require('tls');
+
+{
+  // Check tlsClientError on invalid pskIdentityHint.
+
+  const server = tls.createServer({
+    ciphers: 'PSK+HIGH',
+    pskCallback: () => {},
+    pskIdentityHint: 'a'.repeat(512), // Too long identity hint.
+  });
+  server.on('tlsClientError', (err) => {
+    assert.ok(err instanceof Error);
+    assert.strictEqual(err.code, 'ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED');
+    server.close();
+  });
+  server.listen(0, () => {
+    const client = tls.connect({
+      port: server.address().port,
+      ciphers: 'PSK+HIGH',
+      checkServerIdentity: () => {},
+      pskCallback: () => {},
+    }, () => {});
+    client.on('error', common.expectsError({ code: 'ECONNRESET' }));
+  });
+}