bug in masked_match function

[Andrew Bird]

Tomoki
I think the right solution is to revert the patch - i've knocked up
the following test program.

testbox$ ./a.out
255.255.254.0 == addr/23
     1110  

Reverting old change - correct
(ALLONES  atoi(slash + 1)) ^ ALLONES
     1110  

changing XOR to AND is incorrect
(ALLONES  atoi(slash + 1))  ALLONES
  1000     



Best Regards



Andrew Bird (Unix Consultant)



#define ALLONES  ((uint32)0x)

typedef unsigned int uint32;


print_uint32(uint32 val) {
   int i;
   for (i = 31 ; i = 0 ;i--) {
   printf(%c, val  (1  i) ? '1' : '0');
   if( (i % 4) == 0 )
  printf( );
   }
   printf(\n\n);
}


int main() {

char *string=10.0.0.0/23;

char *slash=/23;

uint32 mask;

mask =(25524) + (25516) + (2548) + 0;
printf(255.255.254.0 == addr/23\n);
print_uint32(mask);

mask = (uint32)((ALLONES  atoi(slash + 1)) ^ ALLONES);
printf(Reverting old change - correct\n);
printf((ALLONES  atoi(slash + 1)) ^ ALLONES\n);
print_uint32(mask);


mask = (uint32)((ALLONES  atoi(slash + 1))  ALLONES);
printf(changing XOR to AND is incorrect\n);
printf((ALLONES  atoi(slash + 1))  ALLONES\n);
print_uint32(mask);

}

Re: bug in masked_match function

[Tomoki AONO]

I found this suspisious case (and described shortly in
Samba-JP), so I'll explain more.

In [EMAIL PROTECTED],
[EMAIL PROTECTED] wrote:

 The masked_match function in lib/access.c is wrong.(CVS HEAD and 2_2)

This case matches if CIDR-like notation specified in hosts
allow/deny (ex. '10.0.0.0/23') only. This is not case if
specified with network/subnet mask. (ex. '10.0.0.0/255.255.254.0')

I cite more lines in lib/access.c:
33  if (strlen(slash + 1)  2) {
34  mask = interpret_addr(slash + 1);
35  } else {
36  mask = (uint32)((ALLONES  atoi(slash + 1)) ^ ALLONES);
37  }

 Example: hosts allow = 10.0.0.0/23
 
 This produces following result. This isn't mask.
 mask =   0111     

In case '10.0.0.0/255.255.254.0', program execute line 34
and returns:
mask =      1110  

 I don't know why this change was made.
 http://cvs.samba.org/cgi-bin/cvsweb/samba/source/lib/access.c.diff?r1=1.19.4.12r2=1.19.4.13

I think reverting change in line 36 (reverse shift
direction) or replacing '^'(XOR) to ''(AND) would solve
this case. Am I right?

Patch (I prefer replacing '^' to '') follows:

Index: lib/access.c
===
RCS file: /cvsroot/samba/source/lib/access.c,v
retrieving revision 1.35
diff -u -u -w -r1.35 access.c
--- lib/access.c12 Nov 2002 23:15:49 -  1.35
+++ lib/access.c14 Mar 2003 10:43:09 -
@@ -33,7 +33,7 @@
 if (strlen(slash + 1)  2) {
 mask = interpret_addr(slash + 1);
 } else {
-   mask = (uint32)((ALLONES  atoi(slash + 1)) ^ ALLONES);
+   mask = (uint32)((ALLONES  atoi(slash + 1))  ALLONES);
 }
 
if (net == INADDR_NONE || mask == INADDR_NONE) {


Tomoki AONO ([EMAIL PROTECTED])

bug in masked_match function

[Yasuma Takeda]

Hello,

I heard a following problem in Samba-JP.

The masked_match function in lib/access.c is wrong.(CVS HEAD and 2_2)

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

Example: hosts allow = 10.0.0.0/23

This produces following result. This isn't mask.
mask =   0111     

Therefore 'hosts allow' and 'hosts deny' doesn't match.

I don't know why this change was made.
http://cvs.samba.org/cgi-bin/cvsweb/samba/source/lib/access.c.diff?r1=1.19.4.12r2=1.19.4.13

Please check.

Thanks,
Yasuma Takeda [EMAIL PROTECTED]