Skip to content

Commit 6dd14a0

Browse files
committed
Addressed copilot comments + added -4/-6 switch to proxy
1 parent 2b5ec70 commit 6dd14a0

6 files changed

Lines changed: 106 additions & 32 deletions

File tree

proxy/dohproxyd.c

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,8 +1067,10 @@ static void accept_client(int fd, short revents, void *arg)
10671067
static void usage(const char *name)
10681068
{
10691069
fprintf(stderr, "%s, ODoH proxy with legacy RFC8484 forwarding.\n", name);
1070-
fprintf(stderr, "Usage: %s -c cert -k key [-p port] [-u user] [-A cafile] [--target-url https://host/path]... [--targets-file file] [--target-cert cert --target-key key] [-F] [-v] [-V] [-h]\n", name);
1070+
fprintf(stderr, "Usage: %s -c cert -k key [-p port] [-4|-6] [-u user] [-A cafile] [--target-url https://host/path]... [--targets-file file] [--target-cert cert --target-key key] [-F] [-v] [-V] [-h]\n", name);
10711071
fprintf(stderr, " cert/key: TLS server certificate and key\n");
1072+
fprintf(stderr, " -4: force IPv4 only\n");
1073+
fprintf(stderr, " -6: force IPv6 only (default: dual-stack)\n");
10721074
fprintf(stderr, " -A/--ca-file: CA bundle for verifying upstream target TLS certs\n");
10731075
fprintf(stderr, " --target-url: repeatable legacy RFC8484 target URL\n");
10741076
fprintf(stderr, " --targets-file: file with target URLs (one per line)\n");
@@ -1083,11 +1085,13 @@ int main(int argc, char *argv[])
10831085
char *targets_file = NULL;
10841086
char *upstream_cafile = NULL;
10851087
uint16_t port = PROXY_PORT;
1088+
int ip_version = 0; /* 0 = dual-stack, 4 = IPv4 only, 6 = IPv6 only */
10861089
int foreground = 0;
10871090
int loglvl = DOH_WARN;
10881091
int c, option_idx = 0;
10891092
int yes = 1;
1090-
struct sockaddr_in6 addr;
1093+
struct sockaddr_in6 addr6;
1094+
struct sockaddr_in addr4;
10911095
struct option long_options[] = {
10921096
{"help", 0, 0, 'h'},
10931097
{"version", 0, 0, 'V'},
@@ -1102,11 +1106,13 @@ int main(int argc, char *argv[])
11021106
{"target-key", 1, 0, 'y'},
11031107
{"target-url", 1, 0, 't'},
11041108
{"targets-file", 1, 0, 'T'},
1109+
{"ipv4", 0, 0, '4'},
1110+
{"ipv6", 0, 0, '6'},
11051111
{NULL, 0, 0, 0}
11061112
};
11071113

11081114
while (1) {
1109-
c = getopt_long(argc, argv, "hVc:k:p:u:A:vFx:y:t:T:", long_options, &option_idx);
1115+
c = getopt_long(argc, argv, "46hVc:k:p:u:A:vFx:y:t:T:", long_options, &option_idx);
11101116
if (c < 0)
11111117
break;
11121118

@@ -1122,6 +1128,8 @@ int main(int argc, char *argv[])
11221128
case 'F': foreground = 1; break;
11231129
case 'x': target_client_cert = strdup(optarg); break;
11241130
case 'y': target_client_key = strdup(optarg); break;
1131+
case '4': ip_version = 4; break;
1132+
case '6': ip_version = 6; break;
11251133
case 't':
11261134
if (add_target_url(optarg) != 0)
11271135
return 2;
@@ -1181,19 +1189,50 @@ int main(int argc, char *argv[])
11811189
if (wolfSSL_CTX_use_PrivateKey_file(srv_ctx, key, SSL_FILETYPE_PEM) != SSL_SUCCESS)
11821190
return 1;
11831191

1184-
lfd = socket(AF_INET6, SOCK_STREAM, 0);
1185-
if (lfd < 0)
1186-
return 1;
1192+
/* Create listening socket based on IP version preference */
1193+
if (ip_version == 4) {
1194+
/* IPv4 only */
1195+
lfd = socket(AF_INET, SOCK_STREAM, 0);
1196+
if (lfd < 0)
1197+
return 1;
1198+
setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes));
1199+
setsockopt(lfd, SOL_SOCKET, SO_REUSEPORT, (char *)&yes, sizeof(yes));
11871200

1188-
setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes));
1189-
setsockopt(lfd, SOL_SOCKET, SO_REUSEPORT, (char *)&yes, sizeof(yes));
1201+
memset(&addr4, 0, sizeof(addr4));
1202+
addr4.sin_family = AF_INET;
1203+
addr4.sin_addr.s_addr = INADDR_ANY;
1204+
addr4.sin_port = htons(port);
11901205

1191-
memset(&addr, 0, sizeof(addr));
1192-
addr.sin6_family = AF_INET6;
1193-
addr.sin6_port = htons(port);
1206+
if (bind(lfd, (struct sockaddr *)&addr4, sizeof(addr4)) != 0)
1207+
return 1;
1208+
dohprint(DOH_NOTICE, "dohproxyd listening on 0.0.0.0:%u (IPv4 only)", port);
1209+
} else {
1210+
/* IPv6 (with or without dual-stack) */
1211+
lfd = socket(AF_INET6, SOCK_STREAM, 0);
1212+
if (lfd < 0)
1213+
return 1;
1214+
setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes));
1215+
setsockopt(lfd, SOL_SOCKET, SO_REUSEPORT, (char *)&yes, sizeof(yes));
11941216

1195-
if (bind(lfd, (struct sockaddr *)&addr, sizeof(addr)) != 0)
1196-
return 1;
1217+
if (ip_version == 6) {
1218+
/* IPv6 only - disable dual-stack */
1219+
int ipv6only = 1;
1220+
setsockopt(lfd, IPPROTO_IPV6, IPV6_V6ONLY, &ipv6only, sizeof(ipv6only));
1221+
}
1222+
1223+
memset(&addr6, 0, sizeof(addr6));
1224+
addr6.sin6_family = AF_INET6;
1225+
addr6.sin6_addr = in6addr_any;
1226+
addr6.sin6_port = htons(port);
1227+
1228+
if (bind(lfd, (struct sockaddr *)&addr6, sizeof(addr6)) != 0)
1229+
return 1;
1230+
1231+
if (ip_version == 6)
1232+
dohprint(DOH_NOTICE, "dohproxyd listening on [::]:%u (IPv6 only)", port);
1233+
else
1234+
dohprint(DOH_NOTICE, "dohproxyd listening on [::]:%u (dual-stack)", port);
1235+
}
11971236

11981237
if (listen(lfd, 32) != 0)
11991238
return 1;
@@ -1207,7 +1246,6 @@ int main(int argc, char *argv[])
12071246
}
12081247

12091248
evquick_addevent(lfd, EVQUICK_EV_READ, accept_client, NULL, NULL);
1210-
dohprint(DOH_NOTICE, "dohproxyd listening on [::]:%u", port);
12111249

12121250
while (run)
12131251
evquick_loop();

src/dohd.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,14 @@ static int dns_send_request_h2(struct req_slot *req)
680680
}
681681
req->ev_dns = evquick_addevent(req->dns_sd, EVQUICK_EV_READ, dohd_reply,
682682
NULL, req);
683+
if (!req->ev_dns) {
684+
/* Event registration failed - destroy request immediately */
685+
dohprint(DOH_ERR, "Failed to register DNS socket event");
686+
DOH_Stats.socket_errors++;
687+
check_stats();
688+
dohd_destroy_request(req);
689+
return -1;
690+
}
683691
ret = sendto(req->dns_sd, req->h2_request_buffer, req->h2_request_len, 0,
684692
(struct sockaddr *)req->resolver, req->resolver_sz);
685693
if (ret < 0) {
@@ -1070,9 +1078,16 @@ static int h2_cb_on_frame_recv(nghttp2_session *session,
10701078
};
10711079
nghttp2_submit_response(session, frame->hd.stream_id, nva, 2, NULL);
10721080
dohd_destroy_request(req);
1073-
} else if (req->h2_request_len > 0)
1074-
dns_send_request_h2(req);
1075-
else
1081+
} else if (req->h2_request_len > 0) {
1082+
if (dns_send_request_h2(req) < 0) {
1083+
/* Request already destroyed by dns_send_request_h2, send error */
1084+
nghttp2_nv nva[] = {
1085+
MAKE_NV(":status", "500"),
1086+
MAKE_NV("server", "dohd"),
1087+
};
1088+
nghttp2_submit_response(session, frame->hd.stream_id, nva, 2, NULL);
1089+
}
1090+
} else
10761091
dohd_destroy_request(req);
10771092
}
10781093
break;

src/libevquick.c

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -348,28 +348,31 @@ CTX evquick_init(void)
348348
if (!ctx)
349349
return NULL;
350350
ctx->giveup = 0;
351+
ctx->epfd = -1;
352+
ctx->time_machine[0] = -1;
353+
ctx->time_machine[1] = -1;
354+
351355
ctx->timers = heap_init();
352356
if (!ctx->timers)
353-
return NULL;
357+
goto fail;
354358
if(pipe(ctx->time_machine) < 0)
355-
return NULL;
359+
goto fail;
356360
(void)yes;
357361
fcntl(ctx->time_machine[1], F_SETFL, O_NONBLOCK);
358362

359363
/* Create epoll instance */
360364
ctx->epfd = epoll_create1(0);
361365
if (ctx->epfd < 0) {
362366
perror("epoll_create1");
363-
return NULL;
367+
goto fail;
364368
}
365369

