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.

Sign up for GitHub

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

allow domains of length 254 with a trailing dot #73

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

allow domains of length 254 with a trailing dot #73

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce1dfbd
allow domains of length 254 with a trailing dot
zacknewman Feb 8, 2025
e2548e3
Fix test for domains of length 254 with a trailing dot.
zacknewman Feb 8, 2025
2d7ff92
account for MSRV
zacknewman Feb 8, 2025
ad30c10
easier to understand and added links to the relevant rfc sections
zacknewman Feb 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/server_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,24 @@ const fn validate(input: &[u8]) -> Result<(), InvalidDnsNameError> {
/// "Labels must be 63 characters or less."
const MAX_LABEL_LENGTH: usize = 63;

/// https://devblogs.microsoft.com/oldnewthing/20120412-00/?p=7873
const MAX_NAME_LENGTH: usize = 253;

if input.len() > MAX_NAME_LENGTH {
return Err(InvalidDnsNameError);
/// The maximum lenght of a domain name is 253 when there is no trailing dot.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have chapter and verse from the relevant RFCs on this?

Copy link
Author

@zacknewman zacknewman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on what RFC you choose to adhere to. The complicated thing about this is there are so many variations of "representation format". The comment in code states that RFC 1035 is adhered to with the added allowance of underscores. First, does that comment mean § 2.3.1 (i.e., "preferred name syntax")? If so, then the code is wrong for allowing trailing dots at all. If that is indeed the goal, then the length check is fine; however an additional check that verifies there is not a trailing dot needs to be added.

Copy link
Author

@zacknewman zacknewman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue this relates to explains the problem. 255 is the maximum length a domain is allowed to be per § 2.3.4, but that refers to the wire format. When dealing with a representation format, you have to calculate what the wire-format length is from it. For "simple" representation formats (i.e., where you don't de-escape "characters" and where each "character" is encoded in 1 byte), the algorithm is as simple as adding 2 to the length when the domain does not contain a trailing dot or adding 1 when it does. From that, the max length of the former is clearly 253 since 253 + 2 = 255; meanwhile the max length of the latter is 254 since 254 + 1 = 255.

Copy link
Author

@zacknewman zacknewman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, this PR only addresses the incorrect/inconsistent treatment of trailing dots. Personally, I think using "preferred name syntax" is far too restrictive. The fact the code adds additional "characters" (specifically underscores) is indicative of this restriction. There is no real gain by being so restrictive since there are real domains that can never be accessed by clients that use rustls since rustls will incorrectly reject them as being invalid.

On the most extreme side, you could allow all ASCII domains. That is to say all domains whose labels consist of any and all ASCII sans . (since that is used as the label separator). A less extreme case would be allowing all "printable" ASCII. Or use what Servo's IDNA crate recommends and allow domains that conform to WHATWG's URL standard. All of those are better since you will allow rustls to be used by clients to access more valid domains than it incorrectly rejecting them. This is a tangential problem though; although answering the question "what domains in representation format do we want to allow?" is necessary to solve both the length bounds and the allowed "characters".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what problem are you actually solving here? It all seems pretty theoretical to me.

Copy link
Author

@zacknewman zacknewman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djc,

My actual issue is that rustls does not allow one to customize how DnsName parses domains in representation format. This means you literally cannot use rustls as a client-side TLS software suite if you ever wanted to access domains that it does not deem valid but in fact are. If you don't want to modify how DnsName parses domains in representation format, can we add another method/trait that defers to downstream code? I want my code to accept more domains that rustls doesn't, but ServerName does not allow me to pass a DnsName that is valid because there is no way for me to construct it without relying on DnsName::try_from_str. For example, if Firefox ever wanted to use rustls for its TLS-handling, it literally couldn't without sacrificing the ability to access certain domains that it can now (e.g., Firefox happily accesses domains that contain ; in a label).

I would like to construct a DnsName that is a valid domain whose labels consist of any printable ASCII (sans space and .) or perhaps at a minimum what WHATWG says is allowed (and thus what popular web browsers accept since I don't know why rustls shouldn't accept something that web browsers do) since those are valid domains per RFC 1035 (but of course may not be valid per the section on "preferred syntax" which rustls has already (partially) deviated from), but I cannot since DnsName::try_from_str is overly restrictive. This means changing its behavior to allow more domains, live with the fact that my code is overly restrictive and can't be used to access legitimate domains, add additional functionality that defers to downstream code that allows for more valid domains, or use native-tls or other crate that does what I want.

Clearly, this PR is one part of the "change its behavior to allow more domains". I was planning to create another "issue" asking if we could allow more characters; but if there is push back on something as simple (and "theoretically"/technically correct) as this, then surely there will be push back on allowing more characters (e.g., printable ASCII). Thus perhaps adding another associated method/trait that allows calling code to dictate what is valid would be the best approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means you literally cannot use rustls as a client-side TLS software suite if you ever wanted to access domains that it does not deem valid but in fact are.

Do you have a practical example of such a name -- specifically one that is allowed by RFC4985, issued in a certificate under cabforum BR 7.1.2.7.12, etc?

DnsName::try_from_str does not allow " " which is a valid domain per that section.

To be clear the DnsName type we have here exists to support the ServerName type, and ServerName contains the name or address for a TLS server from the view of a client. Therefore, there is little to be gained to allow a client to express not-useful values like " ", even if they are technically allowed and useful in DNS software.

I'm definitely open to updating references here to (probably) refer to https://www.rfc-editor.org/rfc/rfc9499.html#name-names -- a BCP which reflects modern usage better than RFC1035

Copy link
Author

@zacknewman zacknewman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a practical example of such a name -- specifically one that is allowed by RFC4985, issued in a certificate under cabforum BR 7.1.2.7.12, etc?

There was no mention of that RFC in the code which is why I keep citing RFC 1035. If the goal is to allow only those domains allowed by RFC 4985, then code should state that and importantly the validation function should be using it to determine what is a valid domain and not "preferred syntax" of RFC 1035 with the added allowance of underscores, added restriction that " " is forbidden, and unless this PR is accepted, an inconsistent stance on when trailing dots are allowed: normally yes despite violating "preferred syntax", but not allowed when the domain has length 254 despite being valid per RFC 1035. Since I'm not familiar with that RFC, I'll have to get back to you. I will say rustls should support "private" deployments. Thus if I can construct an X.509 v3 certificate for a domain that I control, I can create the appropriate AAAA/A record for said domain, and other very popular software (e.g., Firefox) have no problem accessing said domain and handling said certificate, then rustls should be able to as well, right? Point being, "practical example" is doing a lot of heavy-lifting. Do you require me to use a "popular" CA (e.g., Let's Encrypt) to issue the certificate?

Tangential, with popular CAs like Let's Encrypt planning to issue X.509 v3 certificates to IP addresses, do you know if rustls already/will support such certs?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cursory read of RFC 4985 seems to suggest that IP addresses are not allowed; thus rustls intends to not support such certs despite popular CAs like Let's Encrypt issuing them?

Copy link
Member

@cpu cpu Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.rs/rustls-pki-types/latest/rustls_pki_types/enum.ServerName.html#variant.IpAddress

https://docs.rs/rustls-pki-types/latest/rustls_pki_types/enum.IpAddr.html

https://github.com/rustls/rustls/blob/fa3e31746d5721b60df4d407c5df346d4ec7e423/ROADMAP.md?plain=1#L100-L106

/// When there is a trailing dot, the maximum length is 254. This is true since
/// a domain with a trailing dot gets transformed into the same wire-format value
/// as one without.
const MAX_NAME_LENGTH_WITH_TRAILING_DOT: usize = 254;

match input.len() {
0 | 255.. => return Err(InvalidDnsNameError),
MAX_NAME_LENGTH_WITH_TRAILING_DOT => if let Some(lst) = input.last() {
if *lst != b'.' {
return Err(InvalidDnsNameError);
}
} else {
// Ideally this would be `unreachable`, but that can't happen until
// MSRV is updated to a version where it is `const`.
panic!("there is a bug in [u8]::len()");
},
_ => (),
}

let mut idx = 0;
Expand Down Expand Up @@ -838,6 +851,7 @@ mod tests {
("numeric-only-middle-label.4.com", true),
("1000-sans.badssl.com", true),
("twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfiftythreecharacters.twohundredandfi", true),
("twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfour.", true),
("twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourcharacters.twohundredandfiftyfourc", false),
];

Expand Down
Loading