[Samba] CIDR prefix with a non-multiple of 8

[Crossman, Eric]

First off, I apologize for reporting a problem with a "historical"
version of Samba, but this is what we use in our production environment
due to some integration issues with the 3.0 series.

 

I've been experiencing a problem with Samba 2.2.x when trying to use a
CIDR notation for hosts allow/deny when that CIDR prefix is not one of (
/8, /16, /24, or /32). When I included a network such as
192.168.64.0/18, addresses that are clearly in that range (such as
192.168.72.3) are blocked as if they were not present in the allow list
at all. There is also no deny list defined.

 

I have originally experienced this with Samba 2.2.7a on RedHat
Enterprise Linux 4 update 3 running a 2.6.14.7 kernel. I have tried
upgrading to the latest release in the samba 2.2 series, 2.2.12 which
also exhibits this problem. 

 

Running controlled experiments with different forms of the CIDR networks
shows that as long as you use a CIDR network which has a prefix that is
a multiple of 8, the connections from that network will be allowed
correctly. However, if that prefix is a number in between those
multiples of 8, it will be denied unless it happens to be within the
first classful network of the summarized network. For example, the
summarized network 72.224.0.0/13, will allow connections from 72.224.x.x
but not 72.225.x.x, etc..

 

Digging into the source code, I found the problem to be in the way the
netmask value is calculated. When the network in question has a multiple
of 8 prefix, each octet of its netmask value, is either 0 or 255
decimal. On the other hand, the other prefixes yield netmask octet
values between 0 and 255. In this latter case, the bit ordering within
each octet is significant.

 

The method that is used to calculate the netmask is as follows:

 

mask = (uint32)((ALLONES << atoi(slash + 1)) ^ ALLONES);

 

On a little endian machine the bytes are stored in least significant
byte first, but the most significant bits are stored from the left. With
this in mind, the above bit shifting operation does not yield the
correct netmask value on Intel machines (for prefixes that are not a
multiple of 8). 

 

In order to correct the netmask value, the bits within a byte need to be
reversed.

 

To correct this problem, I have written a function to reverse the bits
and added a call to it immediately after the initial mask calculation.

 

Is anyone aware of this problem and if so has it been corrected in the
3.0 series?

 

Below is a patch to lib/access.c to correct the behavior: (I realize
this doesn't address skipping the conversion on big endian arches but I
am unaware of how to check that within the samba code base.)

 

--- access.c.orig       2006-10-19 18:44:42.000000000 -0400

+++ access.c    2006-10-20 17:30:16.000000000 -0400

@@ -17,6 +17,32 @@

 

 #define ALLONES  ((uint32)0xFFFFFFFF)

 

+/* revbits8 - reverse the bits within a byte */

+static short revbits8(short a)

+{

+       short b = 0;

+

+        printf("revbits8: reversing %d\n", a);

+        b = ( ((a & 0x80) >> 7) + (((a & 0x40) >> 6) << 1) +

+                 (((a & 0x20) >> 5) << 2) + (((a & 0x10) >> 4) << 3) +

+                 (((a & 0x08) >> 3) << 4) + (((a & 0x04) >> 2) << 5) +

+                 (((a & 0x02) >> 1) << 6) + ((a & 0x01) << 7) );

+        printf("revbit8: returning %d\n", b);

+        return ( ((a & 0x80) >> 7) + (((a & 0x40) >> 6) << 1) +

+                 (((a & 0x20) >> 5) << 2) + (((a & 0x10) >> 4) << 3) +

+                 (((a & 0x08) >> 3) << 4) + (((a & 0x04) >> 2) << 5) +

+                 (((a & 0x02) >> 1) << 6) + ((a & 0x01) << 7) );

+}

+

+/* revbits - reverse the bits in each byte of a uint32 */

+unsigned long revbits(unsigned long a)

+{

+        return ( ((revbits8((a & 0xFF) >> 0)) << 0) +

+                 ((revbits8((a & 0xFF00) >> 8)) << 8) +

+                 ((revbits8((a & 0xFF0000) >> 16)) << 16) +

+                 ((revbits8((a & 0xFF000000) >> 24)) << 24) );

+}

+

 /* masked_match - match address against netnumber/netmask */

 static int masked_match(char *tok, char *slash, char *s)

 {

@@ -34,6 +60,8 @@

                 mask = interpret_addr(slash + 1);

         } else {

                mask = (uint32)((ALLONES << atoi(slash + 1)) ^ ALLONES);

+               /* Fixup the netmask by reversing the bits in each byte
*/

+               mask = revbits(mask);

         }

 

        if (net == INADDR_NONE || mask == INADDR_NONE) {

 

======================================

Eric Crossman

Assistant Systems Administrator, School of Science

Siena College

515 Loudon Road

Loudonville, NY 12211