-
-
Notifications
You must be signed in to change notification settings - Fork 204
implement SVCB and HTTPS RRs #83
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
base: master
Are you sure you want to change the base?
Conversation
sometimes rustc warns but if you go back to implicit, then it errors out. make up your mind, rustc
cargo run -q -p dog -- HTTPS crypto.cloudflare.com --json \
| jq '.responses[] .answers[].data.params.ech' -r \
| cargo run -q -p ech-config \
| jq
|
Some notes:
|
| } | ||
|
|
||
| for byte in self.0.iter().copied() { | ||
| if byte < 32 || byte >= 128 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reminds me, I think dog's existing ascii escaping here is not quite correct, it's meant to be three digits unconditionally so fails for the byte < 32 case. If you slap this output in a zone file it may not parse. (Scroll down in the actual file)
| "mandatory": mandatory.iter().map(|x| x.to_string()).collect::<Vec<String>>(), | ||
| "alpn": alpn.as_ref().map(|x| x.ids.iter().map(|id| id.to_string()).collect::<Vec<String>>()), | ||
| "port": *port, | ||
| "no-default-alpn": alpn.as_ref().map(|x| x.no_default_alpn).unwrap_or(false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we prefer some kind of underscores convention or matching the presentation format in the RFC (hyphens as shown here)?
| sshfp: Cyan.normal(), | ||
| soa: Purple.normal(), | ||
| srv: Cyan.normal(), | ||
| svcb: Cyan.normal(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picked to match SRV records which these new ones are somewhat similar to.
| #[serde(untagged)] | ||
| pub enum ECHConfigContents { | ||
| // if version == 0xfe0d | ||
| Version0xfe0d { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call this Draft13 because that's what 0xfe0d means
| let inner = self.get_ref(); | ||
| let len = inner.len() as u64; | ||
| let start = self.position().min(len); | ||
| let end = (start + to_length).min(len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. This should obviously just error if the remaining slice is not big enough to hold the requested length. And return a wire error
Fixes #76
And piping the
--jsonoutput through my other little tool to decode the ECH configuration:cargo run -q -p dog -- HTTPS crypto.cloudflare.com --json \ | jq '.responses[] .answers[].data.params.ech' -r \ | cargo run -q -p ech-config \ | jq[ { "version": 65037, "contents": { "key_config": { "config_id": 138, "kem_id": "DHKEM_X25519_HKDF_SHA512", "public_key": "7MUqu7LT9RDPRTQPDoGxIneC9BDIpXXRPDyHOrPMWAs=", "cipher_suites": [ { "kdf_id": "HKDF_SHA256", "aead_id": "AES_128_GCM" } ] }, "maximum_name_length": 0, "public_name": "cloudflare-esni.com", "extensions": [] } } ]