On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <[email protected]>
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é <[email protected]>
---
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;
}
