We used to validate that the first character of the hostname is a
letter. Later, RFC1123 relaxed the requirements to allow a number
as well.

Drop the validation completely, since we do not care about the
following characters, and neither does dnsmasq (even if it's a comma,
which is a delimiter in the hosts file).

Reverts: 673b74be5fda928da5e9f3c2cfbf6c1cb1eda0c6
Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/network_conf.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c769bbaeb5..8f50e22be5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -548,12 +548,6 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
     }
 
     name = virXMLPropString(node, "name");
-    if (name && !(g_ascii_isalpha(name[0]) || g_ascii_isdigit(name[0]))) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       _("Cannot use host name '%s' in network '%s'"),
-                       name, networkName);
-        return -1;
-    }
 
     ip = virXMLPropString(node, "ip");
     if (ip && (virSocketAddrParse(&inaddr, ip, AF_UNSPEC) < 0)) {
-- 
2.31.1

On 1/27/22 4:33 PM, Ján Tomko wrote:
> We used to validate that the first character of the hostname is a
> letter. Later, RFC1123 relaxed the requirements to allow a number
> as well.
> 
> Drop the validation completely, since we do not care about the
> following characters, and neither does dnsmasq (even if it's a comma,
> which is a delimiter in the hosts file).

Was there some discussion somewhere that prompted this patch (and thus 
invalidates the opinion I'm about to spout)? The only email I could find 
about it was the email of the "reverted" patch itself (sent by Peter on 
behalf of the author, with r-b given in the same email).

My opinion is that if a current RFC restricts the first letter of a 
hostname, then we should validate that too, *especially* if dnsmasq 
doesn't; who knows what entity beyond dnsmasq will barf on it in some 
way, and the closer to the source the non-compliance is reported, the 
easier it will be to fix. (Additionally, it's easy to remove extra 
validation, but much more difficult to add it back later if we decide it 
shouldn't have been removed)


> 
> Reverts: 673b74be5fda928da5e9f3c2cfbf6c1cb1eda0c6
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>   src/conf/network_conf.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index c769bbaeb5..8f50e22be5 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -548,12 +548,6 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
>       }
>   
>       name = virXMLPropString(node, "name");
> -    if (name && !(g_ascii_isalpha(name[0]) || g_ascii_isdigit(name[0]))) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Cannot use host name '%s' in network '%s'"),
> -                       name, networkName);
> -        return -1;
> -    }
>   
>       ip = virXMLPropString(node, "ip");
>       if (ip && (virSocketAddrParse(&inaddr, ip, AF_UNSPEC) < 0)) {

On a Sunday in 2022, Laine Stump wrote:
>On 1/27/22 4:33 PM, Ján Tomko wrote:
>>We used to validate that the first character of the hostname is a
>>letter. Later, RFC1123 relaxed the requirements to allow a number
>>as well.
>>
>>Drop the validation completely, since we do not care about the
>>following characters, and neither does dnsmasq (even if it's a comma,
>>which is a delimiter in the hosts file).
>
>Was there some discussion somewhere that prompted this patch (and thus 
>invalidates the opinion I'm about to spout)? The only email I could 
>find about it was the email of the "reverted" patch itself (sent by 
>Peter on behalf of the author, with r-b given in the same email).
>
>My opinion is that if a current RFC restricts the first letter of a 
>hostname,

It restricts all of the characters, not just the first one.
But in the past we only checked for one specific restriction of the
first character, that it is a letter. So I believe the better way
to remove the restriction is to remove the first check, not add a new
one.

I am unsure where the restriction came from (looking at the dates of
the RFCs the "reverted" commit references, I was too young to care at
the time.) The libvirt commit:

   commit b73d4957540938a61b95bd30696efa6553d14b5f
   CommitDate: 2008-08-20 12:50:29 +0000

     * src/network_conf.c src/network_conf.h src/qemu_driver.c: allow to
       add static host definition for dnsmasq

added the check with no explanation.

 From RFC 952 (Oct 1985):
The old check enforced one specific restriction
       <hname> ::= <name>*["."<name>]
       <name>  ::= <let>[*[<let-or-digit-or-hyphen>]<let-or-digit>]
RFC 1123 (Oct 1989):
       The syntax of a legal Internet host name was specified in RFC-952
       [DNS:4].  One aspect of host name syntax is hereby changed: the
       restriction on the first character is relaxed to allow either a
       letter or a digit.  Host software MUST support this more liberal
       syntax.

So, someone could add a validation for all of these constraints,
but I have not verified whether non-compliant hostnames actually
are broken with dnsmasq.

>then we should validate that too, *especially* if dnsmasq 
>doesn't; who knows what entity beyond dnsmasq will barf on it in some 
>way, and the closer to the source the non-compliance is reported, the 
>easier it will be to fix. (Additionally, it's easy to remove extra 
>validation, but much more difficult to add it back later if we decide 
>it shouldn't have been removed)
>

Right, we don't seem to have a separate validation step for network
parsing at the moment so adding a new check would be tedious.
But as-is, this check is pointless.

Jano