Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid double-marshal'ing strings (uint byte arrays) #471

Merged
merged 3 commits into from
Nov 1, 2024

Conversation

phillip-stephens
Copy link
Contributor

@phillip-stephens phillip-stephens commented Nov 1, 2024

Closes #463
Since strings are represented as uint8 arrays, we were double-marshal'ing the string here. This caused the bug seen where when you decoded the base-64 encoded string, you got another base-64 encoded string in quotes.

Testing

$ echo defo.ie | ./zdns HTTPS --name-servers 8.8.8.8                                                                                         

{"name":"defo.ie","results":{"HTTPS":{"data":{"additionals":[{"flags":"","type":"EDNS0","udpsize":512,"version":0}],"answers":[{"class":"IN","name":"defo.ie","priority":1,"svcparams":{"ech":"AED+DQA8awAgACD/2gZ+zTEEWYcDbLHxYEmDqk8SJPcAu2p3EOCdkPQMdAAEAAEAAQANY292ZXIuZGVmby5pZQAA","ipv4hint":["213.108.108.101"],"ipv6hint":["2a00:c6c0:0:116:5::10"]},"target":".","ttl":1800,"type":"HTTPS"}],"protocol":"udp","resolver":"8.8.8.8:53"},"duration":0.297062125,"status":"NOERROR","timestamp":"2024-11-01T13:13:11-04:00"}}}

$ echo "AED+DQA8awAgACD/2gZ+zTEEWYcDbLHxYEmDqk8SJPcAu2p3EOCdkPQMdAAEAAEAAQANY292ZXIuZGVmby5pZQAA" | base64 -d
<k  ��~�1Y�l��`I��O$��jw����
cover.defo.ie%   

This also fixes where we were base-64 encoding many strings when printing the TLS handshake

~/zdns on  main! ⌚ 13:25:38
$ echo "cloudflare.com" | ./zdns A --threads=2 --https --verify-server-cert --root-cas-file="/Users/phillip/Desktop/macos-root-certs.pem"  | grep -o -E '.{0,5}:\"I.{0,5}' | wc -l
      80 # 80 strings were doubly base-64 encoded

~/zdns on  phillip/463-double-encode-bug! ⌚ 13:25:50
$ echo "cloudflare.com" | ./zdns A --threads=2 --https --verify-server-cert --root-cas-file="/Users/phillip/Desktop/macos-root-certs.pem"  | grep -o -E '.{0,5}:\"I.{0,5}' | wc -l
       2


$ echo "cloudflare.com" | ./zdns A --threads=2 --https --verify-server-cert --root-cas-file="/Users/phillip/Desktop/macos-root-certs.pem"  | grep -o -E '.{0,5}:\"I.{0,5}'
lass":"IN","n
lass":"IN","n

@phillip-stephens phillip-stephens marked this pull request as ready for review November 1, 2024 17:28
@phillip-stephens phillip-stephens requested a review from a team as a code owner November 1, 2024 17:28
@zakird zakird merged commit e7cd062 into main Nov 1, 2024
3 checks passed
@zakird zakird deleted the phillip/463-double-encode-bug branch November 1, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] ECH value in HTTPS RR is double base64-encoded
2 participants