On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
Hostnames must comply with certain rules about maximum length and permitted characters. Since these need to be validated in several places, introduce a helper to centralize the rules and provide testing.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virutil.c | 50 ++++++++++++++++++++++++++++ src/util/virutil.h | 2 ++ tests/utiltest.c | 72 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 798d2d28c8..66520a3e35 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3791,6 +3791,7 @@ virGetUserRuntimeDirectory; virGetUserShell; virHostGetDRMRenderNode; virHostHasIOMMU; +virHostnameValidate; virIndexToDiskName; virKernelCmdlineMatchParam; virKernelCmdlineNextParam; diff --git a/src/util/virutil.c b/src/util/virutil.c index 2be9a460ac..75ecb6da5a 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1979,3 +1979,53 @@ virPipeNonBlock(int fds[2]) { return virPipeImpl(fds, true, true); } + +#define VIR_HOSTNAME_VALID_CHARS \ + "abcdefghijklmnopqrstuvwxyz" \ + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \ + "0123456789-" + +int virHostnameValidate(const char *hostname, bool qualified) +{ + g_auto(GStrv) labels = g_strsplit(hostname, ".", -1); + size_t i; + + if (!qualified && g_strv_length(labels) > 1) { + virReportError(VIR_ERR_INVALID_ARG, + _("Hostname '%1$s' must not be qualified"), + hostname); + return -1; + } + if (strlen(hostname) > 253) { + virReportError(VIR_ERR_INVALID_ARG, + _("Hostname '%1$s' must be 253 characters or less"), + hostname); + return -1; + } + + for (i = 0; labels[i] != NULL; i++) { + size_t labellen = strlen(labels[i]); + if (labellen == 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Hostname '%1$s' components must be non-empty"), + hostname); + return -1; + } + + if (labellen > 63) { + virReportError(VIR_ERR_INVALID_ARG, + _("Hostname '%1$s' components must be 63 characters or less"), + hostname); + return -1; + } + + if (strspn(labels[i], + VIR_HOSTNAME_VALID_CHARS) != labellen) { + virReportError(VIR_ERR_INVALID_ARG, + _("Hostname '%1$s' components must only contain a-z, A-Z, 0-9 and -"), + hostname); + return -1; + }
1) Additionally, although a hostname is now allowed to start with a number (originally it wasn't), it cannot start or end with a "-" (more specifically, each individual label cannot start or end with a "-". 2) In general, a domainname's octets can't be interpreted or limited by a DNS server; that is declared to be the responsibility of the application that is actually using the information. (e.g. a DNS A or AAAA record key can completely legitimately contain a "_", or any other character and the DNS server is supposed to accept that and forward it on or whatever). It's only when an application decides to use a domainname. And actually I've been reading RFC2181 for the first time and section 11 says "any binary string whatever can be used as the label of any resource record" (after first listing the length limitations you've checked for above). It also says "DNS server must not refuse to serve a zone because it contains labels that might not be acceptable to some DNS client programs" (oddly the "must" isn't capitalized, which has always had a special meaning in RFCs (at least to me). (https://www.rfc-editor.org/info/rfc2181/#section-11). So technically we *could* allow anything in a hostname and rely on the receiving client to validate. Not that I'm suggesting that, I just found it interesting that was the case :-) 3) I was curious about what happens with hostnames/domain names containing international characters. I've known for several years that it's now supported, but never looked at the details before. This is my chance! So, on the wire, only the characters you've specified are supported (in a hostname), and any character beyond 0x80 will cause the name to be translated into "Punycode"[*], which I won't try to describe fully, but it ends up with a string that starts with "xn--" and then does a special encoding of the source string that I'll let you look up for yourself. We *could* just say that libvirt only allows using the Punycode-encoded version of international hostnames in its config files (and thus your validation would remain the same, but with addition of checking leading and trailing "-"), but dnsmasq actually has support for international characters in hostnames and domainnames in its config files, and will automatically translate any hostname/domainname with non-ascii characters to Punycode before putting it on the wire. And certainly it would be much easier to recognize: mötleycrüe.com in the config file rather than xn-mtleycre-n4a5pc.com :-P. (and possibly there are even already people doing this in their libvirt config files, so adding the validation as you've written it could break someone's setup.) A qualifier - it's possible to compile this support out of dnsmasq to save space, and in that case dnsmasq *should* flag an error if it encounters any international characters. A further qualifier - you can check if the dnsmasq on the current host has this support by looking for "IDN" or "IDN2" in the output of "dnsmasq -v", so we could conditionally allow international characters based on that flag. Or we could just always translate to Punycode ourselves. g_hostname_to_ascii() does that (glib has several functions to handle hostnames with international characters, including validation; along with g_hostname_to_ascii(), there is also g_hostname_is_non_ascii(), g_hostname_is_ascii_encoded() (I would include a link to a webpage with all the related functions listed together, but it ends in .php and I don't want to be the reason your laptop gets PHPwned :-P). I think using these functions would also allow you to get rid of the opencoded VIR_HOSTNAME_VALID_CHARS, and possibly even the length checks and leading/trailing "-" checks) In the end, ACK if check for leading/trailing "-" is added. But we might want to be more inclusive about international characters, and it could be possible to replace the opencoded stuff with appropriate g_hostname_*() calls.
+ } + return 0; +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 5678c339ba..b2718fa1df 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -230,3 +230,5 @@ int virPipeQuiet(int fds[2]); * Returns: -1 on error, 0 on success */ int virPipeNonBlock(int fds[2]); + +int virHostnameValidate(const char *hostname, bool qualified); diff --git a/tests/utiltest.c b/tests/utiltest.c index 0bef502a84..7e14c0a161 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -423,6 +423,77 @@ testGetSubIDs(const void *data G_GNUC_UNUSED) }
+struct testHostnamesData +{ + const char *name; + bool qualified; + bool valid; +}; + +static const struct testHostnamesData testHostnamesData[] = { + { .name = "foo", .qualified = false, .valid = true }, + { .name = "foo", .qualified = true, .valid = true }, + { .name = "foo.example.com", .qualified = false, .valid = false }, + { .name = "foo.example.com", .qualified = true, .valid = true }, + { .name = "foo ", .qualified = false, .valid = false }, + { .name = "foo\n", .qualified = false, .valid = false }, + { .name = "foo\t", .qualified = false, .valid = false }, + { .name = "0foo", .qualified = false, .valid = true }, + { .name = "foo\n", .qualified = false, .valid = false }, + { .name = "foo.example.com\t", .qualified = true, .valid = false }, + { .name = ".foo", .qualified = false, .valid = false }, + { .name = "foo.", .qualified = false, .valid = false }, + { .name = "foo.example.com.", .qualified = false, .valid = false }, + { .name = "foo..example.com", .qualified = false, .valid = false }, + { .name = "foo1234567foo1234567foo1234567foo1234567foo1234567foo1234567abc", + .qualified = false, .valid = true }, + { .name = "foo1234567foo1234567foo1234567foo1234567foo1234567foo1234567abcd", + .qualified = false, .valid = false }, + { .name = + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "abc", + .qualified = true, .valid = true }, + { .name = + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "foo1234567foo1234567foo1234567foo1234567foo123456." + "abcd", + .qualified = true, .valid = false }, +}; + + +static int +testHostnames(const void *data G_GNUC_UNUSED) +{ + size_t i; + for (i = 0; i < G_N_ELEMENTS(testHostnamesData); i++) { + int ret = virHostnameValidate(testHostnamesData[i].name, + testHostnamesData[i].qualified); + if (ret < 0) { + if (testHostnamesData[i].valid) { + g_printerr("Hostname '%s' should be valid: %s", + testHostnamesData[i].name, + virGetLastErrorMessage()); + return -1; + } + } else { + if (!testHostnamesData[i].valid) { + g_printerr("Hostname '%s' should be invalid", + testHostnamesData[i].name); + return -1; + } + } + } + return 0; +} + + static int mymain(void) { @@ -447,6 +518,7 @@ mymain(void) DO_TEST(KernelCmdlineNextParam); DO_TEST(KernelCmdlineMatchParam); DO_TEST(GetSubIDs); + DO_TEST(Hostnames);
return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }