diff --git a/Cargo.lock b/Cargo.lock index 8a2a792c6fd0..0820e33bd3ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -577,6 +577,7 @@ dependencies = [ "cln-plugin", "cln-rpc", "futures-util", + "http-body-util", "hyper 1.11.0", "log", "log-panics", diff --git a/common/bolt12.c b/common/bolt12.c index 3b80f0578376..ace3ff853595 100644 --- a/common/bolt12.c +++ b/common/bolt12.c @@ -666,31 +666,51 @@ bool bolt12_has_prefix(const char *str) bolt12_has_request_prefix(str); } -/* Inclusive span of tlv range >= minfield and <= maxfield */ +/* Byte length of the run of TLV records whose type is in [minfield, maxfield], + * storing its offset into tlvstream in *startp. TLV records are ordered by + * type, so the wanted records are contiguous: we track two byte offsets into + * the stream, `start` (the first record at or above minfield) and `end` (just + * past the last record at or below maxfield), and return end - start. + * + * Offsets, not pointers: a pointer past the end of a range that was never + * entered would underflow to a near-SIZE_MAX length, which callers hash. */ size_t tlv_span(const u8 *tlvstream, u64 minfield, u64 maxfield, size_t *startp) { const u8 *cursor = tlvstream; size_t tlvlen = tal_bytelen(tlvstream); - const u8 *start, *end; + size_t start, end; + /* Distinguishes "start is offset 0" from "no in-range field seen". */ + bool have_start = false; - start = end = NULL; + start = end = 0; while (tlvlen) { - const u8 *before = cursor; + size_t before = cursor - tlvstream; bigsize_t type = fromwire_bigsize(&cursor, &tlvlen); bigsize_t len = fromwire_bigsize(&cursor, &tlvlen); - if (type >= minfield && start == NULL) + /* Truncated header: stop, keeping the span so far. */ + if (!cursor) + break; + if (type >= minfield && !have_start) { start = before; + have_start = true; + } + /* Past the range: later records are all higher, so we're done. */ if (type > maxfield) break; fromwire_pad(&cursor, &tlvlen, len); - end = cursor; + /* Truncated value: this record does not count. */ + if (!cursor) + break; + end = cursor - tlvstream; } - if (!start) + /* No in-range field, or the range opened only after the last record we + * accepted: collapse to an empty span at end rather than a bogus length. */ + if (!have_start || end < start) start = end; if (startp) - *startp = start - tlvstream; + *startp = start; return end - start; } diff --git a/common/json_parse_simple.c b/common/json_parse_simple.c index 7e0e299c9dfa..356942fe4c42 100644 --- a/common/json_parse_simple.c +++ b/common/json_parse_simple.c @@ -179,6 +179,47 @@ const jsmntok_t *json_next(const jsmntok_t *tok) return t; } +/* We refuse JSON nested deeper than this. jsmn tokenizes iteratively, but + * json_next() and the validators below recurse once per nesting level, so an + * unbounded depth overflows the C stack. Real JSON-RPC and BOLT payloads nest + * only a handful of levels; this sits far above them and far below the stack + * limit. Enforced in json_parse_input(), so every token array handed to + * json_next() and friends elsewhere has already been bounded. */ +#define JSON_MAX_NESTING 256 + +/* Iteratively count the tokens in the first datum of toks[], rejecting + * anything nested deeper than JSON_MAX_NESTING. On success sets *len to the + * token count (as json_next(toks) - toks would) and returns true; returns + * false without recursing on over-nested, attacker-controlled input. */ +static bool bounded_datum_len(const jsmntok_t *toks, size_t *len) +{ + /* remaining[d] = child datums still to visit at nesting level d; + * level 0 holds the single root datum. */ + size_t remaining[JSON_MAX_NESTING + 1]; + size_t depth = 0, i = 0; + + remaining[0] = 1; + for (;;) { + /* Ascend out of every level we have finished. */ + while (remaining[depth] == 0) { + if (depth == 0) { + *len = i; + return true; + } + depth--; + } + remaining[depth]--; + + /* Descend into this token's children, if it has any. */ + if (toks[i].size != 0) { + if (depth == JSON_MAX_NESTING) + return false; + remaining[++depth] = toks[i].size; + } + i++; + } +} + const jsmntok_t *json_get_membern(const char *buffer, const jsmntok_t tok[], const char *label, size_t len) @@ -492,8 +533,11 @@ bool json_parse_input(jsmn_parser *parser, /* If we read a partial element at the end of the stream we'll get a * errro, but due to the previous check we know we read at * least one full element, so count tokens that are part of this root - * element. */ - ret = json_next(*toks) - *toks; + * element. Bound the nesting depth here, before any recursive walk. */ + size_t datumlen; + if (!bounded_datum_len(*toks, &datumlen)) + return false; + ret = datumlen; if (!validate_jsmn_parse_output(input, *toks, *toks + ret)) return false; diff --git a/common/test/run-json.c b/common/test/run-json.c index 1fc42a2223ae..c0fcaeaaa92a 100644 --- a/common/test/run-json.c +++ b/common/test/run-json.c @@ -226,6 +226,49 @@ static void test_json_bad_utf8(void) assert(json_parse_simple(tmpctx, buf, strlen(buf))); } +static void test_json_deep_nesting(void) +{ + char *buf; + size_t d, i; + + /* Arrays nested exactly at the limit still parse. */ + d = JSON_MAX_NESTING; + buf = tal_arr(tmpctx, char, 2 * d + 2); + memset(buf, '[', d); + buf[d] = '0'; + memset(buf + d + 1, ']', d); + buf[2 * d + 1] = '\0'; + assert(json_parse_simple(tmpctx, buf, 2 * d + 1)); + + /* One level deeper is rejected, not crashed. */ + d = JSON_MAX_NESTING + 1; + buf = tal_arr(tmpctx, char, 2 * d + 2); + memset(buf, '[', d); + buf[d] = '0'; + memset(buf + d + 1, ']', d); + buf[2 * d + 1] = '\0'; + assert(!json_parse_simple(tmpctx, buf, 2 * d + 1)); + + /* A pathologically deep array is rejected iteratively, without + * overflowing the stack. */ + d = 100000; + buf = tal_arr(tmpctx, char, 2 * d + 2); + memset(buf, '[', d); + buf[d] = '0'; + memset(buf + d + 1, ']', d); + buf[2 * d + 1] = '\0'; + assert(!json_parse_simple(tmpctx, buf, 2 * d + 1)); + + /* Same for deeply nested objects. */ + buf = tal_strdup(tmpctx, ""); + for (i = 0; i < 100000; i++) + tal_append_fmt(&buf, "{\"a\":"); + tal_append_fmt(&buf, "1"); + for (i = 0; i < 100000; i++) + tal_append_fmt(&buf, "}"); + assert(!json_parse_simple(tmpctx, buf, strlen(buf))); +} + int main(int argc, char *argv[]) { common_setup(argv[0]); @@ -234,6 +277,7 @@ int main(int argc, char *argv[]) test_json_tok_bitcoin_amount(); test_json_tok_millionths(); test_json_bad_utf8(); + test_json_deep_nesting(); common_shutdown(); } diff --git a/common/test/run-tlv_span.c b/common/test/run-tlv_span.c index 8d5e22d8314f..67b9d57f11b8 100644 --- a/common/test/run-tlv_span.c +++ b/common/test/run-tlv_span.c @@ -115,10 +115,17 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE { fprintf(stderr, "towire_u8_array called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ +/* bolt12 id-hash field ranges: invoice_request hashes 0..159, offer 1..79. */ +#define INVREQ_MIN 0 +#define INVREQ_MAX 159 +#define OFFER_MIN 1 +#define OFFER_MAX 79 + int main(int argc, char *argv[]) { u8 *wire; size_t len, start; + const char *hex; common_setup(argv[0]); @@ -135,6 +142,42 @@ int main(int argc, char *argv[]) len = tlv_span(wire, 0, 1, &start); assert(start == 0); assert(len == strlen("0010b8538094dbd70d8a0f0439d8e64f766f") / 2); + + /* Stream whose first field is already above maxfield: empty span. */ + hex = "f0020102"; + wire = tal_hexdata(tmpctx, hex, strlen(hex)); + len = tlv_span(wire, INVREQ_MIN, INVREQ_MAX, &start); + assert(start <= tal_bytelen(wire)); + assert(len == 0); + + len = tlv_span(wire, OFFER_MIN, OFFER_MAX, &start); + assert(start <= tal_bytelen(wire)); + assert(len == 0); + + /* Same, but with a field below the range in front of it. */ + hex = "0002010af0020102"; + wire = tal_hexdata(tmpctx, hex, strlen(hex)); + len = tlv_span(wire, OFFER_MIN, OFFER_MAX, &start); + assert(start <= tal_bytelen(wire)); + assert(len == 0); + + len = tlv_span(wire, INVREQ_MIN, INVREQ_MAX, &start); + assert(start == 0); + assert(len == strlen("0002010a") / 2); + + /* Truncated stream. */ + hex = "f0"; + wire = tal_hexdata(tmpctx, hex, strlen(hex)); + len = tlv_span(wire, INVREQ_MIN, INVREQ_MAX, &start); + assert(start <= tal_bytelen(wire)); + assert(len == 0); + + hex = "0002"; + wire = tal_hexdata(tmpctx, hex, strlen(hex)); + len = tlv_span(wire, INVREQ_MIN, INVREQ_MAX, &start); + assert(start <= tal_bytelen(wire)); + assert(len == 0); + common_shutdown(); return 0; } diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c index 5c10d191b25d..c94d451c4e49 100644 --- a/common/test/run-wireaddr.c +++ b/common/test/run-wireaddr.c @@ -1,5 +1,6 @@ #include "config.h" #include +#include #include #include #include @@ -263,6 +264,47 @@ int main(int argc, char *argv[]) assert(fromwire_wireaddr((const u8 **) &encoded_wa, &encoded_wa_len, &decoded_wa) == FROMWIREADDR_OK); assert(wireaddr_eq(&wa, &decoded_wa)); + /* A DNS descriptor which isn't actually a hostname is ignored, rather + * than handed on to the rest of the daemon. */ + const char *baddnsaddrs[] = { "", + "not a hostname", + "invalid..example.com", + "-.invalid.com" }; + + for (size_t i = 0; i < ARRAY_SIZE(baddnsaddrs); i++) { + struct wireaddr bad_wa = { + .type = ADDR_TYPE_DNS, + .addrlen = strlen(baddnsaddrs[i]), + .port = DEFAULT_PORT + }; + memcpy(bad_wa.addr, baddnsaddrs[i], bad_wa.addrlen); + + u8 *encoded_bad = tal_arr(tmpctx, u8, 0); + towire_wireaddr(&encoded_bad, &bad_wa); + size_t encoded_bad_len = tal_bytelen(encoded_bad); + + struct wireaddr decoded_bad; + assert(fromwire_wireaddr((const u8 **) &encoded_bad, + &encoded_bad_len, + &decoded_bad) == FROMWIREADDR_IGNORE); + } + + /* ...and we don't produce one either: a dns: address which isn't a + * hostname is rejected at parse time, rather than accepted here and + * silently dropped by every peer we announce it to. */ + for (size_t i = 0; i < ARRAY_SIZE(baddnsaddrs); i++) { + struct wireaddr parsed; + const char *arg = tal_fmt(tmpctx, "dns:%s", baddnsaddrs[i]); + assert(parse_wireaddr(tmpctx, arg, DEFAULT_PORT, NULL, + &parsed) != NULL); + } + + /* A real hostname still works. */ + struct wireaddr good_dns; + assert(parse_wireaddr(tmpctx, "dns:example.com", DEFAULT_PORT, NULL, + &good_dns) == NULL); + assert(good_dns.type == ADDR_TYPE_DNS); + tal_free(expect); common_shutdown(); } diff --git a/common/wireaddr.c b/common/wireaddr.c index 80932566c6d5..1364e4931383 100644 --- a/common/wireaddr.c +++ b/common/wireaddr.c @@ -65,6 +65,22 @@ enum fromwireaddr_ret fromwire_wireaddr(const u8 **cursor, size_t *max, struct w /* FIXME: This seems universal? */ if (addr->port == 0) return FROMWIREADDR_IGNORE; + + /* BOLT #7: + * * `5`: DNS hostname; data = `[1:hostname_len][hostname_len:hostname][2:port]` (length up to 258) + * * `hostname` bytes MUST be ASCII characters. + */ + /* Don't let a malformed name loose in the rest of the daemon: it + * ends up in log messages, in connect requests, and in whatever a + * plugin does with it. */ + if (addr->type == ADDR_TYPE_DNS) { + char hostname[DNS_ADDRLEN + 1]; + + memcpy(hostname, addr->addr, addr->addrlen); + hostname[addr->addrlen] = '\0'; + if (!is_dnsaddr(hostname)) + return FROMWIREADDR_IGNORE; + } return FROMWIREADDR_OK; } @@ -563,6 +579,12 @@ const char *parse_wireaddr(const tal_t *ctx, if (strlen(ip) > DNS_ADDRLEN) return "DNS address too long"; + /* fromwire_wireaddr() ignores a DNS descriptor which isn't a + * hostname, so anything we announce that fails this is dropped + * by every peer. Say so here, where the operator can see it. */ + if (!is_dnsaddr(ip)) + return tal_fmt(ctx, "dns: '%s' is not a hostname", ip); + addr->addrlen = strlen(ip); memcpy(addr->addr, ip, addr->addrlen); addr->port = port; diff --git a/connectd/tor.c b/connectd/tor.c index 95ba253abc42..83c25df7c6fe 100644 --- a/connectd/tor.c +++ b/connectd/tor.c @@ -17,7 +17,6 @@ #define SOCKS_TYP_IPV6 4 #define SOCKS_V5 5 -#define MAX_SIZE_OF_SOCKS5_REQ_OR_RESP 255 #define SIZE_OF_RESPONSE 4 #define SIZE_OF_REQUEST 3 #define SIZE_OF_IPV4_RESPONSE 6 @@ -26,6 +25,14 @@ #define SOCK_REQ_V5_LEN 5 #define SOCK_REQ_V5_HEADER_LEN 7 +/* The domain name in a SOCKS5 request is preceded by a single length + * byte, so it can never be longer than this. */ +#define MAX_SIZE_OF_SOCKS5_DOMAIN 255 +/* The largest thing we ever put in the buffer is a domain-name CONNECT + * request: the header plus a maximum-length domain name. */ +#define MAX_SIZE_OF_SOCKS5_REQ_OR_RESP (SOCK_REQ_V5_HEADER_LEN \ + + MAX_SIZE_OF_SOCKS5_DOMAIN) + /* some crufts can not forward ipv6 */ #undef BIND_FIRST_TO_IPV6 @@ -154,6 +161,21 @@ static struct io_plan *io_tor_connect_after_resp_to_connect(struct io_conn if (connect->buffer[1] == '\0') { /* make the V5 request */ connect->hlen = strlen(connect->host); + + /* The length is carried in a single byte, and the whole + * request has to fit in our buffer: refuse rather than + * build a request we can't represent. */ + if (connect->hlen > MAX_SIZE_OF_SOCKS5_DOMAIN) { + const char *msg = tal_fmt(tmpctx, + "Connected out for %s error: hostname too long for socks5 request", + connect->host); + status_debug("%s", msg); + add_errors_to_error_list(connect->connect, msg); + + errno = ECONNREFUSED; + return io_close(conn); + } + connect->buffer[0] = SOCKS_V5; connect->buffer[1] = SOCKS_CONNECT; connect->buffer[2] = 0; @@ -161,7 +183,7 @@ static struct io_plan *io_tor_connect_after_resp_to_connect(struct io_conn connect->buffer[4] = connect->hlen; memcpy(connect->buffer + SOCK_REQ_V5_LEN, connect->host, connect->hlen); - memcpy(connect->buffer + SOCK_REQ_V5_LEN + strlen(connect->host), + memcpy(connect->buffer + SOCK_REQ_V5_LEN + connect->hlen, &(connect->port), sizeof connect->port); status_io(LOG_IO_OUT, NULL, "proxy", connect->buffer, diff --git a/plugins/rest-plugin/Cargo.toml b/plugins/rest-plugin/Cargo.toml index 5db65b12e9ce..572e84e311ef 100644 --- a/plugins/rest-plugin/Cargo.toml +++ b/plugins/rest-plugin/Cargo.toml @@ -41,6 +41,7 @@ futures-util = { version = "0.3", default-features = false, features = [ ] } rcgen = "0.14" hyper = "1" +http-body-util = "0.1" tower = "0.5" tower-http = { version = "0.7", features = ["cors", "set-header"] } utoipa = { version = "5", features = ['axum_extras'] } diff --git a/plugins/rest-plugin/src/handlers.rs b/plugins/rest-plugin/src/handlers.rs index 77bb6375fbee..9a59853ce4d2 100644 --- a/plugins/rest-plugin/src/handlers.rs +++ b/plugins/rest-plugin/src/handlers.rs @@ -23,6 +23,10 @@ use crate::{ structs::{AppError, CheckRuneParams, ClnrestMap, PluginState}, }; +/// Maximum size (in bytes) of an accepted request body. +/// Oversized bodies are rejected with 413. +pub const MAX_BODY_SIZE: usize = 2 * 1024 * 1024; // 2 MiB + /* Handler for list-methods */ #[utoipa::path( get, @@ -115,11 +119,20 @@ pub async fn call_rpc_method( .and_then(|v| v.to_str().ok()) .map(String::from); - let request_bytes = match to_bytes(body.into_body(), usize::MAX).await { + let request_bytes = match to_bytes(body.into_body(), MAX_BODY_SIZE).await { Ok(o) => o, Err(e) => { + if is_body_too_large(&e) { + return Err(AppError::PayloadTooLarge(RpcError { + code: Some(-32600), + data: None, + message: format!( + "Request body exceeds the maximum allowed size of {MAX_BODY_SIZE} bytes" + ), + })); + } return Err(AppError::InternalServerError(RpcError { - code: None, + code: Some(-32700), data: None, message: format!("Could not read request body: {}", e), })); @@ -160,6 +173,21 @@ pub async fn call_rpc_method( convert_json_to_response(headers, &rest_map.rpc_method, cln_result) } +fn is_body_too_large(e: &axum::Error) -> bool { + fn inner(e: &(dyn std::error::Error + 'static)) -> bool { + if e.downcast_ref::() + .is_some() + { + return true; + } + match e.source() { + Some(src) => inner(src), + None => false, + } + } + inner(e) +} + fn fill_rune_restrictions( rest_map: &mut ClnrestMap, rpc_params: &serde_json::Map, diff --git a/plugins/rest-plugin/src/structs.rs b/plugins/rest-plugin/src/structs.rs index 0cc0ce103e69..b77666f60154 100644 --- a/plugins/rest-plugin/src/structs.rs +++ b/plugins/rest-plugin/src/structs.rs @@ -31,6 +31,7 @@ pub enum AppError { MethodNotAllowed(RpcError), InternalServerError(RpcError), NotAcceptable(RpcError), + PayloadTooLarge(RpcError), } impl IntoResponse for AppError { @@ -42,6 +43,7 @@ impl IntoResponse for AppError { AppError::MethodNotAllowed(err) => (StatusCode::METHOD_NOT_ALLOWED, err), AppError::InternalServerError(err) => (StatusCode::INTERNAL_SERVER_ERROR, err), AppError::NotAcceptable(err) => (StatusCode::NOT_ACCEPTABLE, err), + AppError::PayloadTooLarge(err) => (StatusCode::PAYLOAD_TOO_LARGE, err), }; let body = Json(json!(error_message)); @@ -58,6 +60,7 @@ impl std::fmt::Display for AppError { AppError::MethodNotAllowed(err) => write!(f, "Method not allowed: {err}"), AppError::InternalServerError(err) => write!(f, "Internal Server Error: {err}"), AppError::NotAcceptable(err) => write!(f, "Not Acceptable: {err}"), + AppError::PayloadTooLarge(err) => write!(f, "Payload Too Large: {err}"), } } } diff --git a/tests/test_clnrest.py b/tests/test_clnrest.py index 479ca25a7938..bcc9a668e122 100644 --- a/tests/test_clnrest.py +++ b/tests/test_clnrest.py @@ -27,6 +27,7 @@ def http_session_with_retry(): retry = Retry(connect=10, backoff_factor=0.5) adapter = HTTPAdapter(max_retries=retry) http_session.mount('https://', adapter) + http_session.mount('http://', adapter) return http_session @@ -131,7 +132,7 @@ def test_generate_certificate(node_factory): assert [c[0] != c[1] for c in zip(contents, contents_2)] == [True] * len(files) -def start_node_with_clnrest(node_factory, plugin=None): +def start_node_with_clnrest(node_factory, plugin=None, protocol='https'): """Start a node with the clnrest plugin, whose options are the default options. Return: - the node, @@ -139,11 +140,12 @@ def start_node_with_clnrest(node_factory, plugin=None): - the certificate authority path used for the self-signed certificates.""" rest_port = str(node_factory.get_unused_port()) rest_certs = node_factory.directory + '/clnrest-certs' - options = {'clnrest-port': rest_port, 'clnrest-certs': rest_certs} + options = {'clnrest-port': rest_port, 'clnrest-certs': rest_certs, + 'clnrest-protocol': protocol} if plugin is not None: options['plugin'] = plugin l1 = node_factory.get_node(options=options) - base_url = 'https://127.0.0.1:' + rest_port + base_url = f'{protocol}://127.0.0.1:' + rest_port # This might happen really early! l1.daemon.logsearch_start = 0 l1.daemon.wait_for_log(r'plugin-clnrest: REST server running at ' + base_url) @@ -882,3 +884,22 @@ def test_dynamic_path_rune(node_factory): dynamic_res.raise_for_status() dynamic_json = dynamic_res.json() assert dynamic_json["test-dynamic-clnrest"] == "success" + + +def test_large_request_body(node_factory): + """Test large request bodies getting rejected without a crash. + + Run over plain HTTP: with TLS, a client that sends the whole body before + reading the response can only see a connection reset instead of the 413, + depending on the OpenSSL version. + """ + l1, base_url, _ = start_node_with_clnrest(node_factory, protocol='http') + http_session = http_session_with_retry() + + body = b'{"pad":"' + b"B" * (32 * 1024 * 1024) + b'"}' + response = http_session.post(base_url + "/v1/getinfo", data=body) + assert response.status_code == 413 + assert response.json()["code"] == -32600 + assert "Request body exceeds the maximum allowed size" in response.json()["message"] + + l1.rpc.getinfo() diff --git a/tests/test_connection.py b/tests/test_connection.py index 4b07bda708c9..0e672477c94d 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -25,8 +25,10 @@ import unittest import websocket import signal +import socket import ssl import sys +import threading def test_connect_basic(node_factory): @@ -5136,3 +5138,84 @@ def test_open_channel_funding_above_max_supply(node_factory, bitcoind): funding_sat, push_msat) assert l1.rpc.getinfo()['id'] == l1.info['id'] + + +def test_connect_proxy_maxlen_hostname(node_factory): + """A maximum-length hostname must produce a well-formed SOCKS5 request. + + The request is assembled in a fixed buffer, and the hostname was copied + into it without checking that it fit, which overran the buffer and + corrupted the adjacent length field. That length was then used for the + write, so the proxy got a wildly oversized read of connectd's memory + instead of the request (and connectd died on the way). + + A gossiped DNS address reaches the same builder, so this covers that + path too. + """ + # A minimal SOCKS5 server: accept the "no authentication" greeting, + # then collect whatever request connectd sends us. + received = [] + listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + listener.bind(('127.0.0.1', 0)) + listener.listen(1) + proxyport = listener.getsockname()[1] + + def serve(): + data = b'' + conn, _ = listener.accept() + with conn: + try: + conn.settimeout(TIMEOUT) + conn.recv(len(b'\x05\x01\x00')) + conn.sendall(b'\x05\x00') + # Deliberately ask for far more than the largest legal + # request (262 bytes), so an oversized write is visible + # here rather than silently truncated by us. We stop on the + # timeout, once connectd has finished writing and is waiting + # for a reply we're never going to send. + conn.settimeout(2) + while len(data) < 65536: + more = conn.recv(65536) + if not more: + break + data += more + except OSError: + pass + received.append(data) + + server = threading.Thread(target=serve, daemon=True) + server.start() + + l1 = node_factory.get_node(options={'proxy': '127.0.0.1:{}'.format(proxyport), + 'always-use-proxy': 'true'}, + # Without the fix connectd dies here, and we + # want that to be a test failure rather than + # a teardown error. + may_fail=True, broken_log='.*') + + # unresolved.name[256] is what limits us: 255 is the longest we can ask for. + hostname = 'a' * (255 - len('.example.com')) + '.example.com' + assert len(hostname) == 255 + + # There's nothing on the far side of the proxy, so this fails: it just + # must not take connectd down with it. + # Any valid pubkey will do: we never get far enough to talk to it. + nodeid = '0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798' + with pytest.raises(RpcError, match="All addresses failed"): + l1.rpc.connect(nodeid, hostname, 1234) + + server.join(TIMEOUT) + listener.close() + + # connectd is still there, and so is the node. + assert not l1.daemon.is_in_log('FATAL SIGNAL') + l1.rpc.getinfo() + + # And the proxy got exactly the request it should have: version, CONNECT, + # reserved, "domain name", length, the name itself, then the port. + request = only_one(received) + assert request == (b'\x05\x01\x00\x03' + + bytes([len(hostname)]) + + hostname.encode('ascii') + + (1234).to_bytes(2, 'big'))