[PATCH gwproxy v5 0/2] Initial work on integration of DNS parser lib in gwproxy

[PATCH gwproxy v5 0/2] Initial work on integration of DNS parser lib in gwproxy

[Ahmad Gani]

Hi Chief,

This is revision v5 of initial work on integration of DNS parser lib in 
gwproxy. This is an RFC draft, the patches itself aren't final.

With current model, the caller is responsible to do the socket part, this
imply unlike conventional glibc's getaddrinfo that handle both socket and
crafting payload inside its abstraction, thus allow AF_UNSPEC, the build
query only craft packet for particular family at a time [1], either
AF_INET or AF_INET6, not both, I see this not as a limitation, but rather
an advantage, most of the time we don't need to request for both IPv6 and
IPv4, but there's still a case where both family is desired, it's often 
happened for fallback mechanism. I'll spend some time thinking about how
to integrate these functions into gwproxy.

# Changelog
v1 -> v2:
- use existing convert_str_to_ssaddr instead of init_addr
- fix memory leak when init_addr (now it's replaced) failed
- modify convert_str_to_ssaddr to support default port
- bring back __cold attribute on convert_ssaddr_to_str
- don't fill a dangling pointer as Sir Alviro said
- for now it's blocking, attempt_retry label is not needed

v2 -> v3:
- remove TODO(reyuki): hints->ai_family is used to filter results
- make gw_ares_getaddrinfo asynchronous by change UDP socket to non-blocking
- move socket creation to library initialization
- make UDP socket unconnected
- restructure internal struct and program execution flow to support async
- update unit test of dns parser
- transaction id creation is delegated to caller
- fix logic bug in serialize_answ that can lead to memory error: invalid read
- rename parameter prt and split commit message
- move variable declaration in for loop according to the Sir Alviro coding-style

v3 -> v4:
- add else block in the convert_str_to_ssaddr function
- use format specifier %hu for uint16_t in printf
- update base commit to branch master from upstream remote repository

v4 -> v5:
- squash commits to eliminate changes unrelated to the commit subject
- change commit subject: use imperative form for the subject
- update base commit to branch master from upstream remote repository
- changing the email subject to depict a turnaround of design changes
- drop the c-ares style thing
- restructure dnsparser.c and dnsparser.h

[1]:
- https://lore.gnuweeb.org/gwml/20250806035727.216702-1-reyuki@gnuweeb.org/
- www.ietf.org/rfc/rfc9619.pdf
- https://stackoverflow.com/questions/4082081/requesting-a-and-aaaa-records-in-single-dns-query/4083071

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---

Ahmad Gani (2):
  dns: Allow only port string number
  dnsparser: Add dns parser code

 man/gwp_dns_queue.3     |   2 +-
 src/gwproxy/dns.h       |   3 +-
 src/gwproxy/dnsparser.c | 568 ++++++++++++++++++++++++++++++++++++++++
 src/gwproxy/dnsparser.h | 210 +++++++++++++++
 4 files changed, 780 insertions(+), 3 deletions(-)
 create mode 100644 src/gwproxy/dnsparser.c
 create mode 100644 src/gwproxy/dnsparser.h


base-commit: cff091cf6441845c1a8fc56b9e1216bf6cf48dca
-- 
Ahmad Gani

[PATCH gwproxy v5 1/2] dns: Allow only port string number

[Ahmad Gani]

Instead of adding additional work for parsing well-known services like
http, https, etc... (port in socks5 field is a number anyway).

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
 man/gwp_dns_queue.3 | 2 +-
 src/gwproxy/dns.h   | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/man/gwp_dns_queue.3 b/man/gwp_dns_queue.3
index dcd06746bbcc..f57807517df5 100644
--- a/man/gwp_dns_queue.3
+++ b/man/gwp_dns_queue.3
@@ -77,7 +77,7 @@ contains:
 The hostname being resolved.
 .TP
 .B char *service
-The service name or port number (e.g., "http", "443").
+The port number in ascii format.
 .TP
 .B _Atomic(int) refcnt
 Atomic reference count for the entry.
diff --git a/src/gwproxy/dns.h b/src/gwproxy/dns.h
index a4038fc0b254..10c7cea2ebe4 100644
--- a/src/gwproxy/dns.h
+++ b/src/gwproxy/dns.h
@@ -71,8 +71,7 @@ void gwp_dns_ctx_free(struct gwp_dns_ctx *ctx);
  *
  * @param ctx		Pointer to the DNS context.
  * @param name		Name to resolve.
- * @param service 	Service to resolve (e.g., "http", "https", or port
- *			number).
+ * @param service 	Service to resolve in port number ascii format.
  * @return		Pointer to a gwp_dns_entry on success, NULL on failure.
  */
 struct gwp_dns_entry *gwp_dns_queue(struct gwp_dns_ctx *ctx,
-- 
Ahmad Gani

[PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

Introducing dns parser for better flexibility over the program flow, the
DNS protocol implementation is limited to standard query (OPCODE_QUERY).

Also, Delegate creation of transaction id to the caller, It gives the
caller more flexibility in handling DNS requests.

A single domain name lookup can have two responses: one for IPv4 and one
for IPv6. With this change, the caller may use the same txid for both
queries, making it easier to group related DNS responses.

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
 src/gwproxy/dnsparser.c | 568 ++++++++++++++++++++++++++++++++++++++++
 src/gwproxy/dnsparser.h | 210 +++++++++++++++
 2 files changed, 778 insertions(+)
 create mode 100644 src/gwproxy/dnsparser.c
 create mode 100644 src/gwproxy/dnsparser.h

diff --git a/src/gwproxy/dnsparser.c b/src/gwproxy/dnsparser.c
new file mode 100644
index 000000000000..3a04f37f72aa
--- /dev/null
+++ b/src/gwproxy/dnsparser.c
@@ -0,0 +1,568 @@
+#define _DEFAULT_SOURCE
+#include <endian.h>
+#include <gwproxy/dnsparser.h>
+#include <gwproxy/net.h>
+
+static ssize_t construct_qname(uint8_t *dst, size_t dst_len, const char *qname)
+{
+	const uint8_t *p = (const uint8_t *)qname;
+	uint8_t *lp = dst; // Length position.
+	uint8_t *sp = lp + 1;  // String position.
+	size_t total = 0;
+	uint16_t l;
+
+	l = 0;
+	while (1) {
+		uint8_t c = *p++;
+
+		total++;
+		if (total >= dst_len)
+			return -ENAMETOOLONG;
+
+		if (c == '.' || c == '\0') {
+			if (l < 1 || l > 255)
+				return -EINVAL;
+
+			*lp = (uint8_t)l;
+			lp = sp++;
+			l = 0;
+			if (!c)
+				break;
+		} else {
+			l++;
+			*sp = c;
+			sp++;
+		}
+	}
+
+	return total;
+}
+
+static ssize_t calculate_question_len(uint8_t *in, size_t in_len)
+{
+	const uint8_t *p = in;
+	size_t tot_len, advance_len;
+
+	tot_len = 0;
+	while (true) {
+		if (*p == 0x0) {
+			tot_len++;
+			break;
+		}
+
+		if (tot_len >= in_len)
+			return -ENAMETOOLONG;
+
+		advance_len = *p + 1;
+		tot_len += advance_len;
+		p += advance_len;
+	}
+
+	tot_len += 4;
+
+	return  tot_len;
+}
+
+static int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
+{
+	gwdns_header_pkt *hdr;
+	uint16_t raw_flags;
+	size_t idx, i;
+	void *ptr;
+	int ret;
+
+	idx = sizeof(*hdr);
+	if (idx >= in_len)
+		return -EAGAIN;
+
+	hdr = (void *)in;
+	if (memcmp(&txid, &hdr->id, sizeof(txid)))
+		return -EINVAL;
+
+	memcpy(&raw_flags, &hdr->flags, sizeof(raw_flags));
+	raw_flags = ntohs(raw_flags);
+	/* QR MUST 1 = response from dns server */
+	if (!DNS_QR(raw_flags))
+		return -EINVAL;
+
+	/* OPCODE MUST 0 = standard query */
+	if (DNS_OPCODE(raw_flags))
+		return -EINVAL;
+
+	/* RCODE MUST 0 = No error */
+	if (DNS_RCODE(raw_flags))
+		return -EPROTO;
+
+	// is it safe or recommended to alter the in buffer directly?
+	hdr->ancount = ntohs(hdr->ancount);
+	if (!hdr->ancount)
+		return -ENODATA;
+
+	/*
+	 * Check the sizes upfront.
+	 *
+	 * 1 bytes for variable-length
+	 * in[idx] for the length of first name
+	 * 1 bytes for null terminator
+	 * 2 bytes for qtype
+	 * 2 bytes for qclass
+	 */
+	if ((size_t)(1 + in[idx] + 1 + 2 + 2) >= in_len)
+		return -EINVAL;
+
+	ret = calculate_question_len(&in[idx], in_len - idx);
+	if (ret <= 0)
+		return -EINVAL;
+
+	idx += ret;
+	if (idx >= in_len)
+		return -EAGAIN;
+
+	out->hdr.ancount = 0;
+	ptr = malloc(hdr->ancount * sizeof(uint8_t *));
+	if (!ptr)
+		return -ENOMEM;
+
+	out->rr_answ = ptr;
+	for (i = 0; i < hdr->ancount; i++) {
+		uint16_t is_compressed, rdlength;
+		gwdns_serialized_answ *item = malloc(sizeof(gwdns_serialized_answ));
+		if (!item) {
+			ret = -ENOMEM;
+			goto exit_free;
+		}
+
+		memcpy(&is_compressed, &in[idx], sizeof(is_compressed));
+		is_compressed = DNS_IS_COMPRESSED(ntohs(is_compressed));
+		assert(is_compressed);
+		idx += 2; // NAME
+		if (idx >= in_len) {
+			ret = -EAGAIN;
+			free(item);
+			goto exit_free;
+		}
+
+		memcpy(&item->rr_type, &in[idx], 2);
+		item->rr_type = ntohs(item->rr_type);
+		idx += 2; // TYPE
+		if (idx >= in_len) {
+			ret = -EAGAIN;
+			free(item);
+			goto exit_free;
+		}
+		memcpy(&item->rr_class, &in[idx], 2);
+		item->rr_class = ntohs(item->rr_class);
+		idx += 2; // CLASS
+		if (idx >= in_len) {
+			ret = -EAGAIN;
+			free(item);
+			goto exit_free;
+		}
+		memcpy(&item->ttl, &in[idx], 4);
+		item->ttl = be32toh(item->ttl);
+		idx += 4; // TTL
+		if (idx >= in_len) {
+			ret = -EAGAIN;
+			free(item);
+			goto exit_free;
+		}
+
+		memcpy(&rdlength, &in[idx], sizeof(rdlength));
+		rdlength = ntohs(rdlength);
+		if (item->rr_type != TYPE_AAAA && item->rr_type != TYPE_A) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		if (item->rr_type == TYPE_AAAA && rdlength != sizeof(struct in6_addr)) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		if (item->rr_type == TYPE_A && rdlength != sizeof(struct in_addr)) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		item->rdlength = rdlength;
+		idx += 2;
+		if (idx >= in_len) {
+			ret = -EAGAIN;
+			free(item);
+			goto exit_free;
+		}
+
+		/*
+		 * considering if condition above,
+		 * maybe we don't need a malloc and just allocate fixed size
+		 * for rdata? however if this parser want to be expanded for
+		 * other dns operation (e.g OPCODE_IQUERY, etc), rdata maybe
+		 * contain more than sizeof in6_addr.
+		 */
+		ptr = malloc(rdlength);
+		if (!ptr) {
+			ret = -ENOMEM;
+			free(item);
+			goto exit_free;
+		}
+
+		memcpy(ptr, &in[idx], rdlength);
+		idx += rdlength;
+		if (idx > in_len) {
+			ret = -EINVAL;
+			free(item);
+			free(ptr);
+			goto exit_free;
+		}
+
+		item->rdata = ptr;
+		out->rr_answ[i] = item;
+		out->hdr.ancount++;
+	}
+
+	return 0;
+exit_free:
+	for (i = 0; i < out->hdr.ancount; i++) {
+		free(out->rr_answ[i]->rdata);
+		free(out->rr_answ[i]);
+	}
+	free(out->rr_answ);
+	return ret;
+}
+
+static void free_serialize_answ(gwdns_answ_data *answ)
+{
+	size_t i;
+	for (i = 0; i < answ->hdr.ancount; i++) {
+		free(answ->rr_answ[i]->rdata);
+		free(answ->rr_answ[i]);
+	}
+	free(answ->rr_answ);
+}
+
+int gwdns_parse_query(uint16_t txid, const char *service,
+			uint8_t *in, size_t in_len,
+			struct gwdns_addrinfo_node **ai)
+{
+	struct gwdns_addrinfo_node *results, *tail;
+	gwdns_answ_data raw_answ;
+	int r, port;
+	size_t i;
+
+	port = atoi(service);
+	if (port < 0)
+		return -EINVAL;
+	port = htons(port);
+
+	r = serialize_answ(txid, in, in_len, &raw_answ);
+	if (r)
+		return r;
+
+	if (!raw_answ.hdr.ancount)
+		goto exit_free;
+
+	tail = NULL;
+	for (i = 0; i < raw_answ.hdr.ancount; i++) {
+		struct gwdns_addrinfo_node *new_node;
+		gwdns_serialized_answ *answ;
+		struct sockaddr_in6 *i6;
+		struct sockaddr_in *i4;
+
+		answ = raw_answ.rr_answ[i];
+		new_node = malloc(sizeof(*new_node));
+		if (!new_node) {
+			r = -ENOMEM;
+			goto exit_free;
+		}
+		new_node->ai_next = NULL;
+
+		if (answ->rr_type == TYPE_AAAA) {
+			i6 = &new_node->ai_addr.i6;
+			new_node->ai_family = AF_INET6;
+			new_node->ai_addrlen = sizeof(i6);
+			i6->sin6_port = port;
+			i6->sin6_family = AF_INET6;
+			assert(sizeof(i6->sin6_addr) == answ->rdlength);
+			memcpy(&i6->sin6_addr, answ->rdata, answ->rdlength);
+		} else {
+			i4 = &new_node->ai_addr.i4;
+			new_node->ai_family = AF_INET;
+			new_node->ai_addrlen = sizeof(i4);
+			i4->sin_port = port;
+			i4->sin_family = AF_INET;
+			assert(sizeof(i4->sin_addr) == answ->rdlength);
+			memcpy(&i4->sin_addr, answ->rdata, answ->rdlength);
+			new_node->ai_ttl = answ->ttl;
+		}
+
+		if (!tail)
+			results = new_node;
+		else
+			tail->ai_next = new_node;
+		tail = new_node;
+	}
+
+	*ai = results;
+	r = 0;
+exit_free:
+	free_serialize_answ(&raw_answ);
+	return r;
+}
+
+void gwdns_free_parsed_query(struct gwdns_addrinfo_node *ai)
+{
+	struct gwdns_addrinfo_node *tmp, *node = ai;
+	while (node) {
+		tmp = node->ai_next;
+		free(node);
+		node = tmp;
+	}
+}
+
+static ssize_t construct_question(gwdns_question_part *question)
+{
+	gwdns_header_pkt *hdr;
+	gwdns_query_pkt pkt;
+	uint16_t qtype, qclass;
+	size_t required_len;
+	ssize_t bw;
+
+	switch (question->type) {
+	case AF_INET6:
+		question->type = TYPE_AAAA;
+		break;
+	case AF_INET:
+		question->type = TYPE_A;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	hdr = &pkt.hdr;
+	/*
+	* the memset implicitly set opcode to OPCODE_QUERY
+	*/
+	memset(hdr, 0, sizeof(*hdr));
+	/*
+	 * no need to htons, so no ntohs for comparison in serialize_answ.
+	 */
+	hdr->id = question->txid;
+	DNS_SET_RD(hdr->flags, true);
+	hdr->flags = htons(hdr->flags);
+	hdr->qdcount = htons(1);
+
+	/*
+	* pkt.body is interpreted as question section
+	* for layout and format, see RFC 1035 4.1.2. Question section format
+	*/
+	bw = construct_qname(pkt.body, sizeof(pkt.body) - 3, question->domain);
+	if (bw < 0)
+		return bw;
+
+	pkt.body[bw++] = 0x0;
+	qtype = htons(question->type);
+	qclass = htons(CLASS_IN);
+	memcpy(&pkt.body[bw], &qtype, 2);
+	bw += 2;
+	memcpy(&pkt.body[bw], &qclass, 2);
+	bw += 2;
+
+	required_len = sizeof(pkt.hdr) + bw;
+	if (question->dst_len < required_len)
+		return -ENOBUFS;
+
+	memcpy(question->dst_buffer, &pkt, required_len);
+
+	return required_len;
+}
+
+ssize_t gwdns_build_query(uint16_t txid, const char *name, int family, uint8_t *out, size_t out_len)
+{
+	gwdns_question_part q;
+
+	q.domain = name;
+	q.type = family;
+	q.txid = txid;
+	q.dst_buffer = out;
+	q.dst_len = out_len;
+	return construct_question(&q);
+}
+
+#ifdef RUNTEST
+
+void test_parse_ipv4(void)
+{
+	struct gwdns_addrinfo_node *d, *node;
+	uint16_t txid;
+
+	uint8_t recv_pkt[] = {
+		0x23, 0xc6, 0x81, 0x80, 0x00, 0x01, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00,
+		0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00,
+		0x00, 0x01, 0x00, 0x01, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+		0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x8a, 0xc0, 0x0c, 0x00, 0x01,
+		0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x66,
+		0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04,
+		0x4a, 0x7d, 0x18, 0x64, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+		0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x8b, 0xc0, 0x0c, 0x00, 0x01,
+		0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x65,
+		0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04,
+		0x4a, 0x7d, 0x18, 0x71
+	};
+
+	memcpy(&txid, recv_pkt, 2);
+	d = NULL;
+	assert(!gwdns_parse_query(txid, "80", recv_pkt, sizeof(recv_pkt), &d));
+	assert(d);
+	node = d;
+	while (node) {
+		struct gwdns_addrinfo_node *tmp;
+		char buff[FULL_ADDRSTRLEN];
+
+		tmp = node->ai_next;
+		assert(node->ai_family == AF_INET);
+		convert_ssaddr_to_str(buff, &node->ai_addr);
+		printf("IPv4: %s\n", buff);
+		node = tmp;
+	}
+
+	gwdns_free_parsed_query(d);
+}
+
+void test_parse_ipv6(void)
+{
+	struct gwdns_addrinfo_node *d, *node;
+	uint16_t txid;
+
+	uint8_t recv_pkt[] = {
+		0x45, 0x67,
+		0x81, 0x80,
+		0x00, 0x01,
+		0x00, 0x04,
+		0x00, 0x00,
+		0x00, 0x00,
+
+		0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65,
+		0x03, 0x63, 0x6f, 0x6d,
+		0x00,
+		0x00, 0x1c,
+		0x00, 0x01,
+
+		0xc0, 0x0c,
+		0x00, 0x1c,
+		0x00, 0x01,
+		0x00, 0x00, 0x09, 0x06,
+		0x00, 0x10,
+		0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x71,
+
+		0xc0, 0x0c,
+		0x00, 0x1c, 0x00, 0x01, 0x00, 0x00, 0x09, 0x06,
+		0x00, 0x10,
+		0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8a,
+
+		0xc0, 0x0c,
+		0x00, 0x1c,
+		0x00, 0x01,
+		0x00, 0x00, 0x09, 0x06,
+		0x00, 0x10,
+		0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x65,
+
+		0xc0, 0x0c,
+		0x00, 0x1c,
+		0x00, 0x01,
+		0x00, 0x00, 0x0c, 0x16,
+		0x00, 0x10,
+		0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x71
+	};
+
+	memcpy(&txid, recv_pkt, sizeof(txid));
+
+	node = NULL;
+	assert(!gwdns_parse_query(txid, "80", recv_pkt, sizeof(recv_pkt), &d));
+	assert(d);
+	node = d;
+	while (node) {
+		struct gwdns_addrinfo_node *tmp;
+		char buff[FULL_ADDRSTRLEN];
+
+		tmp = node->ai_next;
+		assert(node->ai_family == AF_INET6);
+		convert_ssaddr_to_str(buff, &node->ai_addr);
+		printf("IPv6: %s\n", buff);
+		node = tmp;
+	}
+
+	gwdns_free_parsed_query(d);
+}
+
+void test_build_ipv4(void)
+{
+	uint8_t buff[UDP_MSG_LIMIT];
+	gwdns_header_pkt *hdr;
+	uint16_t c;
+	ssize_t r;
+
+	c = 0xFFFF;
+	r = gwdns_build_query(c, "google.com", AF_INET, buff, sizeof(buff));
+	assert(r > 0);
+
+	hdr = (void *)buff;
+	assert(ntohs(hdr->qdcount) == 1);
+	assert(!hdr->nscount);
+	assert(!DNS_QR(hdr->flags));
+	assert(DNS_OPCODE(hdr->flags) == OPCODE_QUERY);
+	c = htons(TYPE_A);
+	assert(!memcmp(buff + 12 + 12, &c, 2));
+}
+
+void test_build_ipv6(void)
+{
+	uint8_t buff[UDP_MSG_LIMIT];
+	gwdns_header_pkt *hdr;
+	uint16_t c;
+	ssize_t r;
+
+	c = 0xFFFF;
+	r = gwdns_build_query(c, "google.com", AF_INET6, buff, sizeof(buff));
+	assert(r > 0);
+
+	hdr = (void *)buff;
+	assert(ntohs(hdr->qdcount) == 1);
+	assert(!hdr->nscount);
+	assert(!DNS_QR(hdr->flags));
+	assert(DNS_OPCODE(hdr->flags) == OPCODE_QUERY);
+	c = htons(TYPE_AAAA);
+	assert(!memcmp(buff + 12 + 12, &c, 2));
+}
+
+/*
+ * test mock data of recv in both IPv4 and IPv6
+ *
+ * the mock data are produced by this script:
+ * https://gist.github.com/realyukii/d7b450b4ddc305c66a2d8cd5600f23c4
+ */
+void run_all_tests(void)
+{
+	/*
+	 * We can't use serialize_answ to parse multiple response at once.
+	 * The caller MUST call serialize_answ more than one time if there's
+	 * more than one response, because txid is passed to only verify one
+	 * response.
+	 */
+	fprintf(stderr, "test constructing DNS standard query packet for TYPE_A!\n");
+	test_build_ipv4();
+	fprintf(stderr, "test constructing DNS standard query packet for TYPE_AAAA!\n");
+	test_build_ipv6();
+	fprintf(stderr, "test parsing DNS standard query packet for TYPE_A!\n");
+	test_parse_ipv4();
+	fprintf(stderr, "test parsing DNS standard query packet for TYPE_AAAA!\n");
+	test_parse_ipv6();
+	fprintf(stderr, "all tests passed!\n");
+}
+
+int main(void)
+{
+	run_all_tests();
+	return 0;
+}
+
+#endif
diff --git a/src/gwproxy/dnsparser.h b/src/gwproxy/dnsparser.h
new file mode 100644
index 000000000000..c96d3caafc48
--- /dev/null
+++ b/src/gwproxy/dnsparser.h
@@ -0,0 +1,210 @@
+#include <stdint.h>
+#include <stddef.h>
+#include <assert.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <arpa/inet.h>
+#include <sys/socket.h>
+#include <liburing.h>
+
+#include <gwproxy/net.h>
+
+#ifndef __packed
+#define __packed __attribute__((__packed__))
+#endif
+
+/*
+ * 4. MESSAGES
+ * 4.1. Format
+ *
+ * All communications inside of the domain protocol are carried in a single
+ * format called a message. The top-level format of a message is divided
+ * into 5 sections (some of which may be empty in certain cases), shown below:
+ *
+ *     +---------------------+
+ *     |        Header       |
+ *     +---------------------+
+ *     |       Question      | the question for the name server
+ *     +---------------------+
+ *     |        Answer       | RRs answering the question
+ *     +---------------------+
+ *     |      Authority      | RRs pointing toward an authority
+ *     +---------------------+
+ *     |      Additional     | RRs holding additional information
+ *     +---------------------+
+ *
+ * These sections are defined in RFC 1035 §4.1. The Header section is always
+ * present and includes fields that specify which of the other sections follow,
+ * as well as metadata such as whether the message is a query or response,
+ * the opcode, etc.
+ */
+
+/* Flag bit position in little-endian machine */
+#define DNS_QR_BIT		0xF
+#define DNS_OPCODE_BIT		0xB	// 4-bit field
+#define DNS_AA_BIT		0xA
+#define DNS_TC_BIT		0x9
+#define DNS_RD_BIT		0x8
+#define DNS_RA_BIT		0x7
+#define DNS_Z_BIT		0x4	// 3-bit field
+#define DNS_RCODE_BIT		0x0	// 4-bit field
+#define DNS_COMPRESSION_BIT	(0x3 << 0xE)
+
+/* Flag extraction macros for listtle-endian machine */
+#define DNS_QR(flags)		(((flags) >> DNS_QR_BIT) & 0x1)
+#define DNS_OPCODE(flags)	(((flags) >> DNS_OPCODE_BIT) & 0xF)
+#define DNS_RCODE(flags)	((flags) & 0xF)
+#define DNS_IS_COMPRESSED(mask) ((mask) & DNS_COMPRESSION_BIT)
+
+/* Flag construction macros for little-endian machine */
+#define DNS_SET_RD(flags, val)	(flags) = ((flags) & ~(1 << DNS_RD_BIT)) | ((!!(val)) << DNS_RD_BIT)
+
+/* as per RFC 1035 §2.3.4. Size limits */
+#define DOMAIN_LABEL_LIMIT 63
+#define DOMAIN_NAME_LIMIT 255
+#define UDP_MSG_LIMIT 512
+
+typedef enum {
+	OPCODE_QUERY		= 0,	// Standard query (QUERY)
+	OPCODE_IQUERY		= 1,	// Inverse query (IQUERY)
+	OPCODE_STATUS		= 2,	// Server status request (STATUS)
+	OPCODE_RESERVED_MIN	= 3,	// Reserved for future use (inclusive)
+	OPCODE_RESERVED_MAX	= 15	// Reserved for future use (inclusive)
+} gwdns_op;
+
+typedef enum {
+	TYPE_A		= 1,	// IPv4 host address
+	TYPE_NS		= 2,	// an authoritative name server
+	TYPE_CNAME	= 5,	// the canonical name for an alias
+	TYPE_SOA	= 6,	// marks the start of a zone of authority
+	TYPE_MB		= 7,	// a mailbox domain name (EXPERIMENTAL)
+	TYPE_MG		= 8,	// a mail group member (EXPERIMENTAL)
+	TYPE_MR		= 9,	// a mail rename domain name (EXPERIMENTAL)
+	TYPE_NULL	= 10,	// a null RR (EXPERIMENTAL)
+	TYPE_WKS	= 11,	// a well known service description
+	TYPE_PTR	= 12,	// a domain name pointer
+	TYPE_HINFO	= 13,	// host information
+	TYPE_MINFO	= 14,	// mailbox or mail list information
+	TYPE_MX		= 15,	// mail exchange
+	TYPE_TXT	= 16,	// text strings
+	TYPE_AAAA	= 28,	// IPv6 host address
+	QTYPE_AXFR	= 252,	// A request for a transfer of an entire zone
+	QTYPE_MAILB	= 253,	// A request for mailbox-related records (MB, MG or MR)
+	QTYPE_ALL	= 255	// A request for all records
+} gwdns_type;
+
+typedef enum {
+	CLASS_IN	= 1,	// Internet
+	CLASS_CH	= 3,	// CHAOS class
+	CLASS_HS	= 4,	// Hesiod
+	QCLASS_ANY	= 255	// ANY class (matches any class)
+} gwdns_class;
+
+typedef struct {
+	uint16_t id;
+	uint16_t flags;
+	uint16_t qdcount;
+	uint16_t ancount;
+	uint16_t nscount;
+	uint16_t arcount;
+} __packed gwdns_header_pkt;
+
+typedef struct {
+	uint8_t question[UDP_MSG_LIMIT];
+	char answr[UDP_MSG_LIMIT];
+} gwdns_question_buffer;
+
+typedef struct {
+	uint8_t *dst_buffer;
+	uint16_t txid;
+	uint16_t type;
+	size_t dst_len;
+	const char *domain;
+} gwdns_question_part;
+
+/*
+ * 4.1.3. Resource record format
+ *
+ * The answer, authority, and additional sections all share the same
+ * format: a variable number of resource records, where the number of
+ * records is specified in the corresponding count field in the header.
+ */
+typedef struct {
+	uint8_t  *name;		// DOMAIN NAME: variable‑length sequence of labels (length-byte followed by label, ending in 0), possibly compressed
+	uint16_t  rr_type;	// TYPE: two-octet code identifying the RR type (see gwdns_type)
+	uint16_t  rr_class;	// CLASS: two-octet code identifying the RR class (see gwdns_class)
+	uint32_t  ttl;		// TTL: 32-bit unsigned, time to live in seconds
+	uint16_t  rdlength;	// RDLENGTH: length in octets of RDATA
+	uint8_t  *rdata;	// RDATA: variable-length data, format depends on TYPE and CLASS
+} gwdns_serialized_rr;
+
+typedef struct {
+	char qname[DOMAIN_NAME_LIMIT];
+	uint16_t qtype;
+	uint16_t qclass;
+} gwdns_serialized_question;
+
+typedef gwdns_serialized_rr gwdns_serialized_answ;
+
+typedef struct {
+	gwdns_header_pkt hdr;
+	uint8_t body[UDP_MSG_LIMIT];
+} gwdns_query_pkt;
+
+typedef struct {
+	gwdns_header_pkt hdr;
+	gwdns_serialized_question question;
+	gwdns_serialized_answ **rr_answ;
+} gwdns_answ_data;
+
+struct gwdns_addrinfo_node {
+	int				ai_family;
+	int				ai_ttl;
+	socklen_t			ai_addrlen;
+	struct gwp_sockaddr		ai_addr;
+	struct gwdns_addrinfo_node	*ai_next;
+};
+
+/*
+ * Build standard query for domain name lookup.
+ *
+ * The caller may need to check for potential transaction ID collisions.
+ *
+ * Possible errors are:
+ * - ENAMETOOLONG	name is too long.
+ * - ENOBUFS		length specified by out_len is not sufficient.
+ * - EINVAL		malformed name or unsupported value of family.
+ *
+ * @param txid		transaction id
+ * @param name		domain name
+ * @param family	choose request for IPv4 or IPv6
+ * @param out		destination buffer for constructed packet
+ * @param out_len	available capacity of destination buffer
+ * @return		length of bytes written into dst_buffer on success,
+ * 			or a negative integer on failure.
+ */
+ssize_t gwdns_build_query(uint16_t txid, const char *name, int family, uint8_t *out, size_t out_len);
+
+/*
+ * Parse name server's answer
+ *
+ * Possible errors are:
+ * -EAGAIN	in buffer is not sufficient, no bytes are processed, need more data.
+ * -EINVAL	the content of in buffer is not valid.
+ * -ENOMEM	failed to allocate dynamic memory.
+ * -ENODATA	the packet didn't contain any answers.
+ * -EPROTO	the DNS server can't understand your question
+ *
+ * @param txid		transaction id of question
+ * @param service	port number in ascii
+ * @param in		a pointer to buffer that need to be parsed
+ * @param in_len	a pointer to buffer that need to be parsed
+ * @param ai		a pointer to address info
+ * @return		zero on success or a negative number on failure
+ */
+int gwdns_parse_query(uint16_t txid, const char *service,
+			uint8_t *in, size_t in_len,
+			struct gwdns_addrinfo_node **ai);
+void gwdns_free_parsed_query(struct gwdns_addrinfo_node *addrinfo);
-- 
Ahmad Gani

Re: [PATCH gwproxy v5 1/2] dns: Allow only port string number

[Alviro Iskandar Setiawan]

On Thu, 14 Aug 2025 11:46:54 +0700, Ahmad Gani wrote:
> Instead of adding additional work for parsing well-known services like
> http, https, etc... (port in socks5 field is a number anyway).
>
> Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Thu, Aug 14, 2025 at 11:53 AM Ahmad Gani wrote:
> +static int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
> +{
> +       gwdns_header_pkt *hdr;
> +       uint16_t raw_flags;
> +       size_t idx, i;
> +       void *ptr;
> +       int ret;
> +
> +       idx = sizeof(*hdr);
> +       if (idx >= in_len)
> +               return -EAGAIN;
> +
> +       hdr = (void *)in;
> +       if (memcmp(&txid, &hdr->id, sizeof(txid)))
> +               return -EINVAL;
> +
> +       memcpy(&raw_flags, &hdr->flags, sizeof(raw_flags));
> +       raw_flags = ntohs(raw_flags);
> +       /* QR MUST 1 = response from dns server */
> +       if (!DNS_QR(raw_flags))
> +               return -EINVAL;
> +
> +       /* OPCODE MUST 0 = standard query */
> +       if (DNS_OPCODE(raw_flags))
> +               return -EINVAL;
> +
> +       /* RCODE MUST 0 = No error */
> +       if (DNS_RCODE(raw_flags))
> +               return -EPROTO;
> +
> +       // is it safe or recommended to alter the in buffer directly?
> +       hdr->ancount = ntohs(hdr->ancount);
> +       if (!hdr->ancount)
> +               return -ENODATA;
> +
> +       /*
> +        * Check the sizes upfront.
> +        *
> +        * 1 bytes for variable-length
> +        * in[idx] for the length of first name
> +        * 1 bytes for null terminator
> +        * 2 bytes for qtype
> +        * 2 bytes for qclass
> +        */
> +       if ((size_t)(1 + in[idx] + 1 + 2 + 2) >= in_len)
> +               return -EINVAL;
> +
> +       ret = calculate_question_len(&in[idx], in_len - idx);
> +       if (ret <= 0)
> +               return -EINVAL;
> +
> +       idx += ret;
> +       if (idx >= in_len)
> +               return -EAGAIN;
> +
> +       out->hdr.ancount = 0;
> +       ptr = malloc(hdr->ancount * sizeof(uint8_t *));
> +       if (!ptr)
> +               return -ENOMEM;
> +
> +       out->rr_answ = ptr;
> +       for (i = 0; i < hdr->ancount; i++) {
> +               uint16_t is_compressed, rdlength;
> +               gwdns_serialized_answ *item = malloc(sizeof(gwdns_serialized_answ));
> +               if (!item) {
> +                       ret = -ENOMEM;
> +                       goto exit_free;
> +               }
> +
> +               memcpy(&is_compressed, &in[idx], sizeof(is_compressed));
> +               is_compressed = DNS_IS_COMPRESSED(ntohs(is_compressed));
> +               assert(is_compressed);
> +               idx += 2; // NAME
> +               if (idx >= in_len) {
> +                       ret = -EAGAIN;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +
> +               memcpy(&item->rr_type, &in[idx], 2);
> +               item->rr_type = ntohs(item->rr_type);
> +               idx += 2; // TYPE
> +               if (idx >= in_len) {
> +                       ret = -EAGAIN;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +               memcpy(&item->rr_class, &in[idx], 2);
> +               item->rr_class = ntohs(item->rr_class);
> +               idx += 2; // CLASS
> +               if (idx >= in_len) {
> +                       ret = -EAGAIN;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +               memcpy(&item->ttl, &in[idx], 4);
> +               item->ttl = be32toh(item->ttl);
> +               idx += 4; // TTL
> +               if (idx >= in_len) {
> +                       ret = -EAGAIN;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +
> +               memcpy(&rdlength, &in[idx], sizeof(rdlength));
> +               rdlength = ntohs(rdlength);
> +               if (item->rr_type != TYPE_AAAA && item->rr_type != TYPE_A) {
> +                       ret = -EINVAL;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +               if (item->rr_type == TYPE_AAAA && rdlength != sizeof(struct in6_addr)) {
> +                       ret = -EINVAL;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +               if (item->rr_type == TYPE_A && rdlength != sizeof(struct in_addr)) {
> +                       ret = -EINVAL;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +               item->rdlength = rdlength;
> +               idx += 2;
> +               if (idx >= in_len) {
> +                       ret = -EAGAIN;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +
> +               /*
> +                * considering if condition above,
> +                * maybe we don't need a malloc and just allocate fixed size
> +                * for rdata? however if this parser want to be expanded for
> +                * other dns operation (e.g OPCODE_IQUERY, etc), rdata maybe
> +                * contain more than sizeof in6_addr.
> +                */
> +               ptr = malloc(rdlength);
> +               if (!ptr) {
> +                       ret = -ENOMEM;
> +                       free(item);
> +                       goto exit_free;
> +               }
> +
> +               memcpy(ptr, &in[idx], rdlength);
> +               idx += rdlength;
> +               if (idx > in_len) {
> +                       ret = -EINVAL;
> +                       free(item);
> +                       free(ptr);
> +                       goto exit_free;
> +               }
> +
> +               item->rdata = ptr;
> +               out->rr_answ[i] = item;
> +               out->hdr.ancount++;
> +       }
> +
> +       return 0;
> +exit_free:
> +       for (i = 0; i < out->hdr.ancount; i++) {
> +               free(out->rr_answ[i]->rdata);
> +               free(out->rr_answ[i]);
> +       }
> +       free(out->rr_answ);
> +       return ret;
> +}

I think free(item); can be deduplicated. It's repeated several times
here. Consider using goto exit_free_item and add the appropriate
label. Do the same to other places if you have the same pattern
elsewhere.

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Thu, Aug 14, 2025 at 2:28 PM Alviro Iskandar Setiawan wrote:
> I think free(item); can be deduplicated. It's repeated several times
> here. Consider using goto exit_free_item and add the appropriate
> label. Do the same to other places if you have the same pattern
> elsewhere.

Oh, that's inside a for loop. Sorry, ignore my comment. That's indeed
not so good to be deduplicated. I thought it was not part of a
for-loop.

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Thu, Aug 14, 2025 at 11:46:55AM +0700, Ahmad Gani wrote:
> Introducing dns parser for better flexibility over the program flow, the
> DNS protocol implementation is limited to standard query (OPCODE_QUERY).
> 
> Also, Delegate creation of transaction id to the caller, It gives the
> caller more flexibility in handling DNS requests.
> 
> A single domain name lookup can have two responses: one for IPv4 and one
> for IPv6. With this change, the caller may use the same txid for both
> queries, making it easier to group related DNS responses.

Now the question is: How do we integrate this?

I see no Makefile changes. I guess it's still not covered by gwproxy
build system.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Sat, Aug 16, 2025 at 11:03 PM Ammar Faizi wrote:
> Now the question is: How do we integrate this?
>
> I see no Makefile changes. I guess it's still not covered by gwproxy
> build system.

I’ve managed to integrate it into gwproxy’s epoll event loop
and successfully run a simple single-query test with cURL.

However, the integration process isn't fully completed and still requires
significant changes across `dns.c`, `gwproxy.c`, and `epoll.c`.

So, It’s still a WIP.

Here’s the gist of how I integrated the parser:
- A non-blocking UDP socket file descriptor is created each time
`gwp_dns_queue` is called.
- A standard DNS query packet is sent over the network through that socket.
- The UDP socket file descriptor is registered with epoll.
- On epoll notification, the response is received.
- The response is parsed and returned as a reply to the SOCKS5
`CONNECT` command.

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sat, Aug 16, 2025 at 11:30:09PM +0700, Ahmad Gani wrote:
> On Sat, Aug 16, 2025 at 11:03 PM Ammar Faizi wrote:
> > Now the question is: How do we integrate this?
> >
> > I see no Makefile changes. I guess it's still not covered by gwproxy
> > build system.
> 
> I’ve managed to integrate it into gwproxy’s epoll event loop
> and successfully run a simple single-query test with cURL.
> 
> However, the integration process isn't fully completed and still requires
> significant changes across `dns.c`, `gwproxy.c`, and `epoll.c`.
> 
> So, It’s still a WIP.
> 
> Here’s the gist of how I integrated the parser:
> - A non-blocking UDP socket file descriptor is created each time
> `gwp_dns_queue` is called.
> - A standard DNS query packet is sent over the network through that socket.
> - The UDP socket file descriptor is registered with epoll.
> - On epoll notification, the response is received.
> - The response is parsed and returned as a reply to the SOCKS5
> `CONNECT` command.

Nice, sounds good. Looking forward to the updates!

For now, I take the manpage change only from this series.

-- 
Ammar Faizi

Re: (subset) [PATCH gwproxy v5 0/2] Initial work on integration of DNS parser lib in gwproxy

[Ammar Faizi]

On Thu, 14 Aug 2025 11:46:53 +0700, Ahmad Gani wrote:
> This is revision v5 of initial work on integration of DNS parser lib in
> gwproxy. This is an RFC draft, the patches itself aren't final.
> 
> With current model, the caller is responsible to do the socket part, this
> imply unlike conventional glibc's getaddrinfo that handle both socket and
> crafting payload inside its abstraction, thus allow AF_UNSPEC, the build
> query only craft packet for particular family at a time [1], either
> AF_INET or AF_INET6, not both, I see this not as a limitation, but rather
> an advantage, most of the time we don't need to request for both IPv6 and
> IPv4, but there's still a case where both family is desired, it's often
> happened for fallback mechanism. I'll spend some time thinking about how
> to integrate these functions into gwproxy.
> 
> [...]

Applied, thanks!

[1/2] dns: Allow only port string number
      commit: efad824958d533fc943c2b0ac27f76d20f0d3957

Best regards,
-- 
Ammar Faizi <ammarfaizi2@gnuweeb.org>

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Sat, Aug 16, 2025 at 11:30 PM Ahmad Gani wrote:
> So, It’s still a WIP.
>
> Here’s the gist of how I integrated the parser:
> - A non-blocking UDP socket file descriptor is created each time
> `gwp_dns_queue` is called.
> - A standard DNS query packet is sent over the network through that socket.
> - The UDP socket file descriptor is registered with epoll.
> - On epoll notification, the response is received.
> - The response is parsed and returned as a reply to the SOCKS5
> `CONNECT` command.

Do you have a git tree for the public to fetch your WIP?

I've just been back to my home. I'd like to take a look into it. I may
also continue the work if you don't have time. I am free for the rest
of the week, so I'll use it to tackle this one.

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

On Thu, Aug 21, 2025 at 11:50 PM Alviro Iskandar Setiawan wrote:
> Do you have a git tree for the public to fetch your WIP?

I just pushed a new branch at my github fork of gwproxy [1], you can take
a look if you want in the branch refactor-dns-feature.

The work-in-progress contain several TODOs:
- fix the parser (watch only for A record, ignore others like CNAME)
- handle both IPv6 and IPv4 when preferred family is AF_UNSPEC (see
dns.c:480 and dns.c:426)
- insert cache and house-keeping cache
- special domain name handling (e.g., allow localhost but refuse
.onion) and /etc/hosts
- integration with io_uring
- polishing the overall changes

> I've just been back to my home. I'd like to take a look into it. I may
> also continue the work if you don't have time. I am free for the rest
> of the week, so I'll use it to tackle this one.

Actually, I do have plenty of time. But I tend to lose focus when things
don't go well, and then it's hard for me to get back into it.

Maybe it'll be a different story if I talk things out and discuss it with
other
gwproxy contributors, but argh, it's hard to explain—or to change my habits.

Maybe, if you don't mind, it'd be nice to do it in more of a Q&A style
instead
of you continuing the work all by yourself. That way, I might actually start
changing my own habits, but only if you're comfortable with that suggestion.

[1]:
- https://github.com/realyukii/gwproxy/tree/refactor-dns-feature

--
Ahmad Gani

[-- Attachment #2: Type: text/html, Size: 1893 bytes --]

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Fri, Aug 22, 2025 at 12:44 PM Ahmad Gani wrote:
> I just pushed a new branch at my github fork of gwproxy [1], you can take
> a look if you want in the branch refactor-dns-feature.

Nice, tq. I will take a look after the Jackson Hole speech.

> Actually, I do have plenty of time. But I tend to lose focus when things
> don't go well, and then it's hard for me to get back into it.
>
> Maybe it'll be a different story if I talk things out and discuss it with other
> gwproxy contributors, but argh, it's hard to explain—or to change my habits.

Then do. We're here to discuss it.

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Fri, Aug 22, 2025 at 8:48 PM Alviro Iskandar Setiawan wrote:
> On Fri, Aug 22, 2025 at 12:44 PM Ahmad Gani wrote:
> > I just pushed a new branch at my github fork of gwproxy [1], you can take
> > a look if you want in the branch refactor-dns-feature.
>
> Nice, tq. I will take a look after the Jackson Hole speech.

(( The Jackson Hole's speech was very dovish, congrats if you went long ;-) ))

So, I have taken a look into your branch. And I saw commit
a02b58fe34ed ("dns: Remove code block related to the usage of glibc's
getaddrinfo_a function"). I am a little bit worried about that part.

My suggestion would be to create a new option in the configure file to
select what resolver type to be used. Let's mark this new feature as
experimental and keep the old feature for now. If your feature has
become stable enough, we can move forward abandoning getaddrinfo_a()
entirely.

Maybe './configure --use-raw-dns-resolver' or something else to enable
your experimental feature. If you go that way, don't forget to update
the GitHub CI to test your new feature.

What do you think?

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sat, Aug 23, 2025 at 02:09:34AM +0700, Alviro Iskandar Setiawan wrote:
> (( The Jackson Hole's speech was very dovish, congrats if you went long ;-) ))

Lol. No need to bring the portofolio discussion to this thread. Ahmad
won't care about it anyway.

> My suggestion would be to create a new option in the configure file to
> select what resolver type to be used. Let's mark this new feature as
> experimental and keep the old feature for now. If your feature has
> become stable enough, we can move forward abandoning getaddrinfo_a()
> entirely.

Agree with your idea. The same with io_uring feature in gwproxy. I
assume the io_uring feature is still experimental and it is not enabled
by default. We will probably never delete epoll code, forever. But the
old DNS code may be deleted later if this one goes well.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Sat, Aug 23, 2025 at 2:52 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
> On Sat, Aug 23, 2025 at 02:09:34AM +0700, Alviro Iskandar Setiawan wrote:
> > (( The Jackson Hole's speech was very dovish, congrats if you went long ;-) ))
>
> Lol. No need to bring the portofolio discussion to this thread. Ahmad
> won't care about it anyway.

Yeah, I might be interested if I were rich and have a truckload of money :-)

> > My suggestion would be to create a new option in the configure file to
> > select what resolver type to be used. Let's mark this new feature as
> > experimental and keep the old feature for now. If your feature has
> > become stable enough, we can move forward abandoning getaddrinfo_a()
> > entirely.
>
> Agree with your idea. The same with io_uring feature in gwproxy. I
> assume the io_uring feature is still experimental and it is not enabled
> by default. We will probably never delete epoll code, forever. But the
> old DNS code may be deleted later if this one goes well.

I think it should be fine to have old DNS code removed if the raw DNS
resolver feature is only applied when it's completed and tested.

You can still keep the old DNS feature in the master branch that is meant
to be stable (?)

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sat, Aug 23, 2025 at 08:07:08AM +0700, Ahmad Gani wrote:
> I think it should be fine to have old DNS code removed if the raw DNS
> resolver feature is only applied when it's completed and tested.

I will ask for your feature to be experimental because your DNS feature
is not super simple. It may have undiscovered hidden bugs even if we
have extensively tested it. The same thing with the io_uring support.

And note that, getaddrinfo() is not as simple as resolving DNS queries
via UDP port 53. It does more than that. For example, it reads
/etc/hosts, /etc/resolv.conf, /etc/gai.conf and many more files.

If we missed very important details and the old resolver is already
removed, it will be a pain in the ass to 'git revert' all your work.

So please, keep the old feature intact for the time being. Develop a
new DNS resolver without bothering the old DNS feature. It's not that
simple to replace this monster.

And the good thing is that, making it an experimental feature allows
specific environments to use the feature in a stable release, but
keeping the old stable feature usable for those who are not willing to
get involved in testing and reporting bugs.

JFYI, I have started using the epoll version in a real production. And
that one still uses the epoll version because I am more confident with
my epoll version than io_uring version.

Next time, when I am confident enough with your DNS feature, I will
consider making your DNS resolver the primary one and probably kill the
old DNS resolver.

We also have a lot of features like that in the Linux kernel, for
example:

  - CONFIG_BTRFS_EXPERIMENTAL
  - CONFIG_IEEE802154_NL802154_EXPERIMENTAL
  - CONFIG_LTO (yes, LTO in the kernel is marked as experimental)
  - CONFIG_AUTOFDO_CLANG
  - any many more...

Those are unstable features, but can be configured to run on stable
kernel. I want to follow that tradition when it comes to stability of a
new feature.

> You can still keep the old DNS feature in the master branch that is meant
> to be stable (?)

I don't consider the master branch to be stable. But a tag is.
Currently we HAVE NO TAGS in gwproxy. Soon, I'll make a release.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Sat, Aug 23, 2025 at 8:52 AM Ammar Faizi wrote:
> I will ask for your feature to be experimental because your DNS feature
> is not super simple. It may have undiscovered hidden bugs even if we
> have extensively tested it. The same thing with the io_uring support.
>
> And note that, getaddrinfo() is not as simple as resolving DNS queries
> via UDP port 53. It does more than that. For example, it reads
> /etc/hosts, /etc/resolv.conf, /etc/gai.conf and many more files.
>
> If we missed very important details and the old resolver is already
> removed, it will be a pain in the ass to 'git revert' all your work.
>
> So please, keep the old feature intact for the time being. Develop a
> new DNS resolver without bothering the old DNS feature. It's not that
> simple to replace this monster.
>
> And the good thing is that, making it an experimental feature allows
> specific environments to use the feature in a stable release, but
> keeping the old stable feature usable for those who are not willing to
> get involved in testing and reporting bugs.

Alright, that sounds reasonable. I’ll revert any changes that remove the
old DNS resolver part.

One question though: should the option be compile-time
with a -dev binary, or runtime via command-line parameters?

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Alviro Iskandar Setiawan]

On Sat, Aug 23, 2025 at 8:07 AM Ahmad Gani wrote:
> I think it should be fine to have old DNS code removed if the raw DNS
> resolver feature is only applied when it's completed and tested.

It does not have to be complete to get merged into master. Unfinished
work is ok, partially usable is fine, as long as it does not break the
build and not leave a known bug.

I think the UDP 53 resolver can be merged earlier, then later you add
/etc/hosts support, and other missing bits after that.

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sat, Aug 23, 2025 at 09:17:20AM +0700, Ahmad Gani wrote:
> One question though: should the option be compile-time
> with a -dev binary, or runtime via command-line parameters?

Better be a new command line option and a configure script to enable
it. Just like the io_uring support.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sat, Aug 23, 2025 at 09:20:17AM +0700, Alviro Iskandar Setiawan wrote:
> I think the UDP 53 resolver can be merged earlier, then later you add
> /etc/hosts support, and other missing bits after that.

Yes, let's only do that "UDP 53 resolver" first for the initial work.

Ahmad, I'll apply your patch series if your DNS resolver is already
usable from at least the epoll event loop.

io_uring support can be later, but you must not break it if you enable
your new experimental feature.

Your objective now is only focus on resolving DNS via UDP port 53. Make
it usable from the epoll event loop.

Next work will be parsing these files:

  - /etc/hosts
  - /etc/resolv.conf
  - /etc/gai.conf

and probably more files. We don't need to wait those files to be parsed
by your feature to merge it.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Sat, Aug 23, 2025 at 9:28 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
> Ahmad, I'll apply your patch series if your DNS resolver is already
> usable from at least the epoll event loop.

By the way, you can call me Gani, as that’s what people usually call me.

> Your objective now is only focus on resolving DNS via UDP port 53. Make
> it usable from the epoll event loop.

I've reverted the previous working DNS code and marked the raw DNS backend
as experimental, with the feature disabled by default [1].

I'll avoid rebasing unless necessary to prevent frequent forced pushes to
the repository. In the end, the work will likely be squashed down to just
two commits:
1. Add DNS parser code
2. Add experimental raw DNS feature

Next, I'll work on fixing the parser issue. Right now, it can only handle
A and AAAA records, and doesn't handle CNAME records properly.

[1]:
- main changes:
https://github.com/realyukii/gwproxy/commit/a8d495342d993cbf813e3591070a619a55395ab8
- fix Github CI:
https://github.com/realyukii/gwproxy/commit/d5e508106faa42e2e39397f2756762ceabe5619c

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sun, Aug 24, 2025 at 08:36:24PM +0700, Ahmad Gani wrote:
> By the way, you can call me Gani, as that’s what people usually call me.

Noted.

> I'll avoid rebasing unless necessary to prevent frequent forced pushes to
> the repository.
 
Frequent force-push to your personal development branch is fine, just
don't do that on upstream master branch which people use it as their
base of their work.

> Next, I'll work on fixing the parser issue. Right now, it can only handle
> A and AAAA records, and doesn't handle CNAME records properly.

Good that you found that early. Better to sort those missing bits out
before it eventually breaks someone's setup in the future.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Sun, Aug 24, 2025 at 08:36:24PM +0700, Ahmad Gani wrote:
> Next, I'll work on fixing the parser issue. Right now, it can only handle
> A and AAAA records, and doesn't handle CNAME records properly.

FYI: I've merged the HTTP proxy support into the master branch.

Other features can now be developed independently of the DNS work.
Your DNS work is taking longer than I expected, and waiting for it
isn't productive. Your series no longer applies.

Please rebase your work on top of the latest master branch if you're
still planning to work on it. If you're not, let me know so someone
else can take it over.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Thu, Aug 28, 2025 at 7:13 AM Ammar Faizi wrote:
> FYI: I've merged the HTTP proxy support into the master branch.
>
> Other features can now be developed independently of the DNS work.
> Your DNS work is taking longer than I expected, and waiting for it
> isn't productive. Your series no longer applies.

Yeah, I know I haven't been productive for a week. I haven't touched it
since then. I'm not sure why I don't put more thought into it. I guess I
need to hang out in the VC once again so I have this feeling of responsibility
to get this done, and I also miss conversations with people in general.

> Please rebase your work on top of the latest master branch if you're
> still planning to work on it. If you're not, let me know so someone
> else can take it over.

Okay, I will rebase it. Sorry to have kept you waiting for so long.

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

BTW, How to deal with divergent branches in git?
```
pick 318a29c80ad3 # github: codespell: Add 'TE' word
pick 5f25e0dec782 # github: codespell: Only spellcheck man dir
pick 84b09581bf25 # configure: Add '-Wno-format-truncation' flag
pick abe0c1334a0f # github: Add 'fetch-depth: 0' to codespell target
pick ab6096090cc7 # dnsparser: Add dns parser code
```

I picked commit abe0c1334a0f and edited it. However, when I pulled from
the master branch, it prompted me to either merge, rebase, or fast-forward.
I'm not sure which option to use in this case.

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Thu, Aug 28, 2025 at 9:29 AM Ahmad Gani wrote:
> BTW, How to deal with divergent branches in git?

I asked an AI, and it told me to do it differently:
```
# Make sure your master is up to date
git checkout master
git pull upstream master

# Switch to your feature branch
git checkout refactor-dns-feature

# Rebase your work onto updated master
git rebase master
```

However, I'm not sure if this is the correct way. It's different from what
Sir Ammar taught to me.

But I guess it's the same as a combination of the above snippet with
`git pull origin master --rebase`?

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ahmad Gani]

On Thu, Aug 28, 2025 at 9:45 AM Ahmad Gani wrote:
> On Thu, Aug 28, 2025 at 9:29 AM Ahmad Gani wrote:
> > BTW, How to deal with divergent branches in git?
>
> I asked an AI, and it told me to do it differently

Anyway, I ended up following the AI suggestion, and now I'm resolving
the conflict.

On Thu, Aug 28, 2025 at 9:29 AM Ahmad Gani wrote:
> I picked commit abe0c1334a0f and edited it. However, when I pulled from
> the master branch, it prompted me to either merge, rebase, or fast-forward.
> I'm not sure which option to use in this case.

It seems the rebase option is the correct way; it puts the patch series
on top of the newest commits from the master branch.

--
Ahmad Gani

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Thu, Aug 28, 2025 at 8:52 AM Ahmad Gani wrote:
> Yeah, I know I haven't been productive for a week. I haven't touched it
> since then. I'm not sure why I don't put more thought into it. I guess I
> need to hang out in the VC once again so I have this feeling of responsibility
> to get this done, and I also miss conversations with people in general.

I don't usually start a voice chat, but if you invite me when I don’t
have other work to attend to, I would be ok to join.

-- 
Ammar Faizi

Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code

[Ammar Faizi]

On Thu, Aug 28, 2025 at 9:46 AM Ahmad Gani wrote:
> However, I'm not sure if this is the correct way. It's different from what
> Sir Ammar taught to me.

Don't blame a different approach. git-rebase covers a wide topic. Not
all problems have the same exact solution.

> But I guess it's the same as a combination of the above snippet with
> `git pull origin master --rebase`?

Yes, that command will end up giving the same result, except that it
does not update your local master branch. The former commands do.

-- 
Ammar Faizi