From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B82335A026F for ; Sat, 23 Sep 2023 09:45:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1695455108; bh=hfMnHs27yu/tSzmPdgxqQq9tjSYIAUFfSv2Or64qrPI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Yx/bthlDRKWrenLM812+teCMFhskj/SyAwD2HNX8QclgorvATHzHOALngkBALZctk /ZqmFGmsrOJPAfNC5/VbK1NksGsVEyDU3ac08bp00PdStDM3lOGfr8slvpZvd1V9XE hHuJZ9EL1aanFOA6xP9SJnv+5pjoBEpr1gMAKLh8= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Rt1Pw0TMnz4xQP; Sat, 23 Sep 2023 17:45:08 +1000 (AEST) Date: Sat, 23 Sep 2023 17:44:58 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] dhcpv6: Properly separate domain names in search list Message-ID: References: <20230920150506.3341961-1-sbrivio@redhat.com> <20230921170822.46c441f2@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AlUvUDPpTk6p22ch" Content-Disposition: inline In-Reply-To: <20230921170822.46c441f2@elisabeth> Message-ID-Hash: YHZGUNNQUSP5FCYLMFPNEPIY2DV3ULNJ X-Message-ID-Hash: YHZGUNNQUSP5FCYLMFPNEPIY2DV3ULNJ X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Sebastian Mitterle X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --AlUvUDPpTk6p22ch Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 21, 2023 at 05:08:22PM +0200, Stefano Brivio wrote: > On Thu, 21 Sep 2023 09:55:11 +1000 > David Gibson wrote: >=20 > > On Wed, Sep 20, 2023 at 05:05:06PM +0200, Stefano Brivio wrote: > > > If we go over the flattened list of search domains and just replace > > > dots and zero bytes with the length of the next label to implement > > > the encoding specified by section 3.1 of RFC 1035, if there are > > > multiple domains in the search list, we'll also replace separators > > > between two domain names with the length of the first label of the > > > second domain, plus one. =20 > >=20 > > That is... an impressively long sentence. Any chance you could reword > > that in shorter ones that are easier to follow ;). >=20 > Oops. :) What about: >=20 > To prepare the DHCPv6 domain search list option, we go over the > flattened list of domains, and replace both dots and zero bytes with > a counter of bytes in the next label, implementing the encoding specifi= ed > by section 3.1 of RFC 1035. >=20 > If there are multiple domains in the list, however, zero bytes serve > as markers for the end of a domain name, and we'll replace them with > the length of the first label of the next domain, plus one. This is > wrong. We should only convert the dots before the labels. >=20 > ? Better.. I think my main confusion is that I don't remember enough about DNS to recall what the distinction is between domains and labels. > > > Those should remain as zero bytes to > > > separate domains, though. > > >=20 > > > To distinguish between label separators and domain names separators, > > > for simplicity, introduce a dot before the first label of every > > > domain we copy to form the list. All dots are then replaced by label > > > lengths, and separators (zero bytes) remain as they are. > > >=20 > > > As we do this, we need to make sure we don't replace the trailing > > > dot, if present: that's already a separator. Skip copying it, and > > > just add separators as needed. > > >=20 > > > Now that we don't copy those, though, we might end up with > > > zero-length domains: skip them, as they're meaningless anyway. > > >=20 > > > And as we might skip domains, we can't use the index 'i' to check if > > > we're at the beginning of the option -- use 'srch' instead. > > >=20 > > > This is very similar to how we prepare the list for NDP option 31, > > > except that we don't need padding (RFC 8106, 5.2) here, and we should > > > refactor this into common functions, but it probably makes sense to > > > rework the NDP responder (https://bugs.passt.top/show_bug.cgi?id=3D21) > > > first. > > >=20 > > > Reported-by: Sebastian Mitterle > > > Link: https://bugs.passt.top/show_bug.cgi?id=3D75 > > > Signed-off-by: Stefano Brivio > > > --- > > > dhcpv6.c | 24 +++++++++++++++++------- > > > 1 file changed, 17 insertions(+), 7 deletions(-) > > >=20 > > > diff --git a/dhcpv6.c b/dhcpv6.c > > > index fc42a84..58171bb 100644 > > > --- a/dhcpv6.c > > > +++ b/dhcpv6.c > > > @@ -376,24 +376,34 @@ search: > > > return offset; > > > =20 > > > for (i =3D 0; *c->dns_search[i].n; i++) { > > > - if (!i) { > > > + size_t name_len =3D strlen(c->dns_search[i].n); > > > + > > > + /* We already append separators, don't duplicate if present */ > > > + if (c->dns_search[i].n[name_len - 1] =3D=3D '.') > > > + name_len--; > > > + > > > + /* Skip root-only search domains */ > > > + if (!name_len) > > > + continue; =20 > >=20 > > Should we consider doing this normalisation when we build > > c->dns_search, rather than here? >=20 > I quickly looked at that, but it complicates the (insane) compression > scheme from RFC 1035 4.1.4 implemented by the DHCP server -- and also > we would need to convert them back before printing them. Ok, fair enough. > I don't think it's necessarily a bad idea though. I guess we should > have a few explicit functions to convert between different encodings and > then stick to the storage format that turns out to be the most > convenient. But that's beyond the scope of this fix. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --AlUvUDPpTk6p22ch Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmUOl18ACgkQzQJF27ox 2Gfp8A//ZSBMpfCSCvqUlz3S6O+1lYl2mYyFJeOsUpEcbbNTmXjsChQEJHBNmuzd qrLR2lUjvkmVgAsXN852YJWT/FNADpJo3Ye8ewJrg+/c9K5RCiS23FtN1nA3MbG8 8///x/zDKqcarxTTIsvx6OYL0LI2gwI9JidMI4g4K9iX7IVN6XWtTjBon3BBf1fu nXZZQZ7xXIaDx+0Xn1gyPoUN7kwGSxTIy1RZcnBtkfIjr0rVtWUgnC36G0kzf5Z6 kmXqQsg1szjNoxh3jCgTspeLhjVoVtL3dUOYxyEpirWzN1Yvt7bMBh73kzj+hmir nxA6JJ6uOcgzPsTd4jzXn2XxrF6S+TEGHM9uwHeRNLXJM13sFVR2rakjLLmhHjKc 0UQLg3jj8YupUbpVPq3YJp7ESwZxxU6/Bx/9gyPK11jPa7N7gWk41NNPpjtkCFuv qMEpo6IRQWXw19DnlhtHNpdqxanmXBMlpoPSA6jHTDtYmSBvcSmN+1slRPw8Lhh8 gE2jdkHlah3OR8l1vsH96H3QWm/bLDWvSMm+ebkkpX6Qzqk5y4qZ4l1HCsRD6u6m sMIDgxi1Ra5E04OCoSnlQwFC6SAKXq1fQdHnEtZ5uuHPz29iaYEb0KHZr5LKF1PL YTsXaP/cVYMTHH7vOHJGekOel2235HXj04w4SD59+b4Qh2MTyR0= =o4F4 -----END PGP SIGNATURE----- --AlUvUDPpTk6p22ch--