366370
/* Add time_machine pipe to epoll for timer wakeups */
367371
ev.events = EPOLLIN;
368372
ev.data.ptr = NULL; /* NULL ptr indicates time_machine */
369373
if (epoll_ctl(ctx->epfd, EPOLL_CTL_ADD, ctx->time_machine[0], &ev) < 0) {
370374
perror("epoll_ctl time_machine");
371-
close(ctx->epfd);
372-
return NULL;
375+
goto fail;
373376
}
374377

375378
ctx->n_events = 1;
@@ -378,11 +381,23 @@ CTX evquick_init(void)
378381
act.sa_flags = SA_NODEFER;
379382
if (sigaction(SIGALRM, &act, NULL) < 0) {
380383
perror("Setting alarm signal");
381-
return NULL;
384+
goto fail;
382385
}
383386
ctx_add(ctx);
384387
timer_new(ctx);
385388
return ctx;
389+
390+
fail:
391+
if (ctx->epfd >= 0)
392+
close(ctx->epfd);
393+
if (ctx->time_machine[0] >= 0)
394+
close(ctx->time_machine[0]);
395+
if (ctx->time_machine[1] >= 0)
396+
close(ctx->time_machine[1]);
397+
if (ctx->timers)
398+
heap_destroy(ctx->timers);
399+
free(ctx);
400+
return NULL;
386401
}
387402

388403

test/stress_test.sh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ worker() {
115115
result=$(curl -s -k --http2 -4 \
116116
--connect-timeout 2 \
117117
--max-time 5 \
118+
-H "Accept: application/dns-message" \
119+
-H "Content-Type: application/dns-message" \
118120
-o /dev/null \
119121
-w "%{http_code}" \
120122
"https://127.0.0.1:$PORT/?dns=$dns_q" 2>&1)
@@ -177,7 +179,7 @@ fi
177179
# Verify responding
178180
echo "Verifying dohd is responsive..."
179181
for i in {1..5}; do
180-
if timeout 5 curl -s -k --http2 -4 "https://127.0.0.1:$PORT/?dns=${DNS_QUERIES[0]}" -o /dev/null 2>&1; then
182+
if timeout 5 curl -s -k --http2 -4 -H "Accept: application/dns-message" -H "Content-Type: application/dns-message" "https://127.0.0.1:$PORT/?dns=${DNS_QUERIES[0]}" -o /dev/null 2>&1; then
181183
echo "dohd is responding"
182184
break
183185
fi
@@ -218,7 +220,7 @@ while [ $(date +%s) -lt $END_TIME ]; do
218220
fi
219221

220222
# Health check
221-
if ! timeout 5 curl -s -k --http2 -4 "https://127.0.0.1:$PORT/?dns=${DNS_QUERIES[0]}" -o /dev/null 2>&1; then
223+
if ! timeout 5 curl -s -k --http2 -4 -H "Accept: application/dns-message" -H "Content-Type: application/dns-message" "https://127.0.0.1:$PORT/?dns=${DNS_QUERIES[0]}" -o /dev/null 2>&1; then
222224
echo "WARNING: Health check failed at $(date +%H:%M:%S)"
223225
fi
224226

tools/Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ all: odoh-keygen
1111
debug: CFLAGS+= -ggdb -O0
1212
debug: odoh-keygen
1313

14-
asan: CFLAGS+=-fsanitize=address
15-
asan: LDFLAGS+=-fsanitize=address
14+
asan: CFLAGS+= -fsanitize=address
15+
asan: LDFLAGS+= -fsanitize=address
1616
asan: odoh-keygen
1717

1818
odoh-keygen: odoh-keygen.o

tools/odoh-keygen.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,19 @@ static int write_odoh_config(const char *path, const uint8_t *pub, size_t pub_le
7979
{
8080
int fd = open(path, O_WRONLY | O_APPEND, 0644);
8181
ssize_t w;
82+
size_t off = 0;
8283
if (fd < 0) {
8384
fprintf(stderr, "Cannot append to %s: %s\n", path, strerror(errno));
8485
return -1;
8586
}
86-
w = write(fd, pub, pub_len);
87-
if (w != (ssize_t)pub_len) {
88-
close(fd);
89-
fprintf(stderr, "Cannot append public key to %s\n", path);
90-
return -1;
87+
while (off < pub_len) {
88+
w = write(fd, pub + off, pub_len - off);
89+
if (w <= 0) {
90+
close(fd);
91+
fprintf(stderr, "Cannot append public key to %s: %s\n", path, strerror(errno));
92+
return -1;
93+
}
94+
off += (size_t)w;
9195
}
9296
if (close(fd) != 0) {
9397
fprintf(stderr, "Cannot close %s: %s\n", path, strerror(errno));

0 commit comments

Comments
 (0)