[Dnsmasq-discuss] Tag requests for a DHCP address from devices using a Locally Administered MAC address

[Geert Stappers]

On Sun, Jul 26, 2020 at 12:37:32PM -0700, dev at lutean.com wrote:
> From: Vladislav Grishenko Sent: July 26, 2020 8:04 AM
> > From: Pali Rohar Sent: Sunday, July 26, 2020 7:20 PM
> > On Sunday 26 July 2020 15:35:24 Geert Stappers wrote:
> > > On Sun, Jul 26, 2020 at 06:07:52AM -0700, dev at lutean.com wrote:
> > > > On Sunday 26 July 2020 Geert Stappers wrote:
> > > > > On Sun, Jul 26, 2020, dev at lutean.com wrote:
> > > > > > 
> > > > > > > In my testing these devices use a MAC address with the LAA bit 
> > > > > > > set (2nd least significant bit of the first byte of the MAC). It 
> > > > > > > restricts this to host addresses (least significant bit is set to 0).
> > > > > > 
> > > > > > Speaks about two bits
> > > > > > 
> > > > > > 
> > > > > > > This patch detects MAC addresses with this bit set and tags the 
> > > > > > > request with the tag "laa-address". This would allow other rules 
> > > > > > > to decide what to do with these requests (such as ignoring them).
> > > > > > 
> > > > > > Speaks about one bit
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Speaking about bits, see https://en.wikipedia.org/wiki/MAC_address#/media/File:MAC-48_Address.svg
> > > > > > for the "exploded view"
> > > > > > 
> > > > > 
> > > > > https://en.wikipedia.org/wiki/MAC_address#Unicast_vs._multicast
> > > > > 
> > > > > The reason two bits are tested is because:
> > > > > - one bit is the UAA / LAA bit
> > > > > - one bit is the unicast / multicast bit
> > > > > 
> > > > > so this patch wouldn't tag LAA multicast MAC addresses should those 
> > > > > happen to be in use somewhere.
> > > > > 
> > > > > So specifically a device with an LAA unicast MAC address would get a 
> > > > > tag. This requires testing two bits.
> > > > > 
> > > > 
> > > > OK, thanks for elaborating
> > > 
> > > I think that big misunderstanding comes from commit message which says
> > > that one bit (LAA) is tested, but in patch itself are tested two bits.
> > > 
> > > I guess that fixing commit message to properly describe that testing
> > > both bits (and which) are needed should be enough.
> > > 
> > > Anyway, I'm not sure if 'laa-address' is correct name if it is not
> > > set for every laa-address, but only for unicast laa-address.
> > > 
> > 
> > LAA stands for locally-administrated address itself, so from my opinion "laa-address" is a bit tautologic.
> > Let's use just "laa", also it ~fits already used one word tags:
> > 	"bootp"
> > 	"cpewan-id"
> > 	"dhcpv6"
> > 	"known"
> > 	"known-othernet"
> > 	<interface>
> > 
> How about this. A device showing up with an LAA gets tagged
> twice. Always with an "laa" tag, but also with one of "laa-unicast"
> or "laa-multicast".
> 
> If someone wanted to block devices, it would be easy with
> 
> # Block all LAA-presenting devices
> dhcp-ignore=tag:laa
> 
> # Block unicast LAA-presenting devices
> dhcp-ignore=tag:laa-unicast
> 
> diff --git a/src/rfc2131.c b/src/rfc2131.c
> index fc54aab..b9da511 100644
> --- a/src/rfc2131.c
> +++ b/src/rfc2131.c
> @@ -93,7 +93,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>    unsigned char *agent_id = NULL, *uuid = NULL;
>    unsigned char *emac = NULL;
>    int vendor_class_len = 0, emac_len = 0;
> -  struct dhcp_netid known_id, iface_id, cpewan_id;
> +  struct dhcp_netid known_id, iface_id, cpewan_id, laa_id, laa_cast_id;
>    struct dhcp_opt *o;
>    unsigned char pxe_uuid[17];
>    unsigned char *oui = NULL, *serial = NULL;
> @@ -114,6 +114,32 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
>    if (mess->htype == 0 && mess->hlen != 0)
>      return 0;
>  
> +  /* Check if sender has a Locally-Administered ethernet Address and set a tag if so. */
> +  if (mess->htype == ARPHRD_ETHER)
> +  {
> +    /* Locally Administered Addresses (LAA) have the 2nd LSb of the first address byte set */
> +    if ((mess->chaddr[0] & 2) == 2)
> +    {
> +      laa_id.net = "laa";
> +      laa_id.next = netid;
> +      netid = &laa_id;
> +
> +      /* Check if this is a unicast or multicast LAA. Also set a tag
> +         for the type of LAA. So a device with an LAA will have two tags
> +         "laa" and either "laa-multicast" or "laa-unicast" */
> +      if ((mess->chaddr[0] & 1) == 1)
> +      {
> +        laa_cast_id.net = "laa-multicast";
> +      }
> +      else
> +      {
> +        laa_cast_id.net = "laa-unicast";
> +      }
> +      laa_cast_id.next = netid;
> +      netid = &laa_cast_id;
> +    }
> +  }
> +
>    /* check for DHCP rather than BOOTP */
>    if ((opt = option_find(mess, sz, OPTION_MESSAGE_TYPE, 1)))
>      {


That revisited patch  solves the main problem I had with previous
version.

I think the next version should also update the manual page
and have the verbatim text for the commit message.



Groeten
Geert Stappers
-- 
Silence is hard to parse