[reginaldalfret]

Summary

In Bundler.BundleFromRemote, format the remote dial target using net.JoinHostPort(host, "443") instead of plain string concatenation (host + ":443").

When ip (or serverName with no ip) is an IPv6 literal (such as 2001:db8::1), plain concatenation produces an invalid address (2001:db8::1:443), causing the dialer to fail with too many colons in address. net.JoinHostPort automatically brackets IPv6 literals ([2001:db8::1]:443) while preserving hostnames and IPv4 addresses unchanged.

Also adds an explicit IPv6 test case to TestBundleFromRemote verifying that [2001:db8::1]:443 is passed to the TLS dialer.

Resolves #1448.

[ang-cloudflare]

@reginaldalfret Thanks for picking this up. The bare IPv6 fix works, and the new test fails on the old code as it should. I left one non-blocking comment inline: already-bracketed IPv6 input now gets double-bracketed.

[ang-cloudflare]

This breaks IPv6 input that's already in brackets. net.JoinHostPort adds brackets to any host that contains :, even if it's already bracketed. So -ip '[2001:db8::1]' or -domain '[2001:db8::1]' (or the same values in the /api/v1/cfssl/bundle JSON) now becomes [[2001:db8::1]]:443. Both dial attempts then fail with code 6000, missing port in address.

With the old concatenation that input produced [2001:db8::1]:443 and worked end to end: SNI is skipped, and VerifyHostname strips the brackets and matches the IP SAN. Bare IPv6 failed with too many colons, so the bracketed form was the only IPv6 literal that worked before. Nothing between the caller and here normalizes the input (cli/bundle/bundle.go:71, api/bundle/bundle.go:56). The suggested fix in #1448 has the same gap.

Tested against a real TLS server on [::]:443:

input	before	this PR	with strip below
ip [::1]	OK	ERR [[::1]]:443: missing port in address	OK
domain [::1], no ip	OK	ERR	OK
ip [::1%lo0]	OK	ERR	OK
ip/domain ::1	ERR too many colons	OK	OK

Suggest stripping one pair of enclosing brackets before the join. Leave serverName as is, since SNI and VerifyHostname already handle brackets:

host := serverName
if ip != "" {
	host = ip
}
if len(host) >= 3 && host[0] == '[' && host[len(host)-1] == ']' {
	host = host[1 : len(host)-1]
}
dialName := net.JoinHostPort(host, "443")

I'd avoid strings.Trim(host, "[]"), because it also strips unbalanced or repeated brackets. Odd inputs such as [foo, [[::1]] and [] dial exactly what they did before.

Two rows in TestBundleFromRemoteDialErrors would pin this. They need no IP SAN and no SNI changes. The first also covers the domain-only IPv6 path from #1448:

{name: "IPv6 literal as hostname", hostname: "2001:db8::1", wantDialName: "[2001:db8::1]:443"},
{name: "bracketed IPv6 literal", ip: "[2001:db8::1]", wantDialName: "[2001:db8::1]:443"},

Not blocking: the bracketed form was never documented, and ocspserve accepted the same break in #1187. But the fix is small and keeps existing IPv6 users working.

[ang-cloudflare]

@reginaldalfret I opened reginaldalfret#1 against your branch with the bracket-strip change and the two test rows, in case that's easier than applying it by hand. Merging it updates this PR. Feel free to squash it into your commit.

[reginaldalfret]

Thanks @ang-cloudflare! I have merged your follow-up PR reginaldalfret#1 into this branch. It strips one valid enclosing bracket pair before
et.JoinHostPort to keep already-bracketed IPv6 literals working without double-bracketing, and adds regression test coverage for bare and bracketed IPv6 inputs. Verified that go test ./bundler/... and go vet ./bundler/... pass cleanly.