diff --git a/Makefile b/Makefile index f0741492..61e541ce 100644 --- a/Makefile +++ b/Makefile @@ -197,6 +197,7 @@ EXE=build/tcpecho build/tcp_netcat_poll build/tcp_netcat_select \ build/test-evloop build/test-dns build/test-wolfssl-forwarding \ build/test-ttl-expired build/test-wolfssl build/test-httpd \ build/test-http-smuggle build/test-http-arg-oob \ + build/test-http-headers \ build/test-http-close-notify \ build/test-freertos-close-last-ack \ build/test-posix-errno \ @@ -750,6 +751,14 @@ build/test-http-arg-oob: src/test/test_http_arg_oob.c src/http/httpd.c @echo "[LD] $@" @$(CC) $(CFLAGS) -o $@ src/test/test_http_arg_oob.c $(LDFLAGS) -lwolfssl +# Standalone regression test for header accumulation in parse_http_request +# every header line must be reachable through struct http_request.headers. +build/test-http-headers:CFLAGS+=-Wno-cpp -DWOLFSSL_DEBUG -DWOLFSSL_WOLFIP -DWOLFIP_ENABLE_HTTP -Isrc/http +build/test-http-headers: src/test/test_http_headers.c src/http/httpd.c + @mkdir -p build || true + @echo "[LD] $@" + @$(CC) $(CFLAGS) -o $@ src/test/test_http_headers.c $(LDFLAGS) -lwolfssl + # Standalone regression test for TLS close_notify on every close path (F-5732). # It #includes httpd.c directly and stubs the wolfSSL teardown calls to record # their order, so it does not link the real wolfSSL library. diff --git a/docs/http_server_howto.md b/docs/http_server_howto.md index 68407b45..b15ae501 100644 --- a/docs/http_server_howto.md +++ b/docs/http_server_howto.md @@ -193,7 +193,7 @@ struct http_request { char method[HTTP_METHOD_LEN]; /* "GET", "POST" (max 8) */ char path[HTTP_PATH_LEN]; /* URL path, percent-decoded (max 128) */ char query[HTTP_QUERY_LEN]; /* raw query string (max 256) */ - char headers[HTTP_HEADERS_LEN]; /* last header line seen (max 512) */ + char headers[HTTP_HEADERS_LEN]; /* header block, CRLF-joined (max 1024) */ char body[HTTP_BODY_LEN]; /* request body (max 1024) */ size_t body_len; }; @@ -204,10 +204,11 @@ response. A negative return from your handler causes the module to close the client connection (`http_recv_cb()` treats a negative parse/handler result as a failure and tears the connection down). -> **Note.** `req->headers` holds only the **last** header line parsed, not the -> full header block — the parser reuses one buffer. Use it for at most a single -> expected header; framing headers (`Content-Length`, `Transfer-Encoding`) are -> consumed internally and are not meant to be re-read here. +> **Note.** `req->headers` holds the request's header lines joined with the CRLF +> they arrived with, so a handler can re-split it on `"\r\n"`. A request whose +> header block does not fit is rejected; framing headers (`Content-Length`, +> `Transfer-Encoding`) are consumed internally and are not meant to be re-read +> here. ## 7. Reading the request: methods, query and form args diff --git a/src/http/httpd.c b/src/http/httpd.c index 2c067db4..3502b8b6 100644 --- a/src/http/httpd.c +++ b/src/http/httpd.c @@ -330,6 +330,8 @@ static int parse_http_request(struct http_client *hc, uint8_t *buf, size_t len) int has_te = 0; /* Transfer-Encoding header present */ struct http_request req; struct http_url *url = NULL; + size_t hdr_len = 0; /* tracks the bytes accumulated in req.headers */ + memset(&req, 0, sizeof(struct http_request)); if (len < 4) goto bad_request; @@ -421,9 +423,21 @@ static int parse_http_request(struct http_client *hc, uint8_t *buf, size_t len) has_te = 1; } } - /* Copy header and terminate */ - memcpy(req.headers, p, n); - req.headers[n] = '\0'; + { + size_t sep = (hdr_len > 0) ? 2 : 0; + if (hdr_len + sep + n >= sizeof(req.headers)) + goto bad_request; + if (sep) { + /* CRLF, so a consumer can re-split req.headers on the same + * delimiter the lines arrived with on the wire. */ + req.headers[hdr_len] = '\r'; + req.headers[hdr_len + 1] = '\n'; + } + /* Copy header and terminate */ + memcpy(req.headers + hdr_len + sep, p, n); + hdr_len += sep+n; + req.headers[hdr_len] = '\0'; + } p = q + 2; } /* Parse the body. The body length is taken from the declared diff --git a/src/http/httpd.h b/src/http/httpd.h index 82ea6a02..d4784196 100644 --- a/src/http/httpd.h +++ b/src/http/httpd.h @@ -16,7 +16,7 @@ #define HTTP_METHOD_LEN 8 #define HTTP_PATH_LEN 128 #define HTTP_QUERY_LEN 256 -#define HTTP_HEADERS_LEN 512 +#define HTTP_HEADERS_LEN 1024 #define HTTP_BODY_LEN 1024 diff --git a/src/test/test_http_headers.c b/src/test/test_http_headers.c new file mode 100644 index 00000000..c1edc50f --- /dev/null +++ b/src/test/test_http_headers.c @@ -0,0 +1,234 @@ +/* test_http_headers.c + * + * Copyright (C) 2024 wolfSSL Inc. + * + * This file is part of wolfIP TCP/IP stack. + * + * wolfIP is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfIP is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + * + * + * Regression test for the header-accumulation defect in parse_http_request. + * The header loop copies every header line to req.headers at a fixed offset of + * zero, so each line overwrites the previous one and only the last header of + * the request survives. struct http_request.headers is documented as "HTTP + * headers" and is the only API a handler has for reading them, so a handler + * that inspects it sees a single line rather than the request's headers. + * + * The accumulated lines are separated by CRLF, the delimiter they arrived with + * on the wire, so a handler can re-split req.headers on "\r\n". + */ + +#include +#include +#include + +/* Pull in the unit under test, parse_http_request is static. */ +#include "httpd.c" + +/* stubs for the wolfIP / wolfSSL symbols referenced by httpd.c */ +int wolfIP_sock_socket(struct wolfIP *s, int d, int t, int p) +{ (void)s; (void)d; (void)t; (void)p; return -1; } +int wolfIP_sock_bind(struct wolfIP *s, int fd, const struct wolfIP_sockaddr *a, socklen_t l) +{ (void)s; (void)fd; (void)a; (void)l; return -1; } +int wolfIP_sock_listen(struct wolfIP *s, int fd, int b) +{ (void)s; (void)fd; (void)b; return -1; } +int wolfIP_sock_accept(struct wolfIP *s, int fd, struct wolfIP_sockaddr *a, socklen_t *l) +{ (void)s; (void)fd; (void)a; (void)l; return -1; } +int wolfIP_sock_send(struct wolfIP *s, int fd, const void *b, size_t l, int f) +{ (void)s; (void)fd; (void)b; (void)f; return (int)l; } +int wolfIP_sock_recv(struct wolfIP *s, int fd, void *b, size_t l, int f) +{ (void)s; (void)fd; (void)b; (void)l; (void)f; return -1; } +int wolfIP_sock_close(struct wolfIP *s, int fd) +{ (void)s; (void)fd; return 0; } +void wolfIP_register_callback(struct wolfIP *s, int fd, tsocket_cb cb, void *arg) +{ (void)s; (void)fd; (void)cb; (void)arg; } +int wolfSSL_SetIO_wolfIP(WOLFSSL *ssl, int fd) +{ (void)ssl; (void)fd; return 0; } +int wolfSSL_SetIO_wolfIP_CTX(WOLFSSL_CTX *ctx, struct wolfIP *s) +{ (void)ctx; (void)s; return 0; } +void wolfSSL_CleanupIO_wolfIP(WOLFSSL *ssl) +{ (void)ssl; } + +/* test harness */ +static int handler_calls; +/* One byte of headroom: strnlen() can return the full field width if the + * parser ever leaves req.headers unterminated, and the capture must still be + * able to terminate its own copy without running off the end. */ +static char seen_headers[HTTP_HEADERS_LEN + 1]; +static size_t seen_headers_len; + +/* Records what a real consumer of the documented API would observe. */ +static int probe_handler(struct httpd *httpd, struct http_client *hc, struct http_request *req) +{ + (void)httpd; (void)hc; + handler_calls++; + seen_headers_len = strnlen(req->headers, sizeof(req->headers)); + memcpy(seen_headers, req->headers, seen_headers_len); + seen_headers[seen_headers_len] = '\0'; + return 0; +} + +static int run(struct httpd *httpd, const char *raw, size_t len) +{ + struct http_client hc; + /* Copy into a writable scratch buffer that mirrors the production recv + * buffer, so the test never hands a read-only string literal to the + * parser - safe even if parse_http_request ever normalizes in-place. */ + uint8_t buf[HTTP_RECV_BUF_LEN]; + if (len > sizeof(buf)) + len = sizeof(buf); + memcpy(buf, raw, len); + memset(&hc, 0, sizeof(hc)); + hc.httpd = httpd; + hc.client_sd = 1; + hc.ssl = NULL; + handler_calls = 0; + seen_headers[0] = '\0'; + seen_headers_len = 0; + return parse_http_request(&hc, buf, len); +} + +#define CHECK(cond) do { if (!(cond)) { \ + printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); failures++; } } while (0) + +int main(void) +{ + struct httpd httpd; + int failures = 0; + int r; + + memset(&httpd, 0, sizeof(httpd)); + httpd_register_handler(&httpd, "/probe", probe_handler); + + /* 1. Every header line sent must be visible through req.headers. */ + { + const char *req = + "GET /probe HTTP/1.1\r\n" + "Host: victim.local\r\n" + "Authorization: Bearer valid_token\r\n" + "X-Foo: bar\r\n" + "X-Last: last\r\n" + "\r\n"; + r = run(&httpd, req, strlen(req)); + CHECK(r == 0); + CHECK(handler_calls == 1); + CHECK(strstr(seen_headers, "Host: victim.local") != NULL); + CHECK(strstr(seen_headers, "Authorization: Bearer valid_token") != NULL); + CHECK(strstr(seen_headers, "X-Foo: bar") != NULL); + CHECK(strstr(seen_headers, "X-Last: last") != NULL); + } + + /* 2. A request carrying an Authorization header and one + * carrying none must not present an identical req.headers, or the two + * are indistinguishable to any handler that authorizes on it. */ + { + char with_auth[sizeof(seen_headers)]; + size_t with_auth_len; + const char *authed = + "GET /probe HTTP/1.1\r\n" + "Authorization: Bearer valid_token\r\n" + "X-Foo: bar\r\n" + "\r\n"; + const char *anon = + "GET /probe HTTP/1.1\r\n" + "X-Foo: bar\r\n" + "\r\n"; + + r = run(&httpd, authed, strlen(authed)); + CHECK(r == 0); + CHECK(handler_calls == 1); + with_auth_len = seen_headers_len; + memcpy(with_auth, seen_headers, with_auth_len + 1); + + r = run(&httpd, anon, strlen(anon)); + CHECK(r == 0); + CHECK(handler_calls == 1); + CHECK(strcmp(with_auth, seen_headers) != 0); + CHECK(with_auth_len > seen_headers_len); + } + + /* 3. Accumulated lines are joined with the CRLF they arrived with, so a + * handler re-splitting req.headers on "\r\n" recovers them. */ + { + const char *req = + "GET /probe HTTP/1.1\r\n" + "Host: victim.local\r\n" + "X-Foo: bar\r\n" + "\r\n"; + r = run(&httpd, req, strlen(req)); + CHECK(r == 0); + CHECK(handler_calls == 1); + CHECK(strcmp(seen_headers, "Host: victim.local\r\nX-Foo: bar") == 0); + } + + /* 4. A single header still round-trips exactly, with no separator or + * padding bolted on. */ + { + const char *req = + "GET /probe HTTP/1.1\r\n" + "Host: victim.local\r\n" + "\r\n"; + r = run(&httpd, req, strlen(req)); + CHECK(r == 0); + CHECK(handler_calls == 1); + CHECK(strcmp(seen_headers, "Host: victim.local") == 0); + } + + /* 5. A request with no headers at all leaves the field empty. */ + { + const char *req = + "GET /probe HTTP/1.1\r\n" + "\r\n"; + r = run(&httpd, req, strlen(req)); + CHECK(r == 0); + CHECK(handler_calls == 1); + CHECK(seen_headers[0] == '\0'); + } + + /* 6. Headers whose total exceeds HTTP_HEADERS_LEN while each individual + * line stays under it. The existing length check bounds a single line, + * not the running total, so accumulating without a total bound would + * overflow req.headers here. Either outcome is acceptable - reject the + * request, or truncate - as long as the field stays NUL-terminated + * within its own storage and the parser does not run off the end. */ + { + char req[HTTP_RECV_BUF_LEN]; + size_t off = 0; + int i; + off += (size_t)snprintf(req + off, sizeof(req) - off, + "GET /probe HTTP/1.1\r\n"); + for (i = 0; i < 20; i++) { + /* ~55 bytes per line * 20 = ~1100 bytes total, each line well + * under the 1024-byte HTTP_HEADERS_LEN cap. */ + off += (size_t)snprintf(req + off, sizeof(req) - off, + "X-Filler-%02d: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\r\n", + i); + } + off += (size_t)snprintf(req + off, sizeof(req) - off, "\r\n"); + r = run(&httpd, req, off); + if (r == 0) { + CHECK(handler_calls == 1); + CHECK(seen_headers_len < HTTP_HEADERS_LEN); + } else { + CHECK(handler_calls == 0); + } + } + + if (failures == 0) + printf("test_http_headers: all checks passed\n"); + else + printf("test_http_headers: %d check(s) failed\n", failures); + return failures ? 1 : 0; +} diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 2bc155af..c347ab1a 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -299,6 +299,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_ip_recv_drops_source_routed_packet); tcase_add_test(tc_utils, test_ip_recv_drops_ssrr_source_routed_packet); tcase_add_test(tc_utils, test_ip_recv_drops_source_routed_packet_overlong_first_option_hides_lsrr); + tcase_add_test(tc_utils, test_ip_recv_drops_source_routed_packet_undersized_option_hides_ssrr); tcase_add_test(tc_utils, test_sock_sendto_error_paths); tcase_add_test(tc_utils, test_sock_sendto_null_buf_or_len_zero); tcase_add_test(tc_utils, test_sock_sendto_tcp_not_established); @@ -449,6 +450,10 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_dns_callback_bad_flags); tcase_add_test(tc_utils, test_dns_callback_truncated_response_aborts_query); tcase_add_test(tc_utils, test_dns_inflight_query_state_not_clobbered_by_second_call); + tcase_add_test(tc_utils, test_dns_callback_mismatched_question_name_rejected); + tcase_add_test(tc_utils, test_dns_callback_mismatched_question_type_class_rejected); + tcase_add_test(tc_utils, test_dns_callback_missing_question_section_rejected); + tcase_add_test(tc_utils, test_dns_query_source_port_rotates_between_queries); tcase_add_test(tc_utils, test_regression_dns_callback_high_bit_octet_ip_no_ub); tcase_add_test(tc_utils, test_dns_callback_bad_name); tcase_add_test(tc_utils, test_dns_callback_short_header_ignored); @@ -551,7 +556,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_sock_close_tcp_close_wait_full_txbuf_preserves_state); tcase_add_test(tc_utils, test_sock_close_tcp_other_state_closes); tcase_add_test(tc_utils, test_sock_close_tcp_cancels_rto_timer); - tcase_add_test(tc_utils, test_sock_close_tcp_closed_returns_minus_one); + tcase_add_test(tc_utils, test_sock_close_tcp_closed_releases_slot); + tcase_add_test(tc_utils, test_sock_close_tcp_closed_frees_pool_slot); + tcase_add_test(tc_utils, test_sock_close_tcp_closed_reaps_deferred_notify); tcase_add_test(tc_utils, test_tcp_syn_sent_to_established); tcase_add_test(tc_utils, test_tcp_input_syn_sent_unexpected_flags); tcase_add_test(tc_utils, test_tcp_input_syn_sent_synack_transitions); @@ -860,6 +867,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_loopback_dest_not_forwarded); tcase_add_test(tc_proto, test_regression_forwarding_rpf_drops_spoofed_source); tcase_add_test(tc_proto, test_regression_forwarding_drops_source_routed_packet); + tcase_add_test(tc_proto, test_regression_forwarding_drops_source_route_behind_undersized_option); tcase_add_test(tc_proto, test_regression_loopback_source_dropped_on_non_loopback_iface); tcase_add_test(tc_proto, test_regression_icmp_echo_request_non_local_dst_no_reply); tcase_add_test(tc_proto, test_tcp_listen_rejects_wrong_interface); @@ -931,6 +939,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_fast_recovery_cwnd_ssthresh_rfc5681); tcase_add_test(tc_proto, test_regression_paws_rejects_stale_timestamp); tcase_add_test(tc_proto, test_regression_paws_accepts_wrapped_newer_timestamp); + tcase_add_test(tc_proto, test_regression_paws_drops_segment_without_timestamp_option); + tcase_add_test(tc_proto, test_regression_paws_drops_last_ack_segment_without_timestamp_option); + tcase_add_test(tc_proto, test_regression_paws_drops_time_wait_segment_without_timestamp_option); tcase_add_test(tc_proto, test_regression_dhcp_nak_restarts_configuration); tcase_add_test(tc_proto, test_regression_dhcp_boot_request_nak_ignored); tcase_add_test(tc_proto, test_regression_dns_rcode_error_aborts_query); @@ -1329,7 +1340,7 @@ Suite *wolf_suite(void) #ifdef IP_MULTICAST tcase_add_test(tc_core, test_poll_tx_udp_multicast_arp_skipped_uses_mcast_mac); #endif /* IP_MULTICAST */ - /* --- unit_tests_dhcp_edges.c (46 tests) --- */ + /* --- unit_tests_dhcp_edges.c (52 tests) --- */ tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_zero_lease_noop); tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_null_noop); tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_renew_gt_lease_clamped); @@ -1378,8 +1389,11 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_timer_cb_null_arg_noop); tcase_add_test(tc_core, test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack); tcase_add_test(tc_core, test_dhcp_parse_ack_without_lease_time_rejected); + tcase_add_test(tc_core, test_dhcp_lease_expiry_relearns_dns_server); + tcase_add_test(tc_core, test_dhcp_nak_relearns_dns_server); + tcase_add_test(tc_core, test_dhcp_lease_expiry_keeps_pinned_dns_server); tcase_add_test(tc_core, test_dhcp_public_apis_null_stack_safe); - /* --- unit_tests_ip_arp_recv.c (34 tests) --- */ + /* --- unit_tests_ip_arp_recv.c (35 tests) --- */ tcase_add_test(tc_core, test_ip_recv_limited_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_directed_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_ipaddr_any_dst_is_local); @@ -1387,6 +1401,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_ip_recv_forward_unconfigured_iface_skipped); tcase_add_test(tc_core, test_ip_recv_forward_link_local_src_rpf_drop); tcase_add_test(tc_core, test_ip_recv_forward_self_ip_src_dropped); + tcase_add_test(tc_core, test_ip_recv_l2_broadcast_frame_not_forwarded); tcase_add_test(tc_core, test_ip_recv_options_nop_delivered); tcase_add_test(tc_core, test_ip_recv_options_rr_stripped_and_delivered); tcase_add_test(tc_core, test_ip_recv_options_bad_length_aborts_parse); @@ -1566,6 +1581,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_vlan_rx_delete_then_dropped); tcase_add_test(tc_proto, test_vlan_rx_dei_bit_accepted); tcase_add_test(tc_proto, test_vlan_rx_tagged_arp_processed); + tcase_add_test(tc_proto, test_vlan_rx_eth_filter_notified_on_subif_after_strip); + tcase_add_test(tc_proto, test_vlan_rx_eth_filter_deny_subif_blocks_arp_poisoning); + tcase_add_test(tc_proto, test_vlan_rx_unfiltered_arp_reply_learns_neighbor); tcase_add_test(tc_proto, test_vlan_mtu_inherited); tcase_add_test(tc_proto, test_vlan_delete_purges_arp_neighbor_cache); #if WOLFIP_PACKET_SOCKETS diff --git a/src/test/unit/unit_shared.c b/src/test/unit/unit_shared.c index 615e875e..a19421c0 100644 --- a/src/test/unit/unit_shared.c +++ b/src/test/unit/unit_shared.c @@ -676,6 +676,45 @@ static int enqueue_tcp_tx_with_payload(struct tsocket *ts, const uint8_t *payloa return fifo_push(&ts->sock.tcp.txbuf, tcp, frame_len); } +/* Wire-format QNAMEs (length-prefixed labels + terminator) matching the + * question sections the DNS tests build into their synthetic responses. */ +static const uint8_t dns_qname_example_com[] = + {7,'e','x','a','m','p','l','e',3,'c','o','m',0}; +static const uint8_t dns_qname_a[] = {1,'a',0}; +static const uint8_t dns_qname_a_com[] = {1,'a',3,'c','o','m',0}; + +/* Arm an outstanding DNS query the way dns_send_query() would, for tests that + * synthesize a response directly instead of going through nslookup(). Since + * dns_callback() verifies the response's question section against + * s->dns_query_buf (RFC 1035 s7.3), a hand-set dns_id alone is no longer + * enough to reach the answer section. qname is the wire-format name + * (length-prefixed labels + terminator) the test builds into its response. */ +static void arm_dns_query(struct wolfIP *s, uint16_t id, const uint8_t *qname, + int qname_len, uint16_t qtype) +{ + struct dns_header *hdr = (struct dns_header *)s->dns_query_buf; + struct dns_question *q; + int pos = (int)sizeof(struct dns_header); + + ck_assert_uint_le((size_t)(pos + qname_len + (int)sizeof(struct dns_question)), + sizeof(s->dns_query_buf)); + memset(s->dns_query_buf, 0, sizeof(s->dns_query_buf)); + hdr->id = ee16(id); + hdr->flags = ee16(DNS_QUERY | DNS_RD); + hdr->qdcount = ee16(DNS_QUESTION_COUNT); + memcpy(s->dns_query_buf + pos, qname, (size_t)qname_len); + pos += qname_len; + q = (struct dns_question *)(s->dns_query_buf + pos); + q->qtype = ee16(qtype); + q->qclass = ee16(DNS_CLASS_IN); + pos += (int)sizeof(struct dns_question); + + s->dns_query_len = (uint16_t)pos; + s->dns_id = id; + s->dns_query_type = (qtype == DNS_PTR) ? DNS_QUERY_TYPE_PTR : + DNS_QUERY_TYPE_A; +} + static void enqueue_udp_rx(struct tsocket *ts, const void *payload, uint16_t payload_len, uint16_t src_port) { uint8_t buf[sizeof(struct wolfIP_udp_datagram) + 1024]; diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 360c9d18..8d621ec7 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -4926,3 +4926,92 @@ START_TEST(test_ip_recv_drops_source_routed_packet_overlong_first_option_hides_l ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN); } END_TEST + +/* An option carrying an illegal length byte (< 2) must not end the option scan + * early and let a source route behind it reach a local socket. */ +START_TEST(test_ip_recv_drops_source_routed_packet_undersized_option_hides_ssrr) +{ + struct wolfIP s; + int listen_sd; + struct tsocket *listener; + struct wolfIP_sockaddr_in sin; + uint8_t pkt[ETH_HEADER_LEN + 32 + TCP_HEADER_LEN]; + struct wolfIP_ip_packet *ip; + struct wolfIP_ll_dev *ll; + union transport_pseudo_header ph; + uint16_t *tcp_csum_field; + static const uint8_t src_mac[6] = {0x10, 0x20, 0x30, 0x40, 0x50, 0x60}; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + listen_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(listen_sd, 0); + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_sd, 1), 0); + listener = &s.tcpsockets[SOCKET_UNMARK(listen_sd)]; + + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + memset(pkt, 0, sizeof(pkt)); + + ip = (struct wolfIP_ip_packet *)pkt; + memcpy(ip->eth.dst, ll->mac, 6); + memcpy(ip->eth.src, src_mac, 6); + ip->eth.type = ee16(ETH_TYPE_IP); + + /* IHL=8 for 12 bytes of IP options. */ + ip->ver_ihl = 0x48; + ip->ttl = 64; + ip->proto = WI_IPPROTO_TCP; + ip->len = ee16(32 + TCP_HEADER_LEN); + ip->src = ee32(0x0A0000A1U); + ip->dst = ee32(0x0A000001U); + + { + uint8_t *opts = pkt + ETH_HEADER_LEN + IP_HEADER_LEN; + opts[0] = 0x44; /* Timestamp */ + opts[1] = 1; /* illegal: an option is at least type + length */ + opts[2] = 0x89; /* SSRR hidden behind the undersized stub */ + opts[3] = 7; /* well formed: type + length + pointer + one route */ + opts[4] = 4; /* pointer (RFC 791) */ + opts[5] = 10; opts[6] = 0; opts[7] = 0; opts[8] = 9; + opts[9] = 0x00; /* End of Options padding */ + opts[10] = 0x00; + opts[11] = 0x00; + } + ip->csum = 0; + iphdr_set_checksum(ip); + + { + uint8_t *tcp = pkt + ETH_HEADER_LEN + 32; + tcp[0] = (uint8_t)(40000 >> 8); + tcp[1] = (uint8_t)(40000 & 0xFF); + tcp[2] = (uint8_t)(1234 >> 8); + tcp[3] = (uint8_t)(1234 & 0xFF); + tcp[4] = 0; tcp[5] = 0; tcp[6] = 0; tcp[7] = 1; + tcp[12] = TCP_HEADER_LEN << 2; + tcp[13] = TCP_FLAG_SYN; + tcp[14] = 0xFF; tcp[15] = 0xFF; + tcp_csum_field = (uint16_t *)(tcp + 16); + *tcp_csum_field = 0; + memset(&ph, 0, sizeof(ph)); + ph.ph.src = ip->src; + ph.ph.dst = ip->dst; + ph.ph.proto = WI_IPPROTO_TCP; + ph.ph.len = ee16(TCP_HEADER_LEN); + *tcp_csum_field = ee16(transport_checksum(&ph, tcp)); + } + + ip_recv(&s, TEST_PRIMARY_IF, ip, sizeof(pkt)); + + /* Listener must stay in LISTEN: with the buggy parser the walk breaks out + * on the length-1 stub, the SSRR behind it is never examined and the SYN + * flips the listener to TCP_SYN_RCVD. */ + ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN); +} +END_TEST diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index f167bb93..4de69593 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1239,6 +1239,151 @@ START_TEST(test_dhcp_parse_ack_without_lease_time_rejected) } END_TEST +/* A lease that expires drops the DNS server along with the address, mask and + * gateway, so the fresh DORA adopts the resolver advertised by whichever server + * answers next. */ +START_TEST(test_dhcp_lease_expiry_relearns_dns_server) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + const uint32_t first_srv = 0x0A0000FEU; /* 10.0.0.254 */ + const uint32_t first_dns = 0x0A0000FEU; + const uint32_t second_srv = 0x0A000001U; /* 10.0.0.1 */ + const uint32_t second_dns = 0x08080808U; /* 8.8.8.8 */ + const uint32_t client_ip = 0x0A000064U; + const uint32_t mask = 0xFFFFFF00U; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + /* First lease: the resolver comes from the server that answered first. */ + s.dhcp_xid = 0x11110001U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + build_full_ack(&s, &msg, first_srv, client_ip, mask, first_srv, + first_dns, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + ck_assert_uint_eq(s.dns_server, first_dns); + ck_assert_uint_ne(s.dhcp_lease_expires, 0U); + + /* The lease expires: every parameter it carried is released. */ + s.last_tick = s.dhcp_lease_expires; + dhcp_timer_cb(&s); + ck_assert_int_eq(s.dhcp_state, DHCP_DISCOVER_SENT); + ck_assert_uint_eq(primary->ip, 0U); + ck_assert_uint_eq(primary->gw, 0U); + ck_assert_uint_eq(s.dhcp_ip, 0U); + ck_assert_uint_eq(s.dhcp_server_ip, 0U); + ck_assert_uint_eq(s.dns_server, 0U); + + /* The next lease installs its own resolver, not the released one. */ + s.dhcp_xid = 0x22220002U; + s.dhcp_state = DHCP_REQUEST_SENT; + build_full_ack(&s, &msg, second_srv, client_ip, mask, second_srv, + second_dns, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + ck_assert_uint_eq(s.dhcp_server_ip, second_srv); + ck_assert_uint_eq(s.dns_server, second_dns); +} +END_TEST + +/* A DHCPNAK drops the DNS server along with the rest of the lease. */ +START_TEST(test_dhcp_nak_relearns_dns_server) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + uint8_t *p; + const uint32_t first_srv = 0x0A0000FEU; + const uint32_t first_dns = 0x0A0000FEU; + const uint32_t client_ip = 0x0A000064U; + const uint32_t mask = 0xFFFFFF00U; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + s.dhcp_xid = 0x33330003U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + build_full_ack(&s, &msg, first_srv, client_ip, mask, first_srv, + first_dns, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_uint_eq(s.dns_server, first_dns); + + /* RFC 2131 s4.4.1: a DHCPNAK restarts configuration from scratch. */ + s.dhcp_state = DHCP_RENEWING; + build_dhcp_msg_base(&s, &msg, DHCP_NAK); + p = (uint8_t *)msg.options + 3; + append_opt4(&p, DHCP_OPTION_SERVER_ID, first_srv); + append_end(&p); + ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), DHCP_NAK); + dhcp_deconfigure_lease(&s); + + ck_assert_uint_eq(primary->ip, 0U); + ck_assert_uint_eq(s.dhcp_server_ip, 0U); + ck_assert_uint_eq(s.dns_server, 0U); +} +END_TEST + +/* A resolver configured out-of-band survives a lease loss and is never replaced + * by the one a DHCPACK advertises. */ +START_TEST(test_dhcp_lease_expiry_keeps_pinned_dns_server) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + const uint32_t server_ip = 0x0A000001U; + const uint32_t pinned_dns = 0x09090909U; /* 9.9.9.9, set out-of-band */ + const uint32_t offered_dns = 0x08080808U; + const uint32_t client_ip = 0x0A000064U; + const uint32_t mask = 0xFFFFFF00U; + + wolfIP_init(&s); + mock_link_init(&s); + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + s.dns_server = pinned_dns; + s.dns_server_pinned = 1; + + s.dhcp_xid = 0x44440004U; + s.dhcp_state = DHCP_REQUEST_SENT; + s.last_tick = 1000U; + build_full_ack(&s, &msg, server_ip, client_ip, mask, server_ip, + offered_dns, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_uint_eq(s.dns_server, pinned_dns); + + s.last_tick = s.dhcp_lease_expires; + dhcp_timer_cb(&s); + ck_assert_uint_eq(primary->ip, 0U); + ck_assert_uint_eq(s.dns_server, pinned_dns); + + /* The next lease does not get to install its resolver either. */ + s.dhcp_xid = 0x55550005U; + s.dhcp_state = DHCP_REQUEST_SENT; + build_full_ack(&s, &msg, server_ip, client_ip, mask, server_ip, + offered_dns, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_uint_eq(s.dns_server, pinned_dns); +} +END_TEST + /* F-5485: the public DHCP helper APIs must tolerate a NULL stack pointer and * return a deterministic value instead of dereferencing it. */ START_TEST(test_dhcp_public_apis_null_stack_safe) diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index e5aafa52..1f93cce5 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -4888,7 +4888,7 @@ START_TEST(test_sock_close_tcp_cancels_rto_timer) } END_TEST -START_TEST(test_sock_close_tcp_closed_returns_minus_one) +START_TEST(test_sock_close_tcp_closed_releases_slot) { struct wolfIP s; struct tsocket *ts; @@ -4902,7 +4902,66 @@ START_TEST(test_sock_close_tcp_closed_returns_minus_one) ts = &s.tcpsockets[SOCKET_UNMARK(sd)]; ts->sock.tcp.state = TCP_CLOSED; - ck_assert_int_eq(wolfIP_sock_close(&s, sd), -1); + ck_assert_int_eq(wolfIP_sock_close(&s, sd), 0); + ck_assert_int_eq(ts->proto, 0); +} +END_TEST + +/* Closing a never-connected TCP socket returns its pool slot, so repeated + * socket()/close() cycles do not exhaust the pool. */ +START_TEST(test_sock_close_tcp_closed_frees_pool_slot) +{ + struct wolfIP s; + int sd; + int i; + + wolfIP_init(&s); + mock_link_init(&s); + + for (i = 0; i < MAX_TCPSOCKETS; i++) { + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(sd, 0); + ck_assert_int_eq(s.tcpsockets[SOCKET_UNMARK(sd)].sock.tcp.state, TCP_CLOSED); + ck_assert_int_eq(wolfIP_sock_close(&s, sd), 0); + ck_assert_int_eq(s.tcpsockets[SOCKET_UNMARK(sd)].proto, 0); + } + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(sd, 0); +} +END_TEST + +/* An app-initiated close of a slot holding a deferred CB_EVENT_CLOSED reaps it + * without dispatching the callback. */ +START_TEST(test_sock_close_tcp_closed_reaps_deferred_notify) +{ + struct wolfIP s; + struct tsocket *ts; + int sd; + int callback_arg = 0; + + wolfIP_init(&s); + mock_link_init(&s); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP); + ck_assert_int_gt(sd, 0); + ts = &s.tcpsockets[SOCKET_UNMARK(sd)]; + ts->sock.tcp.state = TCP_ESTABLISHED; + wolfIP_register_callback(&s, sd, test_socket_cb, &callback_arg); + + /* Involuntary teardown from the RX path defers one final CB_EVENT_CLOSED. */ + close_socket(ts); + ck_assert_uint_eq(ts->close_notify_pending, 1); + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSED); + ck_assert_int_ne(ts->proto, 0); + + socket_cb_calls = 0; + ck_assert_int_eq(wolfIP_sock_close(&s, sd), 0); + ck_assert_int_eq(ts->proto, 0); + ck_assert_uint_eq(ts->close_notify_pending, 0); + + (void)wolfIP_poll(&s, 1); + ck_assert_int_eq(socket_cb_calls, 0); } END_TEST START_TEST(test_fifo_push_and_pop_multiple) { @@ -5184,8 +5243,8 @@ START_TEST(test_dns_callback_ptr_response) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_PTR; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_a_com, (int)sizeof(dns_qname_a_com), + DNS_PTR); s.dns_ptr_cb = test_dns_ptr_cb; s.dns_lookup_cb = NULL; s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); @@ -5966,6 +6025,259 @@ START_TEST(test_dns_inflight_query_state_not_clobbered_by_second_call) } END_TEST +/* + * Helper that writes the bytes of a fake DNS reply into buf and returns + * how many bytes it wrote. + * */ +static int build_dns_a_response_for_question(uint8_t *buf, size_t buf_sz, + uint16_t id, + const uint8_t *qname, int qname_len, + uint16_t qtype, uint16_t qclass, + uint16_t qdcount, + const uint8_t *ip_bytes) +{ + struct dns_header *hdr = (struct dns_header *)buf; + struct dns_question *q; + struct dns_rr *rr; + int pos; + int qname_off; + + memset(buf, 0, buf_sz); + hdr->id = ee16(id); + hdr->flags = ee16(DNS_FLAGS_RESPONSE_RD); /* QR|RD, RCODE 0, TC clear */ + hdr->qdcount = ee16(qdcount); + hdr->ancount = ee16(1); + pos = (int)sizeof(struct dns_header); + qname_off = pos; + + memcpy(buf + pos, qname, (size_t)qname_len); + pos += qname_len; + if (qdcount != 0) { + q = (struct dns_question *)(buf + pos); + q->qtype = ee16(qtype); + q->qclass = ee16(qclass); + pos += (int)sizeof(struct dns_question); + buf[pos++] = DNS_COMPRESSION_PTR_VALUE; + buf[pos++] = (uint8_t)qname_off; + } + + rr = (struct dns_rr *)(buf + pos); + rr->type = ee16(DNS_A); + rr->class = ee16(DNS_CLASS_IN); + rr->ttl = ee32(60); + rr->rdlength = ee16(DNS_IPV4_RDATA_LEN); + pos += (int)sizeof(struct dns_rr); + memcpy(buf + pos, ip_bytes, DNS_IPV4_RDATA_LEN); + pos += DNS_IPV4_RDATA_LEN; + + ck_assert_uint_le((size_t)pos, buf_sz); + return pos; +} + +/* RFC 1035 s7.3: matching the header ID is only the *preliminary* check. The + * resolver must then "verify that the question section corresponds to the + * information currently desired". + * Dropping the mismatch must not cancel the query. Aborting on an unmatched + * response would let one spoofed packet deny the lookup, so the resolver has + * to stay armed for the legitimate answer still in flight. */ +START_TEST(test_dns_callback_mismatched_question_name_rejected) +{ + struct wolfIP s; + uint16_t id = 0; + uint8_t response[128]; + int len; + static const uint8_t qname_evil[] = {4,'e','v','i','l',3,'c','o','m',0}; + static const uint8_t qname_target[] = {6,'t','a','r','g','e','t',3,'c','o','m',0}; + static const uint8_t evil_ip[DNS_IPV4_RDATA_LEN] = {0x06, 0x06, 0x06, 0x06}; + static const uint8_t good_ip[DNS_IPV4_RDATA_LEN] = {0x01, 0x02, 0x03, 0x04}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + dns_lookup_calls = 0; + dns_lookup_ip = 0; + + /* Real query: "target.com" IN A. dns_send_query() stores it in + * s->dns_query_buf, which is the reference the response must match. */ + ck_assert_int_eq(nslookup(&s, "target.com", &id, test_dns_lookup_cb), 0); + ck_assert_uint_ne(id, 0U); + + /* Well-formed, right ID, RCODE 0, one A/IN answer but the question is + * for a name this resolver never queried. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_evil, (int)sizeof(qname_evil), DNS_A, DNS_CLASS_IN, 1, + evil_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 0); + ck_assert_uint_eq(dns_lookup_ip, 0U); + /* Still armed for the real answer. */ + ck_assert_uint_eq(s.dns_id, id); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + + /* The response that does match the outstanding question still resolves. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_target, (int)sizeof(qname_target), DNS_A, DNS_CLASS_IN, 1, + good_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 1); + ck_assert_uint_eq(dns_lookup_ip, 0x01020304U); + ck_assert_uint_eq(s.dns_id, 0U); +} +END_TEST + +/* Similar test to the one above but it's reached through + * QTYPE and QCLASS instead of the name. */ +START_TEST(test_dns_callback_mismatched_question_type_class_rejected) +{ + struct wolfIP s; + uint16_t id = 0; + uint8_t response[128]; + int len; + static const uint8_t qname_target[] = {6,'t','a','r','g','e','t',3,'c','o','m',0}; + static const uint8_t evil_ip[DNS_IPV4_RDATA_LEN] = {0x06, 0x06, 0x06, 0x06}; + const uint16_t dns_class_ch = 0x0003; /* CHAOS, RFC 1035 s3.2.4 */ + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + dns_lookup_calls = 0; + dns_lookup_ip = 0; + + ck_assert_int_eq(nslookup(&s, "target.com", &id, test_dns_lookup_cb), 0); + + /* Right name, but with wrong QTYPE. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_target, (int)sizeof(qname_target), DNS_PTR, DNS_CLASS_IN, 1, + evil_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 0); + ck_assert_uint_eq(s.dns_id, id); + + /* Right name and QTYPE but with wrong QCLASS. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_target, (int)sizeof(qname_target), DNS_A, dns_class_ch, 1, + evil_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 0); + ck_assert_uint_eq(dns_lookup_ip, 0U); + ck_assert_uint_eq(s.dns_id, id); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); +} +END_TEST + +/* Part of the same checks as above, in this case the skip loop is driven + * by the response's qdcount, so a response can declare qdcount == 0, + * place its answers directly after the header and skip the question + * section altogether. */ +START_TEST(test_dns_callback_missing_question_section_rejected) +{ + struct wolfIP s; + uint16_t id = 0; + uint8_t response[128]; + int len; + static const uint8_t qname_target[] = {6,'t','a','r','g','e','t',3,'c','o','m',0}; + static const uint8_t evil_ip[DNS_IPV4_RDATA_LEN] = {0x06, 0x06, 0x06, 0x06}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + dns_lookup_calls = 0; + dns_lookup_ip = 0; + + ck_assert_int_eq(nslookup(&s, "target.com", &id, test_dns_lookup_cb), 0); + + /* qdcount == 0 so no question at all, answer RR owner name is the queried + * name so only the question-count check can reject this. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_target, (int)sizeof(qname_target), DNS_A, DNS_CLASS_IN, 0, + evil_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 0); + ck_assert_uint_eq(dns_lookup_ip, 0U); + ck_assert_uint_eq(s.dns_id, id); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); +} +END_TEST + +/* RFC 5452 s9.2: the resolver must randomise the query source port, so that an + * off-path attacker has to guess the port and the 16-bit ID to have a forged + * reply accepted. */ +START_TEST(test_dns_query_source_port_rotates_between_queries) +{ + struct wolfIP s; + uint16_t id = 0; + uint8_t response[128]; + int len; + unsigned int i, j; + uint16_t ports[4]; + static const uint32_t rand_per_query[4] = {0x2000U, 0x3000U, 0x4000U, 0x5000U}; + static const uint8_t qname_target[] = {6,'t','a','r','g','e','t',3,'c','o','m',0}; + static const uint8_t good_ip[DNS_IPV4_RDATA_LEN] = {0x01, 0x02, 0x03, 0x04}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + dns_lookup_calls = 0; + dns_lookup_ip = 0; + test_rand_override_enabled = 1; + + for (i = 0; i < 4; i++) { + /* Low 16 bits are >= 1024 and distinct, so each query gets its own + * candidate ID and its own candidate source port. */ + test_rand_override_value = rand_per_query[i]; + + ck_assert_int_eq(nslookup(&s, "target.com", &id, test_dns_lookup_cb), 0); + /* Re-read the socket every round: a fix is free to close and reopen it, + * which may land the DNS socket on a different table slot. */ + ck_assert_int_gt(s.dns_udp_sd, 0); + ports[i] = s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)].src_port; + + /* Complete the lookup the way the real resolver does, so the next + * iteration starts from an idle resolver rather than a hand-cleared + * one. dns_callback() calls dns_abort_query() on success. */ + len = build_dns_a_response_for_question(response, sizeof(response), id, + qname_target, (int)sizeof(qname_target), DNS_A, DNS_CLASS_IN, 1, + good_ip); + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)len, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, (int)i + 1); + ck_assert_uint_eq(dns_lookup_ip, 0x01020304U); + ck_assert_uint_eq(s.dns_id, 0U); + } + + /* Each query must leave from a port of its own. */ + for (i = 0; i < 4; i++) { + ck_assert_uint_ge(ports[i], 1024U); + for (j = 0; j < i; j++) + ck_assert_uint_ne(ports[i], ports[j]); + } + + test_rand_override_enabled = 0; +} +END_TEST + START_TEST(test_regression_dns_callback_high_bit_octet_ip_no_ub) { /* The dns_callback() A-record reassembly used to compute @@ -5988,8 +6300,8 @@ START_TEST(test_regression_dns_callback_high_bit_octet_ip_no_ub) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_A; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); s.dns_lookup_cb = test_dns_lookup_cb; dns_lookup_calls = 0; dns_lookup_ip = 0; @@ -6155,8 +6467,8 @@ START_TEST(test_dns_callback_non_in_a_answer_ignored) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_A; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); s.dns_lookup_cb = test_dns_lookup_cb; dns_lookup_calls = 0; dns_lookup_ip = 0; @@ -6210,8 +6522,8 @@ START_TEST(test_dns_callback_non_in_ptr_answer_ignored) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_PTR; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_a_com, (int)sizeof(dns_qname_a_com), + DNS_PTR); s.dns_ptr_cb = test_dns_ptr_cb; s.dns_lookup_cb = NULL; s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); @@ -6262,8 +6574,7 @@ START_TEST(test_dns_callback_malformed_compressed_name_aborts_query) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_A; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_a, (int)sizeof(dns_qname_a), DNS_A); s.dns_lookup_cb = test_dns_lookup_cb; dns_lookup_calls = 0; dns_lookup_ip = 0; diff --git a/src/test/unit/unit_tests_dns_edges.c b/src/test/unit/unit_tests_dns_edges.c index 720fa363..f467d875 100644 --- a/src/test/unit/unit_tests_dns_edges.c +++ b/src/test/unit/unit_tests_dns_edges.c @@ -129,8 +129,8 @@ START_TEST(test_dns_callback_zero_ancount_no_delivery) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_id = 0xABCD; - s.dns_query_type = DNS_QUERY_TYPE_A; + arm_dns_query(&s, 0xABCD, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); dns_lookup_calls = 0; dns_lookup_ip = 0; s.dns_lookup_cb = test_dns_lookup_cb; @@ -165,8 +165,8 @@ START_TEST(test_dns_callback_aaaa_answer_skipped_for_a_query) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_id = 0x1111; - s.dns_query_type = DNS_QUERY_TYPE_A; + arm_dns_query(&s, 0x1111, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); dns_lookup_calls = 0; dns_lookup_ip = 0; s.dns_lookup_cb = test_dns_lookup_cb; @@ -210,8 +210,8 @@ START_TEST(test_dns_callback_rr_rdlen_truncated_aborts_query) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_id = 0x2222; - s.dns_query_type = DNS_QUERY_TYPE_A; + arm_dns_query(&s, 0x2222, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); dns_lookup_calls = 0; dns_lookup_ip = 0; s.dns_lookup_cb = test_dns_lookup_cb; @@ -297,8 +297,7 @@ START_TEST(test_dns_callback_answer_forward_ptr_aborts_query) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_id = 0x4444; - s.dns_query_type = DNS_QUERY_TYPE_A; + arm_dns_query(&s, 0x4444, dns_qname_a, (int)sizeof(dns_qname_a), DNS_A); s.dns_lookup_cb = test_dns_lookup_cb; s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); ck_assert_int_gt(s.dns_udp_sd, 0); @@ -565,8 +564,7 @@ START_TEST(test_dns_callback_ptr_bad_copy_name_stays_pending) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_id = 0xBBBB; - s.dns_query_type = DNS_QUERY_TYPE_PTR; + arm_dns_query(&s, 0xBBBB, dns_qname_a, (int)sizeof(dns_qname_a), DNS_PTR); s.dns_ptr_cb = test_dns_ptr_cb; s.dns_lookup_cb = NULL; s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); diff --git a/src/test/unit/unit_tests_ip_arp_recv.c b/src/test/unit/unit_tests_ip_arp_recv.c index 18a2771b..e2705a87 100644 --- a/src/test/unit/unit_tests_ip_arp_recv.c +++ b/src/test/unit/unit_tests_ip_arp_recv.c @@ -381,6 +381,64 @@ START_TEST(test_ip_recv_forward_self_ip_src_dropped) } END_TEST +/* + * RFC 1812 sec.5.3.4: a datagram received as a link-layer broadcast must + * never be forwarded. + */ +START_TEST(test_ip_recv_l2_broadcast_frame_not_forwarded) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; /* 10.0.0.1 router LAN IP (if1) */ + ip4 secondary_ip = 0xC0A80101U; /* 192.168.1.1 router WAN IP (if2) */ + ip4 dest_ip = 0xC0A80155U; /* 192.168.1.85 local to if2 */ + ip4 src_ip = 0x0A000002U; /* 10.0.0.2 passes every RPF check */ + static const uint8_t dest_mac[6] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15}; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + /* ARP hit so that, absent the drop, the packet would be forwarded now. */ + arp_store_neighbor(&s, TEST_SECOND_IF, dest_ip, dest_mac); + + memset(frame, 0, sizeof(frame)); + /* The only thing that makes this frame illegal to forward. */ + memcpy(ip->eth.dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 64; + ip->proto = WI_IPPROTO_UDP; + ip->len = ee16(IP_HEADER_LEN + UDP_HEADER_LEN); + ip->src = ee32(src_ip); + ip->dst = ee32(dest_ip); /* unicast, not any broadcast address */ + fix_ip_checksum(ip); + { + uint16_t *udp = (uint16_t *)(frame + ETH_HEADER_LEN + IP_HEADER_LEN); + udp[0] = ee16(9999); udp[1] = ee16(53); + udp[2] = ee16(UDP_HEADER_LEN); udp[3] = 0; + } + + last_frame_sent_size = 0; + /* Enter through wolfIP_recv_on: the L2 accept path is part of the bug. */ + wolfIP_recv_on(&s, TEST_PRIMARY_IF, frame, (uint32_t)sizeof(frame)); + + /* Nothing forwarded, and nothing queued behind an ARP resolution. */ + ck_assert_uint_eq(last_frame_sent_size, 0); + ck_assert_uint_eq(s.arp_pending[0].dest, IPADDR_ANY); + + /* The identical packet addressed to the router's MAC is still + * forwarded, so the drop keys on eth.dst and not on anything else. */ + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + ip->ttl = 64; + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_on(&s, TEST_PRIMARY_IF, frame, (uint32_t)sizeof(frame)); + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_mem_eq(last_frame_sent + 0, dest_mac, 6); +} +END_TEST + /* ========================================================================= * ip_recv: IP with NOP options — options parsed, payload delivered * ========================================================================= @@ -497,10 +555,11 @@ START_TEST(test_ip_recv_options_rr_stripped_and_delivered) END_TEST /* ========================================================================= - * ip_recv: option bad length (opt[1] < 2) — parsing aborts (break) + * ip_recv: option bad length (opt[1] < 2) — packet dropped * ========================================================================= - * Branch: opt + 1 >= opt_end || opt[1] < 2 → break - * A malformed option with length=1 must not loop infinitely. + * Branch: opt + 1 >= opt_end || opt[1] < 2 → return + * A malformed option with length=1 must not loop infinitely, and the rest of + * the option area is unparsable from there on, so the packet is discarded. */ START_TEST(test_ip_recv_options_bad_length_aborts_parse) { @@ -543,9 +602,9 @@ START_TEST(test_ip_recv_options_bad_length_aborts_parse) fix_udp_checksum_raw(ip, udp_hdr, udp_len); fix_ip_checksum_with_hlen(ip, (uint16_t)(IP_HEADER_LEN + 4)); - /* Must not crash; packet may or may not be delivered, but parse terminates */ + /* Must not crash or loop; the packet must not be delivered either */ ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); - /* (no assertion on delivery — the goal is no infinite loop / crash) */ + ck_assert_ptr_null(fifo_peek(&ts->sock.udp.rxbuf)); } END_TEST diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 333b88ce..01a279ca 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4091,6 +4091,65 @@ START_TEST(test_regression_forwarding_drops_source_routed_packet) } END_TEST +/* A source route hidden behind an option with an illegal length byte (< 2) must + * still be caught, so the packet is never relayed with its options intact. */ +START_TEST(test_regression_forwarding_drops_source_route_behind_undersized_option) +{ + static const uint8_t opt_types[] = { 0x83U, 0x89U }; /* LSRR, SSRR */ + static const uint8_t src_mac[6] = {0x52, 0x54, 0x00, 0x12, 0x34, 0x56}; + static const uint8_t iface1_mac[6] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x02}; + static const uint8_t next_hop_mac[6] = {0x02, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE}; + static const uint32_t dest_ip = 0xC0A80164U; /* 192.168.1.100 on TEST_SECOND_IF */ + unsigned int i; + + for (i = 0; i < sizeof(opt_types) / sizeof(opt_types[0]); i++) { + struct wolfIP s; + uint8_t frame_buf[ETH_HEADER_LEN + IP_HEADER_LEN + 12]; + struct wolfIP_ip_packet *frame = (struct wolfIP_ip_packet *)frame_buf; + uint8_t *opts = frame_buf + ETH_HEADER_LEN + IP_HEADER_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + mock_link_init_idx(&s, TEST_SECOND_IF, iface1_mac); + wolfIP_ipconfig_set(&s, 0xC0A80001U, 0xFFFFFF00U, 0); + wolfIP_ipconfig_set_ex(&s, TEST_SECOND_IF, 0xC0A80101U, 0xFFFFFF00U, 0); + s.arp.neighbors[0].ip = dest_ip; + s.arp.neighbors[0].if_idx = TEST_SECOND_IF; + memcpy(s.arp.neighbors[0].mac, next_hop_mac, 6); + + memset(frame_buf, 0, sizeof(frame_buf)); + memcpy(frame->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(frame->eth.src, src_mac, 6); + frame->eth.type = ee16(ETH_TYPE_IP); + frame->ver_ihl = 0x48; /* IHL=8, 32-byte header */ + frame->ttl = 64; + frame->proto = WI_IPPROTO_UDP; + frame->len = ee16(IP_HEADER_LEN + 12); + frame->src = ee32(0xC0A800AAU); /* in TEST_PRIMARY_IF subnet, passes RPF */ + frame->dst = ee32(dest_ip); + + opts[0] = 0x44; /* Timestamp */ + opts[1] = 1; /* illegal: an option is at least type + length */ + opts[2] = opt_types[i]; /* source route hidden behind the stub */ + opts[3] = 7; /* type + length + pointer + one route entry */ + opts[4] = 4; /* pointer (RFC 791) */ + opts[5] = 192; opts[6] = 168; opts[7] = 1; opts[8] = 200; + opts[9] = 0x00; /* end-of-options padding */ + opts[10] = 0x00; + opts[11] = 0x00; + fix_ip_checksum_with_hlen(frame, (uint16_t)(IP_HEADER_LEN + 12)); + + memset(last_frame_sent, 0, sizeof(last_frame_sent)); + last_frame_sent_size = 0; + + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + 12)); + + ck_assert_uint_eq(last_frame_sent_size, 0); + } +} +END_TEST + START_TEST(test_regression_loopback_source_dropped_on_non_loopback_iface) { static const ip4 spoofed_loopback_sources[] = { @@ -6551,6 +6610,222 @@ START_TEST(test_regression_paws_accepts_wrapped_newer_timestamp) } END_TEST +/* RFC 7323 §3.2: once timestamps are negotiated, a non-RST segment that + * arrives without a TSopt is silently dropped. */ +START_TEST(test_regression_paws_drops_segment_without_timestamp_option) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t buf[sizeof(struct wolfIP_tcp_seg) + 4]; + struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)buf; + uint8_t payload[4] = {0xDE, 0xAD, 0xBE, 0xEF}; + uint32_t original_ack; + uint32_t tcp_hlen = TCP_HEADER_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + last_frame_sent_count = 0; + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_ESTABLISHED; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->sock.tcp.ts_enabled = 1; + ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + original_ack = ts->sock.tcp.ack; + + /* In-window data segment carrying no options at all, so the header is + * the bare 20 bytes and tcp_parse_options finds no TSopt. */ + memset(buf, 0, sizeof(buf)); + seg->ip.ver_ihl = 0x45; + seg->ip.ttl = 64; + seg->ip.proto = WI_IPPROTO_TCP; + seg->ip.len = ee16(IP_HEADER_LEN + tcp_hlen + sizeof(payload)); + seg->ip.src = ee32(ts->remote_ip); + seg->ip.dst = ee32(ts->local_ip); + seg->dst_port = ee16(ts->src_port); + seg->src_port = ee16(ts->dst_port); + seg->hlen = (uint8_t)(tcp_hlen << 2); + seg->flags = TCP_FLAG_ACK; + seg->seq = ee32(100); /* == rcv_nxt, in-window */ + seg->ack = ee32(ts->sock.tcp.seq); + seg->win = ee16(65535); + + memcpy(seg->data, payload, sizeof(payload)); + fix_tcp_checksums(seg); + + tcp_input(&s, TEST_PRIMARY_IF, seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + tcp_hlen + sizeof(payload))); + + /* Dropped: rcv_nxt does not advance and no payload reaches the receive + * queue. Silently: nothing is transmitted in reply. */ + ck_assert_uint_eq(ts->sock.tcp.ack, original_ack); + ck_assert_uint_eq(queue_len(&ts->sock.tcp.rxbuf), 0); + ck_assert_uint_eq(last_frame_sent_count, 0); +} +END_TEST + +/* RFC 7323 §3.2: LAST_ACK is a synchronized state, so a final ACK that + * arrives without a TSopt is silently dropped instead of closing the + * connection. */ +START_TEST(test_regression_paws_drops_last_ack_segment_without_timestamp_option) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg seg; + uint32_t tcp_hlen = TCP_HEADER_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + last_frame_sent_count = 0; + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_LAST_ACK; + ts->sock.tcp.ack = 100; + ts->sock.tcp.seq = 1000; + ts->sock.tcp.snd_una = 1000; + ts->sock.tcp.last = 999; /* FIN was at seq 999 */ + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->sock.tcp.ts_enabled = 1; + ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + /* In-window ACK covering our FIN, carrying no options at all, so the + * header is the bare 20 bytes and tcp_parse_options finds no TSopt. */ + memset(&seg, 0, sizeof(seg)); + seg.ip.ver_ihl = 0x45; + seg.ip.ttl = 64; + seg.ip.proto = WI_IPPROTO_TCP; + seg.ip.len = ee16(IP_HEADER_LEN + tcp_hlen); + seg.ip.src = ee32(ts->remote_ip); + seg.ip.dst = ee32(ts->local_ip); + seg.dst_port = ee16(ts->src_port); + seg.src_port = ee16(ts->dst_port); + seg.hlen = (uint8_t)(tcp_hlen << 2); + seg.flags = TCP_FLAG_ACK; + seg.seq = ee32(100); /* == rcv_nxt, in-window */ + seg.ack = ee32(1001); /* ACKs the FIN */ + seg.win = ee16(65535); + fix_tcp_checksums(&seg); + + tcp_input(&s, TEST_PRIMARY_IF, &seg, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + tcp_hlen)); + + /* Dropped: the connection must not be torn down by a segment that + * never passed PAWS. Silently: nothing is queued or transmitted. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_LAST_ACK); + ck_assert_ptr_null(fifo_peek(&ts->sock.tcp.txbuf)); + ck_assert_uint_eq(last_frame_sent_count, 0); +} +END_TEST + +/* RFC 7323 §3.2: TIME_WAIT is a synchronized state, so a retransmitted FIN + * that arrives without a TSopt is silently dropped instead of being + * answered with an ACK. */ +START_TEST(test_regression_paws_drops_time_wait_segment_without_timestamp_option) +{ + struct wolfIP s; + struct tsocket *ts; + struct wolfIP_tcp_seg fin_retx; + uint32_t tcp_hlen = TCP_HEADER_LEN; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + last_frame_sent_size = 0; + last_frame_sent_count = 0; + + s.arp.neighbors[0].ip = 0x0A000002U; + s.arp.neighbors[0].if_idx = TEST_PRIMARY_IF; + memcpy(s.arp.neighbors[0].mac, + (uint8_t[]){0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}, 6); + + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->if_idx = TEST_PRIMARY_IF; + ts->sock.tcp.state = TCP_TIME_WAIT; + /* Peer FIN had seq=100; we already advanced rcv_nxt past it. */ + ts->sock.tcp.ack = 101; + ts->sock.tcp.seq = 200; + ts->sock.tcp.snd_una = 200; + ts->sock.tcp.cwnd = TCP_MSS; + ts->sock.tcp.peer_rwnd = TCP_MSS; + ts->sock.tcp.ts_enabled = 1; + ts->sock.tcp.last_ts = ee32(5000); /* TS.Recent = 5000 */ + ts->src_port = 1234; + ts->dst_port = 4321; + ts->local_ip = 0x0A000001U; + ts->remote_ip = 0x0A000002U; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE); + + memset(&fin_retx, 0, sizeof(fin_retx)); + fin_retx.ip.ver_ihl = 0x45; + fin_retx.ip.ttl = 64; + fin_retx.ip.proto = WI_IPPROTO_TCP; + fin_retx.ip.len = ee16(IP_HEADER_LEN + tcp_hlen); + fin_retx.ip.src = ee32(ts->remote_ip); + fin_retx.ip.dst = ee32(ts->local_ip); + fin_retx.dst_port = ee16(ts->src_port); + fin_retx.src_port = ee16(ts->dst_port); + fin_retx.hlen = (uint8_t)(tcp_hlen << 2); + fin_retx.flags = TCP_FLAG_FIN | TCP_FLAG_ACK; + fin_retx.seq = ee32(100); + fin_retx.ack = ee32(ts->sock.tcp.seq); + fin_retx.win = ee16(65535); + fix_tcp_checksums(&fin_retx); + + tcp_input(&s, TEST_PRIMARY_IF, &fin_retx, + (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + tcp_hlen)); + + /* Dropped silently: no ACK is queued for a segment that never passed + * PAWS. The socket stays in TIME_WAIT under its own close timer. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_TIME_WAIT); + ck_assert_ptr_null(fifo_peek(&ts->sock.tcp.txbuf)); + ck_assert_uint_eq(last_frame_sent_count, 0); +} +END_TEST + /* RFC 2131 s4.4.1: if the client receives a DHCPNAK, it must restart * the configuration process. The current code silently ignores NAKs diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 49717d36..0edff9ff 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -326,8 +326,7 @@ START_TEST(test_dns_callback_bad_rr_rdlen) wolfIP_init(&s); mock_link_init(&s); s.dns_server = 0x0A000001U; - s.dns_query_type = DNS_QUERY_TYPE_A; - s.dns_id = 0x1234; + arm_dns_query(&s, 0x1234, dns_qname_a, (int)sizeof(dns_qname_a), DNS_A); s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); ck_assert_int_gt(s.dns_udp_sd, 0); diff --git a/src/test/unit/unit_tests_vlan.c b/src/test/unit/unit_tests_vlan.c index 382807ed..37286477 100644 --- a/src/test/unit/unit_tests_vlan.c +++ b/src/test/unit/unit_tests_vlan.c @@ -184,6 +184,82 @@ static uint32_t inject_tagged_icmp_echo(struct wolfIP *s, unsigned int parent_id return last_frame_sent_size; } +/* Build an untagged Ethernet/IPv4 ARP frame sent by vlan_remote_mac. + * opcode is ARP_REQUEST or ARP_REPLY; buf must hold sizeof(struct arp_packet). */ +static void build_arp_frame(uint8_t *buf, const uint8_t *eth_dst_mac, + uint16_t opcode, ip4 sip, ip4 tip) +{ + struct arp_packet *arp = (struct arp_packet *)buf; + + memset(buf, 0, sizeof(struct arp_packet)); + memcpy(arp->eth.dst, eth_dst_mac, 6); + memcpy(arp->eth.src, vlan_remote_mac, 6); + arp->eth.type = ee16(ETH_TYPE_ARP); + arp->htype = ee16(1); + arp->ptype = ee16(0x0800); + arp->hlen = 6; + arp->plen = 4; + arp->opcode = ee16(opcode); + memcpy(arp->sma, vlan_remote_mac, 6); + arp->sip = ee32(sip); + /* A request leaves tma zeroed; a reply addresses the resolver. */ + if (opcode == ARP_REPLY) + memcpy(arp->tma, eth_dst_mac, 6); + arp->tip = ee32(tip); +} + +/* These are eth-layer filter observation helpers. + * Record every WOLFIP_FILT_RECEIVING notification raised at the ethernet + * layer so a test can assert which interface index and ethertype the policy + * callback was shown, in order. */ +#define VLAN_FILTER_MAX_EVENTS 4 +static unsigned int vlan_filter_if[VLAN_FILTER_MAX_EVENTS]; +static uint16_t vlan_filter_type[VLAN_FILTER_MAX_EVENTS]; +static int vlan_filter_count; +static unsigned int vlan_filter_block_if; + +static int vlan_filter_record_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (!event || event->reason != WOLFIP_FILT_RECEIVING + || event->meta.ip_proto != WOLFIP_FILTER_PROTO_ETH) + return 0; + if (vlan_filter_count < VLAN_FILTER_MAX_EVENTS) { + vlan_filter_if[vlan_filter_count] = event->if_idx; + vlan_filter_type[vlan_filter_count] = event->meta.eth_type; + } + vlan_filter_count++; + return 0; +} + +/* Allow-by-default L2 policy, deny inbound frames on one interface index, + * the way a caller would express "this VLAN is quarantined". */ +static int vlan_filter_block_if_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (!event || event->reason != WOLFIP_FILT_RECEIVING + || event->meta.ip_proto != WOLFIP_FILTER_PROTO_ETH) + return 0; + vlan_filter_count++; + return (event->if_idx == vlan_filter_block_if) ? 1 : 0; +} + +static void vlan_filter_arm(wolfIP_filter_cb cb) +{ + vlan_filter_count = 0; + memset(vlan_filter_if, 0, sizeof(vlan_filter_if)); + memset(vlan_filter_type, 0, sizeof(vlan_filter_type)); + wolfIP_filter_set_callback(cb, NULL); + wolfIP_filter_set_eth_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_RECEIVING)); +} + +static void vlan_filter_disarm(void) +{ + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_eth_mask(0); + wolfIP_filter_set_mask(0); +} + /* ========================================================================= * 1. API edge tests * ========================================================================= */ @@ -1277,6 +1353,123 @@ START_TEST(test_vlan_rx_tagged_arp_processed) } END_TEST +/* A tagged frame raises two eth-layer RECEIVING notifications: the parent + * with the 0x8100 TPID, then the sub-interface with the inner ethertype. */ +START_TEST(test_vlan_rx_eth_filter_notified_on_subif_after_strip) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + uint8_t plain[sizeof(struct arp_packet)]; + uint8_t tagged[sizeof(struct arp_packet) + 4]; + uint32_t tagged_len; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + wolfIP_ipconfig_set_ex(&s, sub_idx, VLAN_SUB100_IP, 0xFFFFFF00U, 0); + + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + phys->send = mock_send; + + build_arp_frame(plain, phys->mac, ARP_REQUEST, VLAN_REMOTE_IP, + VLAN_SUB100_IP); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, sizeof(plain), + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + vlan_filter_arm(vlan_filter_record_cb); + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + vlan_filter_disarm(); + + /* Two views: the tagged wire frame on the parent, then the demuxed + * frame on the sub-interface it was actually delivered to. */ + ck_assert_int_eq(vlan_filter_count, 2); + ck_assert_uint_eq(vlan_filter_if[0], TEST_PRIMARY_IF); + ck_assert_uint_eq(vlan_filter_type[0], ee16(ETH_TYPE_VLAN_8021Q)); + ck_assert_uint_eq(vlan_filter_if[1], sub_idx); + ck_assert_uint_eq(vlan_filter_type[1], ee16(ETH_TYPE_ARP)); +} +END_TEST + +/* An eth filter denying the sub-interface index stops a tagged ARP reply + * from installing a neighbor entry on that sub-interface. */ +START_TEST(test_vlan_rx_eth_filter_deny_subif_blocks_arp_poisoning) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + uint8_t plain[sizeof(struct arp_packet)]; + uint8_t tagged[sizeof(struct arp_packet) + 4]; + uint32_t tagged_len; + uint8_t mac[6]; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + wolfIP_ipconfig_set_ex(&s, sub_idx, VLAN_SUB100_IP, 0xFFFFFF00U, 0); + + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + phys->send = mock_send; + + build_arp_frame(plain, phys->mac, ARP_REPLY, VLAN_REMOTE_IP, + VLAN_SUB100_IP); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, sizeof(plain), + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + vlan_filter_block_if = sub_idx; + vlan_filter_arm(vlan_filter_block_if_cb); + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + vlan_filter_disarm(); + + ck_assert_int_ge(vlan_filter_count, 1); + /* The frame was denied on sub_idx, so no L2 mapping may be learned. */ + memset(mac, 0, sizeof(mac)); + ck_assert_int_eq(wolfIP_arp_lookup_ex(&s, sub_idx, VLAN_REMOTE_IP, mac), -1); +} +END_TEST + +/* With no filter installed, that same tagged ARP reply does install a + * neighbor entry on the sub-interface. */ +START_TEST(test_vlan_rx_unfiltered_arp_reply_learns_neighbor) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + uint8_t plain[sizeof(struct arp_packet)]; + uint8_t tagged[sizeof(struct arp_packet) + 4]; + uint32_t tagged_len; + uint8_t mac[6]; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + wolfIP_ipconfig_set_ex(&s, sub_idx, VLAN_SUB100_IP, 0xFFFFFF00U, 0); + + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + phys->send = mock_send; + + build_arp_frame(plain, phys->mac, ARP_REPLY, VLAN_REMOTE_IP, + VLAN_SUB100_IP); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, sizeof(plain), + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + + memset(mac, 0, sizeof(mac)); + ck_assert_int_eq(wolfIP_arp_lookup_ex(&s, sub_idx, VLAN_REMOTE_IP, mac), 0); + ck_assert_mem_eq(mac, vlan_remote_mac, 6); +} +END_TEST + /* ========================================================================= * 4. MTU test * ========================================================================= */ diff --git a/src/wolfip.c b/src/wolfip.c index a4faae35..e45bb504 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1374,6 +1374,7 @@ struct wolfIP { uint64_t dhcp_lease_expires; /* Lease expiration time */ uint64_t dhcp_start_tick; /* Start time of current DHCP acquisition/renewal */ ip4 dns_server; + uint8_t dns_server_pinned; /* dns_server is configured out-of-band, not leased */ uint16_t dns_id; int dns_udp_sd; uint32_t dns_timer; @@ -4508,6 +4509,30 @@ static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, return 0; } +#define TCP_PAWS_OK 0 +#define TCP_PAWS_DROP 1 +#define TCP_PAWS_ACK_DROP 2 + +/* RFC 7323 §3.2 PAWS: if timestamps were negotiated, reject segments + * that omit the TSopt or carry a stale TSval. */ +static int tcp_paws_check(const struct tsocket *t, + const struct wolfIP_tcp_seg *tcp, uint32_t frame_len) +{ + struct tcp_parsed_opts po; + + if (!t->sock.tcp.ts_enabled || (tcp->flags & TCP_FLAG_RST)) + return TCP_PAWS_OK; + tcp_parse_options(tcp, frame_len, &po); + /* Once TSopt is negotiated the peer must carry it in every + * non-RST segment, so one that arrives without it is + * dropped silently. */ + if (!po.ts_found) + return TCP_PAWS_DROP; + if (tcp_seq_lt(po.ts_val, ee32(t->sock.tcp.last_ts))) + return TCP_PAWS_ACK_DROP; + return TCP_PAWS_OK; +} + /* Apply RFC6298-style implementation bounds to computed RTO (milliseconds). */ static uint32_t tcp_rto_clamp(uint32_t rto_ms) { @@ -5288,6 +5313,11 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, * caused by our final ACK being lost) is to re-ACK so the * peer can complete its close. RST and SYN are filtered out * earlier in tcp_input. */ + { + int paws = tcp_paws_check(t, tcp, frame_len); + if (paws == TCP_PAWS_DROP) + continue; + } tcp_send_ack(t); continue; } else if (t->sock.tcp.state == TCP_LAST_ACK) { @@ -5297,6 +5327,13 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, tcp_send_ack(t); continue; } + { + int paws = tcp_paws_check(t, tcp, frame_len); + if (paws == TCP_PAWS_ACK_DROP) + tcp_send_ack(t); + if (paws != TCP_PAWS_OK) + continue; + } /* RFC 9293 §3.10.7.4: if the SYN bit is set on a * synchronized connection, send a challenge ACK and * drop the segment (RFC 5961). */ @@ -5322,17 +5359,12 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, continue; } - /* RFC 7323 §3.2 PAWS: if timestamps were negotiated, - * reject segments with stale TSval (send ACK, drop). */ - if (t->sock.tcp.ts_enabled && - !(tcp->flags & TCP_FLAG_RST)) { - struct tcp_parsed_opts po; - tcp_parse_options(tcp, frame_len, &po); - if (po.ts_found && - tcp_seq_lt(po.ts_val, ee32(t->sock.tcp.last_ts))) { + { + int paws = tcp_paws_check(t, tcp, frame_len); + if (paws == TCP_PAWS_ACK_DROP) tcp_send_ack(t); + if (paws != TCP_PAWS_OK) continue; - } } /* RFC 9293 §3.10.7.4: if the SYN bit is set on a @@ -7178,7 +7210,24 @@ int wolfIP_sock_close(struct wolfIP *s, int sockfd) ts->callback_arg = NULL; close_socket(ts); return 0; - } else return -1; + } else { + /* Never connected and never listened: there is no connection to + * tear down, but the slot must still be released or it is lost for + * good -- tcp_new_socket() treats any proto != 0 slot as occupied. + * The only filter event this socket can have emitted is BINDING, + * so DISSOCIATE, not CLOSED, is the matching teardown. + * TCP_CLOSED also covers a slot whose teardown close_socket() + * deferred for a final CB_EVENT_CLOSED: disarming the callback + * before the reap drops that event, which is what an app-initiated + * close wants -- it may already have released callback_arg. */ + (void)wolfIP_filter_notify_socket_event( + WOLFIP_FILT_DISSOCIATE, s, ts, + ts->local_ip, ts->src_port, IPADDR_ANY, 0); + ts->callback = NULL; + ts->callback_arg = NULL; + close_socket(ts); + return 0; + } } else if (IS_SOCKET_UDP(sockfd)) { struct tsocket *ts; if (SOCKET_UNMARK(sockfd) >= MAX_UDPSOCKETS) @@ -7953,6 +8002,8 @@ static void dhcp_deconfigure_lease(struct wolfIP *s) wolfIP_ipconfig_set(s, 0, 0, 0); s->dhcp_ip = 0; s->dhcp_server_ip = 0; + if (!s->dns_server_pinned) + s->dns_server = 0; } #define DHCP_OPT_data_to_u32(opt) \ @@ -9106,6 +9157,7 @@ void wolfIP_init_static(struct wolfIP **s) if (wolfIP_static.dns_server == 0) { #ifdef WOLFIP_STATIC_DNS_IP wolfIP_static.dns_server = atoip4(WOLFIP_STATIC_DNS_IP); + wolfIP_static.dns_server_pinned = 1; #endif } *s = &wolfIP_static; @@ -9215,7 +9267,7 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (type == 0x83 || type == 0x89) /* LSRR or SSRR */ return; if (opt + 1 >= opt_end || opt[1] < 2) - break; + return; if (opt[1] > (uint8_t)(opt_end - opt)) return; opt += opt[1]; @@ -9225,6 +9277,17 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (version == 4 && ip_hlen >= IP_HEADER_LEN) { ip4 dest = ee32(ip->dst); int is_local = 0; + int l2_group = 0; +#ifdef ETHERNET + /* RFC 1812 sec.5.3.4: a datagram received as a link-layer broadcast + * or multicast must never be forwarded. The group bit (LSB of the + * first MAC octet) covers ff:ff:ff:ff:ff:ff and every multicast MAC. + * Only the forwarding attempt is skipped; local delivery below still + * applies, which is what keeps an L2-broadcast DHCP offer carrying a + * not-yet-ours unicast ip.dst reaching the DHCP socket. */ + if (!wolfIP_ll_is_non_ethernet(s, if_idx) && (ip->eth.dst[0] & 0x01)) + l2_group = 1; +#endif if (dest == IPADDR_ANY || wolfIP_ip_is_broadcast(s, dest)) { is_local = 1; } else { @@ -9288,7 +9351,7 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (rpf_drop) return; - { + if (!l2_group) { int out_if = wolfIP_forward_interface(s, if_idx, dest); if (out_if >= 0) { uint8_t mac[6]; @@ -9471,6 +9534,10 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin eth = (struct wolfIP_eth_frame *)buf; if_idx = sub_idx; ll = wolfIP_ll_at(s, if_idx); + /* Re-notify the eth-layer filter with the demuxed view. */ + if (wolfIP_filter_notify_eth(WOLFIP_FILT_RECEIVING, s, + if_idx, eth, len) != 0) + return; } #endif /* WOLFIP_VLAN */ #if WOLFIP_PACKET_SOCKETS @@ -9694,6 +9761,40 @@ static int dns_skip_name(const uint8_t *buf, int len, int offset) return pos; } +/* Simple helper function to convert characters + * to lower case. + * Needed for case insensitive. */ +static uint8_t dns_tolower(uint8_t c) +{ + if ((c >= 'A') && (c <= 'Z')) + return (uint8_t)(c - 'A' + 'a'); + return c; +} + +/* Walks the response's question bytes against the copy of the outbound + * query already sitting in s->dns_query_buf, and dns_callback calls it. + * Returns 1 if they are equal, 0 otherwise. + * */ +static int dns_question_matches(struct wolfIP *s, const uint8_t *buf, int len, + int offset) +{ + const uint8_t *want = s->dns_query_buf + sizeof(struct dns_header); + int want_len = (int)s->dns_query_len - (int)sizeof(struct dns_header); + int name_len = want_len - (int)sizeof(struct dns_question); + int i; + + if (name_len <= 0) + return 0; /* No outstanding query to compare against. */ + if (offset < 0 || want_len > len - offset) + return 0; + for (i = 0; i < name_len; i++) { + if (dns_tolower(buf[offset + i]) != dns_tolower(want[i])) + return 0; + } + return memcmp(buf + offset + name_len, want + name_len, + sizeof(struct dns_question)) == 0; +} + static int dns_copy_name(const uint8_t *buf, int len, int offset, char *out, size_t out_len) { @@ -9803,6 +9904,13 @@ static void dns_abort_query(struct wolfIP *s) if (!s) return; dns_cancel_timer(s); + /* RFC 5452 s9.2: the source port is part of the anti-spoofing entropy, so + * it must not outlive the query it was drawn for. Releasing it here makes + * the next wolfIP_sock_sendto() draw a fresh one, and leaves the socket + * bound to no port in between, so a late forged reply aimed at the retired + * port no longer matches in udp_try_recv(). */ + if (s->dns_udp_sd > 0 && SOCKET_UNMARK(s->dns_udp_sd) < MAX_UDPSOCKETS) + s->udpsockets[SOCKET_UNMARK(s->dns_udp_sd)].src_port = 0; s->dns_id = 0; s->dns_retry_count = 0; s->dns_query_type = DNS_QUERY_TYPE_NONE; @@ -9872,6 +9980,10 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) pos = sizeof(struct dns_header); qcount = ee16(hdr->qdcount); ancount = ee16(hdr->ancount); + /* A reply to our query echoes back exactly the one question we + * asked (RFC 1035 s7.3). */ + if (qcount != DNS_QUESTION_COUNT) + return; while (qcount-- > 0) { pos = dns_skip_name((const uint8_t *)buf, dns_len, pos); if (pos < 0 || pos + (int)sizeof(struct dns_question) > dns_len) { @@ -9880,6 +9992,11 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) } pos += sizeof(struct dns_question); } + /* Drop a response that answers a different question, but leave the + * query outstanding. */ + if (!dns_question_matches(s, (const uint8_t *)buf, dns_len, + (int)sizeof(struct dns_header))) + return; while (ancount-- > 0) { struct dns_rr *rr; uint16_t rdlen; @@ -9986,6 +10103,16 @@ static int dns_send_query(struct wolfIP *s, const char *dname, uint16_t *id, dns_srv.sin_family = AF_INET; dns_srv.sin_port = ee16(DNS_PORT); dns_srv.sin_addr.s_addr = ee32(s->dns_server); + /* RFC 1035 s4.2.1: the reply comes back from the server's port 53. + * Connecting engages udp_try_recv()'s peer filter, so a forged reply from + * any other source is dropped by the demux instead of being queued for + * dns_callback() to sift through. */ + if (wolfIP_sock_connect(s, s->dns_udp_sd, (struct wolfIP_sockaddr *)&dns_srv, + sizeof(struct wolfIP_sockaddr_in)) < 0) { + dns_abort_query(s); + *id = DNS_ID_NONE; + return -1; + } ret = wolfIP_sock_sendto(s, s->dns_udp_sd, buf, s->dns_query_len, 0, (struct wolfIP_sockaddr *)&dns_srv, sizeof(struct wolfIP_sockaddr_in)); if (ret < 0) {