On Fri, 4 Jul 2014 09:26:04 +0200
Kenneth Westerback <[email protected]> wrote:
>> @@ -847,7 +847,7 @@ parse_cidr(FILE *cfile, unsigned char *a
>>         if (token == TOK_NUMBER)
>>                 convert_num(prefix, val, 10, 8);
>>
>> -       if (token != TOK_NUMBER || *prefix < 1 || *prefix > 32) {
>> +       if (token != TOK_NUMBER || *prefix > 32) {
>>                 parse_warn("Expecting CIDR prefix length, got '%s'", val);
>>                 goto nocidr;
>>         }
> 
> This may be correct, as my re-reading of RFC 3442 does not forbid a
> prefix of 0.

Specifying the default router in "Classless Static Routes option"
seems to be intended.  See "DHCP Server Administrator
Responsibilities" section of RFC 3442.

|    Many clients may not implement the Classless Static Routes option.
|    DHCP server administrators should therefore configure their DHCP
|    servers to send both a Router option and a Classless Static Routes
|    option, and should specify the default router(s) both in the Router
|    option and in the Classless Static Routes option.

> This check may be present to ensure our dhclient does not get
> confused, which I will check.

As my reading the code, it seems to be ok.

>> @@ -1156,8 +1156,9 @@ void parse_option_param(cfile, group)
>>                                         return;
>>                                 tree = tree_concat(tree, tree_const(&cprefix,
>>                                     sizeof(cprefix)));
>> -                               tree = tree_concat(tree, tree_const(buf,
>> -                                   sizeof(buf)));
>> +                               if (cprefix > 0)
>> +                                       tree = tree_concat(tree, 
>> tree_const(buf,
>> +                                           cprefix / NBBY));
> 
> Even if the idea is correct, and it seems to be, I'm pretty sure
> 'cprefix/NBBY' is not correct. e.g. if cprefix is 7, will cprefix /
> NBBY not be 0 instead of 1?

Oops, you're right. 

> Perhaps '(cprefix + 7) / 8'? This would also get rid of NBBY which I'm
> leery of in this context.

I noticed existing code of dhclient and dhcpd use just 8.  So I agree
getting rid of NBBY.

Updated the diff

ok?

Index: usr.sbin/dhcpd/confpars.c
===================================================================
RCS file: /cvs/src/usr.sbin/dhcpd/confpars.c,v
retrieving revision 1.22
diff -u -p -r1.22 confpars.c
--- usr.sbin/dhcpd/confpars.c   21 Jan 2014 03:07:51 -0000      1.22
+++ usr.sbin/dhcpd/confpars.c   4 Jul 2014 12:51:57 -0000
@@ -823,7 +823,7 @@ void parse_group_declaration(cfile, grou
  * bit-count :== 0..32
  */
 int
-parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)  
+parse_cidr(FILE *cfile, unsigned char *addr, unsigned char *prefix)
 {
        char *val;
        int token;
@@ -835,7 +835,7 @@ parse_cidr(FILE *cfile, unsigned char *a
                parse_warn("Expecting CIDR subnet");
                goto nocidr;
        }
-       
+
        token = next_token(&val, cfile);
        if (token != '/') {
                parse_warn("Expecting '/'");
@@ -847,7 +847,7 @@ parse_cidr(FILE *cfile, unsigned char *a
        if (token == TOK_NUMBER)
                convert_num(prefix, val, 10, 8);
 
-       if (token != TOK_NUMBER || *prefix < 1 || *prefix > 32) {
+       if (token != TOK_NUMBER || *prefix > 32) {
                parse_warn("Expecting CIDR prefix length, got '%s'", val);
                goto nocidr;
        }
@@ -1156,8 +1156,9 @@ void parse_option_param(cfile, group)
                                        return;
                                tree = tree_concat(tree, tree_const(&cprefix,
                                    sizeof(cprefix)));
-                               tree = tree_concat(tree, tree_const(buf,
-                                   sizeof(buf)));
+                               if (cprefix > 0)
+                                       tree = tree_concat(tree, tree_const(buf,
+                                           (cprefix + 7) / 8));
                                break;
                        default:
                                warning("Bad format %c in parse_option_param.",



Reply via email to