Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 36.4k
quic: add proper error codes & messages for QUIC failures#63198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
d6419935804046e72d6ce4537f5c841af8dc7333adc5bd305File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -9,6 +9,7 @@ const { | ||||||||||||||||||||||||||||||
| ArrayPrototypePush, | ||||||||||||||||||||||||||||||
| BigInt, | ||||||||||||||||||||||||||||||
| DataViewPrototypeGetByteLength, | ||||||||||||||||||||||||||||||
| ErrorCaptureStackTrace, | ||||||||||||||||||||||||||||||
| FunctionPrototypeBind, | ||||||||||||||||||||||||||||||
| Number, | ||||||||||||||||||||||||||||||
| ObjectDefineProperties, | ||||||||||||||||||||||||||||||
| @@ -108,13 +109,11 @@ const { | ||||||||||||||||||||||||||||||
| ERR_INVALID_THIS, | ||||||||||||||||||||||||||||||
| ERR_MISSING_ARGS, | ||||||||||||||||||||||||||||||
| ERR_OUT_OF_RANGE, | ||||||||||||||||||||||||||||||
| ERR_QUIC_APPLICATION_ERROR, | ||||||||||||||||||||||||||||||
| ERR_QUIC_CONNECTION_FAILED, | ||||||||||||||||||||||||||||||
| ERR_QUIC_ENDPOINT_CLOSED, | ||||||||||||||||||||||||||||||
| ERR_QUIC_OPEN_STREAM_FAILED, | ||||||||||||||||||||||||||||||
| ERR_QUIC_STREAM_ABORTED, | ||||||||||||||||||||||||||||||
| ERR_QUIC_STREAM_RESET, | ||||||||||||||||||||||||||||||
| ERR_QUIC_TRANSPORT_ERROR, | ||||||||||||||||||||||||||||||
| ERR_QUIC_VERSION_NEGOTIATION_ERROR, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| } = require('internal/errors'); | ||||||||||||||||||||||||||||||
| @@ -738,10 +737,12 @@ setCallbacks({ | ||||||||||||||||||||||||||||||
| * @param {number} errorType | ||||||||||||||||||||||||||||||
| * @param {number} code | ||||||||||||||||||||||||||||||
| * @param {string} [reason] | ||||||||||||||||||||||||||||||
| * @param {string} [errorName] Decoded TLS alert name when `code` is a | ||||||||||||||||||||||||||||||
| * CRYPTO_ERROR; otherwise undefined. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| onSessionClose(errorType, code, reason) { | ||||||||||||||||||||||||||||||
| debug('session close callback', errorType, code, reason); | ||||||||||||||||||||||||||||||
| this[kOwner][kFinishClose](errorType, code, reason); | ||||||||||||||||||||||||||||||
| onSessionClose(errorType, code, reason, errorName) { | ||||||||||||||||||||||||||||||
| debug('session close callback', errorType, code, reason, errorName); | ||||||||||||||||||||||||||||||
| this[kOwner][kFinishClose](errorType, code, reason, errorName); | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| @@ -931,8 +932,12 @@ setCallbacks({ | ||||||||||||||||||||||||||||||
| // was an abnormal termination even if the session closed cleanly. | ||||||||||||||||||||||||||||||
| const resetCode = getQuicStreamState(this[kOwner]).resetCode; | ||||||||||||||||||||||||||||||
| if (resetCode !== undefined && resetCode > 0n) { | ||||||||||||||||||||||||||||||
| error = new ERR_QUIC_APPLICATION_ERROR( | ||||||||||||||||||||||||||||||
| resetCode, `stream reset with code ${resetCode}`); | ||||||||||||||||||||||||||||||
| error = makeQuicError( | ||||||||||||||||||||||||||||||
| 'ERR_QUIC_APPLICATION_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC application error', | ||||||||||||||||||||||||||||||
| 'application', | ||||||||||||||||||||||||||||||
| resetCode, | ||||||||||||||||||||||||||||||
| `stream reset with code ${resetCode}`); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| debug(`stream ${this[kOwner].id} closed callback with error: ${error}`); | ||||||||||||||||||||||||||||||
| @@ -1054,21 +1059,50 @@ class QuicError extends Error { | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| // Converts a raw QuicError array [type, code, reason] from C++ into a | ||||||||||||||||||||||||||||||
| // proper Node.js Error object. | ||||||||||||||||||||||||||||||
| // Build the human-readable message for an ERR_QUIC_TRANSPORT_ERROR or | ||||||||||||||||||||||||||||||
| // ERR_QUIC_APPLICATION_ERROR. `errorName` is the symbolic name for | ||||||||||||||||||||||||||||||
| // the wire code when known: either the OpenSSL-decoded TLS alert | ||||||||||||||||||||||||||||||
| // (CRYPTO_ERROR; 0x100..0x1ff) or one of the named transport codes | ||||||||||||||||||||||||||||||
| // from RFC 9000 (e.g. PROTOCOL_VIOLATION). Otherwise undefined. | ||||||||||||||||||||||||||||||
| // `reason` is the peer-supplied UTF-8 reason string from the | ||||||||||||||||||||||||||||||
| // CONNECTION_CLOSE / RESET_STREAM frame, often empty. | ||||||||||||||||||||||||||||||
| function quicErrorMessage(prefix, errorCode, reason, errorName) { | ||||||||||||||||||||||||||||||
| let msg = `${prefix} `; | ||||||||||||||||||||||||||||||
| msg += errorName ? `${errorName} (${errorCode})` : `${errorCode}`; | ||||||||||||||||||||||||||||||
| if (reason) msg += `: ${reason}`; | ||||||||||||||||||||||||||||||
| return msg; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| function makeQuicError(code, prefix, type, errorCode, reason, errorName) { | ||||||||||||||||||||||||||||||
| const err = new QuicError( | ||||||||||||||||||||||||||||||
| quicErrorMessage(prefix, errorCode, reason, errorName), | ||||||||||||||||||||||||||||||
| { errorCode, code, type }); | ||||||||||||||||||||||||||||||
| ErrorCaptureStackTrace(err, makeQuicError); | ||||||||||||||||||||||||||||||
| if (reason) err.reason = reason; | ||||||||||||||||||||||||||||||
| if (errorName) err.errorName = errorName; | ||||||||||||||||||||||||||||||
| return err; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
Comment on lines
+1076
to
+1084
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we not keep the same error construction mechanism as the others? We lose a bunch of things by not using that. Not sure we should be taking these out of the global errors set. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a suggestion here, changed here. We still use standard error codes, but we use a single What do we lose by not using the standard error mechanism vs this setup? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We lose the hideStackFrames logic, which means we'll see a bunch more internals things in stack traces here which we might not want to surface, like every validator gains a stack line without this. I'm not against this change, but just want to be sure that special-casing this is acceptable. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, I think the previous code didn't do this either - our validators do hide internal stack traces, but AFAICT the normal error code definitions don't do this unless you use It is easy to do though, and I agree it'd be a nice improvement - I've just pushed a commit that adds Lines 350 to 363 in 92f48f4
Is that what you were looking for? | ||||||||||||||||||||||||||||||
| function convertQuicError(error) { | ||||||||||||||||||||||||||||||
| const type = error[0]; | ||||||||||||||||||||||||||||||
| const code = error[1]; | ||||||||||||||||||||||||||||||
| const reason = error[2]; | ||||||||||||||||||||||||||||||
| const errorName = error[3]; | ||||||||||||||||||||||||||||||
| switch (type) { | ||||||||||||||||||||||||||||||
| case 'transport': | ||||||||||||||||||||||||||||||
| return new ERR_QUIC_TRANSPORT_ERROR(code, reason); | ||||||||||||||||||||||||||||||
| return makeQuicError('ERR_QUIC_TRANSPORT_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC transport error', | ||||||||||||||||||||||||||||||
| 'transport', code, reason, errorName); | ||||||||||||||||||||||||||||||
| case 'application': | ||||||||||||||||||||||||||||||
| return new ERR_QUIC_APPLICATION_ERROR(code, reason); | ||||||||||||||||||||||||||||||
| return makeQuicError('ERR_QUIC_APPLICATION_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC application error', | ||||||||||||||||||||||||||||||
| 'application', code, reason, errorName); | ||||||||||||||||||||||||||||||
| case 'version_negotiation': | ||||||||||||||||||||||||||||||
| return new ERR_QUIC_VERSION_NEGOTIATION_ERROR(); | ||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||
| return new ERR_QUIC_TRANSPORT_ERROR(code, reason); | ||||||||||||||||||||||||||||||
| return makeQuicError('ERR_QUIC_TRANSPORT_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC transport error', | ||||||||||||||||||||||||||||||
| 'transport', code, reason, errorName); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| @@ -3575,7 +3609,7 @@ class QuicSession { | ||||||||||||||||||||||||||||||
| * @param {number} code | ||||||||||||||||||||||||||||||
| * @param {string} [reason] | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| [kFinishClose](errorType, code, reason) { | ||||||||||||||||||||||||||||||
| [kFinishClose](errorType, code, reason, errorName) { | ||||||||||||||||||||||||||||||
| // If code is zero, then we closed without an error. Yay! We can destroy | ||||||||||||||||||||||||||||||
| // safely without specifying an error. | ||||||||||||||||||||||||||||||
| if (code === 0n) { | ||||||||||||||||||||||||||||||
| @@ -3584,7 +3618,8 @@ class QuicSession { | ||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| debug('finishing closing the session with an error', errorType, code, reason); | ||||||||||||||||||||||||||||||
| debug('finishing closing the session with an error', | ||||||||||||||||||||||||||||||
| errorType, code, reason, errorName); | ||||||||||||||||||||||||||||||
| // If the local side initiated this close with an error code (via | ||||||||||||||||||||||||||||||
| // close({ code })), this is an intentional shutdown; not an error. | ||||||||||||||||||||||||||||||
| @@ -3611,10 +3646,14 @@ class QuicSession { | ||||||||||||||||||||||||||||||
| // session would leak with `closed` hanging forever. | ||||||||||||||||||||||||||||||
| switch (errorType) { | ||||||||||||||||||||||||||||||
| case 0: /* Transport Error */ | ||||||||||||||||||||||||||||||
| this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason)); | ||||||||||||||||||||||||||||||
| this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC transport error', | ||||||||||||||||||||||||||||||
| 'transport', code, reason, errorName)); | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| case 1: /* Application Error */ | ||||||||||||||||||||||||||||||
| this.destroy(new ERR_QUIC_APPLICATION_ERROR(code, reason)); | ||||||||||||||||||||||||||||||
| this.destroy(makeQuicError('ERR_QUIC_APPLICATION_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC application error', | ||||||||||||||||||||||||||||||
| 'application', code, reason, errorName)); | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| case 2: /* Version Negotiation Error */ | ||||||||||||||||||||||||||||||
| this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR()); | ||||||||||||||||||||||||||||||
| @@ -3623,7 +3662,9 @@ class QuicSession { | ||||||||||||||||||||||||||||||
| this.destroy(); | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||
| this.destroy(new ERR_QUIC_TRANSPORT_ERROR(code, reason)); | ||||||||||||||||||||||||||||||
| this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC transport error', | ||||||||||||||||||||||||||||||
| 'transport', code, reason, errorName)); | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| @@ -3874,9 +3915,13 @@ class QuicSession { | ||||||||||||||||||||||||||||||
| // decide. In 'strict' mode, the handshake already failed at the C++ | ||||||||||||||||||||||||||||||
| // level (SSL_VERIFY_PEER) so we won't reach here. | ||||||||||||||||||||||||||||||
| if (inner.verifyPeer === 'auto' && validationErrorReason !== undefined) { | ||||||||||||||||||||||||||||||
| const err = new ERR_QUIC_TRANSPORT_ERROR( | ||||||||||||||||||||||||||||||
| 0, `Peer certificate validation failed: ${validationErrorReason}` + | ||||||||||||||||||||||||||||||
| ` [${validationErrorCode}]`); | ||||||||||||||||||||||||||||||
| const err = makeQuicError( | ||||||||||||||||||||||||||||||
| 'ERR_QUIC_TRANSPORT_ERROR', | ||||||||||||||||||||||||||||||
| 'QUIC transport error', | ||||||||||||||||||||||||||||||
| 'transport', | ||||||||||||||||||||||||||||||
| 0n, | ||||||||||||||||||||||||||||||
| `Peer certificate validation failed: ${validationErrorReason}` + | ||||||||||||||||||||||||||||||
| ` [${validationErrorCode}]`); | ||||||||||||||||||||||||||||||
| inner.pendingOpen.reject?.(err); | ||||||||||||||||||||||||||||||
| inner.pendingOpen.resolve = undefined; | ||||||||||||||||||||||||||||||
| inner.pendingOpen.reject = undefined; | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| #if HAVE_OPENSSL && HAVE_QUIC | ||
| #include "guard.h" | ||
| #ifndef OPENSSL_NO_QUIC | ||
| #include "data.h" | ||
| #include <env-inl.h> | ||
| #include <memory_tracker-inl.h> | ||
| #include <ngtcp2/ngtcp2.h> | ||
| #include <node_sockaddr-inl.h> | ||
| #include <openssl/ssl.h> | ||
| #include <string_bytes.h> | ||
| #include <v8.h> | ||
| #include "bindingdata.h" | ||
| #include "data.h" | ||
| #include "defs.h" | ||
| #include "util.h" | ||
| @@ -363,6 +365,62 @@ std::optional<int> QuicError::get_crypto_error() const { | ||
| return code() & ~NGTCP2_CRYPTO_ERROR; | ||
| } | ||
| const char* QuicError::name() const { | ||
| // CRYPTO_ERROR carries a TLS alert in its low byte (RFC 9001 sec. 4.8). | ||
| // OpenSSL's SSL_alert_desc_string_long owns a stable string for every | ||
| // alert it knows about; we filter out the "unknown" placeholder so the | ||
| // JS side can present `errorName` as undefined for unrecognised alerts. | ||
| if (auto alert = get_crypto_error()) { | ||
| const char* n = SSL_alert_desc_string_long(*alert); | ||
| if (n != nullptr && std::string_view(n) != "unknown") return n; | ||
| return nullptr; | ||
| } | ||
| // Named transport-layer error codes from RFC 9000 sec. 20.1 (and the | ||
| // RFC 9368 version-negotiation extension). Application error codes are | ||
| // opaque to QUIC, so we only decode for transport. | ||
| if (type() != Type::TRANSPORT) return nullptr; | ||
| switch (code()) { | ||
| case NGTCP2_NO_ERROR: | ||
| return "NO_ERROR"; | ||
| case NGTCP2_INTERNAL_ERROR: | ||
| return "INTERNAL_ERROR"; | ||
| case NGTCP2_CONNECTION_REFUSED: | ||
| return "CONNECTION_REFUSED"; | ||
| case NGTCP2_FLOW_CONTROL_ERROR: | ||
| return "FLOW_CONTROL_ERROR"; | ||
| case NGTCP2_STREAM_LIMIT_ERROR: | ||
| return "STREAM_LIMIT_ERROR"; | ||
| case NGTCP2_STREAM_STATE_ERROR: | ||
| return "STREAM_STATE_ERROR"; | ||
| case NGTCP2_FINAL_SIZE_ERROR: | ||
| return "FINAL_SIZE_ERROR"; | ||
| case NGTCP2_FRAME_ENCODING_ERROR: | ||
| return "FRAME_ENCODING_ERROR"; | ||
| case NGTCP2_TRANSPORT_PARAMETER_ERROR: | ||
| return "TRANSPORT_PARAMETER_ERROR"; | ||
| case NGTCP2_CONNECTION_ID_LIMIT_ERROR: | ||
| return "CONNECTION_ID_LIMIT_ERROR"; | ||
| case NGTCP2_PROTOCOL_VIOLATION: | ||
| return "PROTOCOL_VIOLATION"; | ||
| case NGTCP2_INVALID_TOKEN: | ||
| return "INVALID_TOKEN"; | ||
| case NGTCP2_APPLICATION_ERROR: | ||
| return "APPLICATION_ERROR"; | ||
| case NGTCP2_CRYPTO_BUFFER_EXCEEDED: | ||
| return "CRYPTO_BUFFER_EXCEEDED"; | ||
| case NGTCP2_KEY_UPDATE_ERROR: | ||
| return "KEY_UPDATE_ERROR"; | ||
| case NGTCP2_AEAD_LIMIT_REACHED: | ||
| return "AEAD_LIMIT_REACHED"; | ||
| case NGTCP2_NO_VIABLE_PATH: | ||
| return "NO_VIABLE_PATH"; | ||
| case NGTCP2_VERSION_NEGOTIATION_ERROR: | ||
| return "VERSION_NEGOTIATION_ERROR"; | ||
pimterry marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| default: | ||
| return nullptr; | ||
| } | ||
| } | ||
| MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const { | ||
| if ((type() == Type::TRANSPORT && code() == NGTCP2_NO_ERROR) || | ||
| (type() == Type::APPLICATION && | ||
| @@ -384,6 +442,7 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const { | ||
| type_str, | ||
| BigInt::NewFromUnsigned(env->isolate(), code()), | ||
| Undefined(env->isolate()), | ||
| Undefined(env->isolate()), | ||
| }; | ||
| // Note that per the QUIC specification, the reason, if present, is | ||
| @@ -397,6 +456,13 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const { | ||
| return {}; | ||
| } | ||
| // Attach a human-readable name for known wire codes (RFC 9000 sec. 20.1 | ||
| // names and OpenSSL TLS alert descriptions for CRYPTO_ERROR). Unknown | ||
| // codes leave the slot as undefined. | ||
| if (const char* n = name()) { | ||
| argv[3] = BindingData::Get(env).error_name_string(n); | ||
| } | ||
| return Array::New(env->isolate(), argv, arraysize(argv)).As<Value>(); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.