Skip to content

Conversation

gulducat
Copy link
Member

@gulducat gulducat commented May 22, 2025

RFC-5942 section 4 recommends (basically) that IPv6 addresses should have their 0's squashed together, and letters lower-cased. E.g. A110:0:0::C8 should become a110::c8.

An audit for the USGv6 program* that turns this recommendation into a requirement found us outputting the verbatim-from-config address in the output of nomad agent, and I also found vault/consul addrs in logs (namely, from our passing them to consul-template).

I opted to normalize the addresses at config-parsing time, so the change runs through the whole system, rather than try to chase down each spot where we represent it visually. This is not without risk, so I could be persuaded to take a more conservative approach if desired.

* If you find the USGv6 reference hard to browse, you are not alone. The main document is here:
https://nvlpubs.nist.gov/nistpubs/specialpublications/NIST.SP.500-267Ar1.pdf
and the relevant part of it covered by this PR is section 4.1.1 and the table in 4.7.1

P.S. Related Vault PRs: hashicorp/vault#29228 & hashicorp/vault#29517

@gulducat gulducat marked this pull request as ready for review May 22, 2025 16:44
@gulducat gulducat requested review from a team as code owners May 22, 2025 16:44
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM

I sort of suspect we might have more problems of this sort with the server discovery configuration, where we do normalization only to pass into the go-discover library (see #24649) and then we don't touch it again. I guess as long as we don't output that in logs that's ok?

Longer-term, we're passing around strings in our codebase instead of network addresses (either IPs or IP/port pairs), which means we've potentially got parsing logic spread out everywhere instead of just here in the config and then a good standard-compliant String() function for printing to the user. But that's a much bigger project to tackle.

@gulducat gulducat merged commit 15c01e5 into main May 22, 2025
40 checks passed
@gulducat gulducat deleted the NMD-770-normalize-ipv6-addrs branch May 22, 2025 18:21
@gulducat gulducat added the backport/1.10.x backport to 1.10.x release line label Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.10.x backport to 1.10.x release line theme/ipv6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants