Date: Sun, 19 Apr 2020 10:23:27 -0400 From: Shawn Webb <shawn.webb@hardenedbsd.org> To: Kristof Provost <kp@FreeBSD.org> Cc: Ronald Klop <ronald-lists@klop.ws>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r360068 - in head/sys: kern net sys Message-ID: <20200419142327.nhotyboqubk3vl2l@mutt-hbsd> In-Reply-To: <67B55A62-848B-4876-8367-DE0D32A8B7D4@FreeBSD.org> References: <202004180750.03I7oUK6032898@repo.freebsd.org> <op.0jbpatnnkndu52@sjakie> <67B55A62-848B-4876-8367-DE0D32A8B7D4@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--flj6q4ccadmcdqix Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Apr 19, 2020 at 04:19:06PM +0200, Kristof Provost wrote: > On 19 Apr 2020, at 15:33, Ronald Klop wrote: > > On Sat, 18 Apr 2020 09:50:30 +0200, Kristof Provost <kp@freebsd.org> > > wrote: > >=20 > > > Author: kp > > > Date: Sat Apr 18 07:50:30 2020 > > > New Revision: 360068 > > > URL: https://svnweb.freebsd.org/changeset/base/360068 > > >=20 > > > Log: > > > ethersubr: Make the mac address generation more robust > > > If we create two (vnet) jails and create a bridge interface in each > > > we end up > > > with the same mac address on both bridge interfaces. > > > These very often conflicts, resulting in same mac address in both > > > jails. > > > Mitigate this problem by including the jail name in the mac address. > > > Reviewed by: kevans, melifaro > > > MFC after: 1 week > > > Differential Revision: https://reviews.freebsd.org/D24383 > > >=20 > > > Modified: > > > head/sys/kern/kern_jail.c > > > head/sys/net/if_ethersubr.c > > > head/sys/sys/jail.h > > >=20 > > > Modified: head/sys/kern/kern_jail.c > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > > --- head/sys/kern/kern_jail.c Sat Apr 18 03:14:16 2020 (r360067) > > > +++ head/sys/kern/kern_jail.c Sat Apr 18 07:50:30 2020 (r360068) > > > @@ -2920,6 +2920,15 @@ getcredhostid(struct ucred *cred, unsigned > > > long *hosti > > > mtx_unlock(&cred->cr_prison->pr_mtx); > > > } > > > +void > > > +getjailname(struct ucred *cred, char *name, size_t len) > > > +{ > > > + > > > + mtx_lock(&cred->cr_prison->pr_mtx); > > > + strlcpy(name, cred->cr_prison->pr_name, len); > > > + mtx_unlock(&cred->cr_prison->pr_mtx); > > > +} > > > + > > > #ifdef VIMAGE > > > /* > > > * Determine whether the prison represented by cred owns > > >=20 > > > Modified: head/sys/net/if_ethersubr.c > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > > --- head/sys/net/if_ethersubr.c Sat Apr 18 03:14:16 2020 (r360067) > > > +++ head/sys/net/if_ethersubr.c Sat Apr 18 07:50:30 2020 (r360068) > > > @@ -1419,27 +1419,39 @@ ether_8021q_frame(struct mbuf **mp, struct > > > ifnet *ife, > > > /* > > > * Allocate an address from the FreeBSD Foundation OUI. This uses a > > > - * cryptographic hash function on the containing jail's UUID and > > > the interface > > > - * name to attempt to provide a unique but stable address. > > > Pseudo-interfaces > > > - * which require a MAC address should use this function to allocate > > > - * non-locally-administered addresses. > > > + * cryptographic hash function on the containing jail's name, UUID > > > and the > > > + * interface name to attempt to provide a unique but stable address. > > > + * Pseudo-interfaces which require a MAC address should use this > > > function to > > > + * allocate non-locally-administered addresses. > > > */ > > > void > > > ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr) > > > { > > > -#define ETHER_GEN_ADDR_BUFSIZ HOSTUUIDLEN + IFNAMSIZ + 2 > > > SHA1_CTX ctx; > > > - char buf[ETHER_GEN_ADDR_BUFSIZ]; > > > + char *buf; > > > char uuid[HOSTUUIDLEN + 1]; > > > uint64_t addr; > > > int i, sz; > > > char digest[SHA1_RESULTLEN]; > > > + char jailname[MAXHOSTNAMELEN]; > > > getcredhostuuid(curthread->td_ucred, uuid, sizeof(uuid)); > > > - sz =3D snprintf(buf, ETHER_GEN_ADDR_BUFSIZ, "%s-%s", uuid, > > > ifp->if_xname); > > > + /* If each (vnet) jail would also have a unique hostuuid this > > > would not > > > + * be necessary. */ > > > + getjailname(curthread->td_ucred, jailname, sizeof(jailname)); > > > + sz =3D asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, if_name(ifp), > > > + jailname); > > > + if (sz < 0) { > > > + /* Fall back to a random mac address. */ > >=20 > >=20 > > I was wondering if it would be valuable to give this fall back something > > like: > >=20 > > printf("%s: unable to create fixed mac address; using random > > mac address", if_name(ifp)); > >=20 > > This will only be printed in rare circumstances. But in that case will > > provide valuable information. > >=20 > That would potentially be valuable, yes. On the other hand, we traditiona= lly > don???t sprinkle a lot of printf()s around in the kernel. This is extreme= ly > unlikely to happen, and if it does odds are attaching the interface will > fail at an earlier or later point, you may struggle to pass packets and r= un > into any number of other issues. > It???s also possible to diagnose absent the printf(), because the MAC > address will be locally administered rather than within the FreeBSD OUI. >=20 > So, in short: not a bad idea. You can argue it both ways, and I find myse= lf > (weakly) on the opposite side. Would displaying the message only when verbose boot mode is enabled be a suitable compromise? Thanks, --=20 Shawn Webb Cofounder / Security Engineer HardenedBSD GPG Key ID: 0xFF2E67A277F8E1FA GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Sha= wn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc --flj6q4ccadmcdqix Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl6cXtgACgkQ/y5nonf4 4fpcDw//W2EWrllSHRy0LWiNhEeaRxQfkStXDJfsZN0YOkeTSn8fOdxeftFQSj7R geY35ZWdd6y56u5UFyP0lszRQMTPgwT5mvhKE4fDaY81rVdiP952KUBUBXmp8hVm ojIhdsFRbGjW6irDP3RfOcu/eZxiCfHGgmD0l/QeLsckP230y8jgl22agy5r/4U1 kG9L4o70E1bk+5qNrI3CfFL3FWarWBDGG+Hd4waV3Nv0WA6hPtLkltYgdOt6SUP3 d0Pi5KUOUrES70mrsIy5QO/oORLHr7SuuKKS9Hzd2wYygY99SaOgU9hR4fH6MpZu ruEY3HfspNkDv1tFPVg1hCO8U6yiDIpWgqpbh3czjUQGluFvWrvAu91WHE+/im4r ZUU7fh882zb3QWBEfOtwoH7wv2O8uWC5Eu45ynZfyctxyiAMa1bOn6Y53yHSvX4N dG4pQEfn5sA3gDn5i+MD4pO494WmULLxISgCdORAPL3+oPDMr0aCgVZ60EiIuUhs s89694ymqTiF3tPatY9rideN/j/6T+rBVIRScSpj4/Nm01oFfiZoFm91OW0VEDRM BupsHzIG0ICtnGvN/8InyVvbYLZGVRe7zyUp/Fx7919m9jjr16QmL7VP3nwDKWJx HyfOKUGohJwrtyPmv2i528Mv9kV9HSfDKguE0TBR+6p0WmRe5Gk= =6E5S -----END PGP SIGNATURE----- --flj6q4ccadmcdqix--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200419142327.nhotyboqubk3vl2l>