[asterisk-dev] [Code Review] Make ACLs IPv6-capable, take 2.

[Mark Michelson]

> On 2010-07-16 07:32:44, Simon Perreault wrote:
> > /trunk/include/asterisk/acl.h, line 119
> > <https://reviewboard.asterisk.org/r/791/diff/1/?file=11624#file11624line119>
> >
> >     While we are here, couldn't both of these parameters be made const?

Yes, good suggestion.


> On 2010-07-16 07:32:44, Simon Perreault wrote:
> > /trunk/main/acl.c, lines 299-300
> > <https://reviewboard.asterisk.org/r/791/diff/1/?file=11626#file11626line299>
> >
> >     Couldn't addr and netmask be made const?

Will do.


> On 2010-07-16 07:32:44, Simon Perreault wrote:
> > /trunk/main/acl.c, line 309
> > <https://reviewboard.asterisk.org/r/791/diff/1/?file=11626#file11626line309>
> >
> >     Do not assume that it's IPv6 if it's not IPv4. Check, and also add an else clause.

Will do.


> On 2010-07-16 07:32:44, Simon Perreault wrote:
> > /trunk/include/asterisk/acl.h, lines 48-54
> > <https://reviewboard.asterisk.org/r/791/diff/1/?file=11624#file11624line48>
> >
> >     Why do you need the "is_ipv4" member? Can't you just call ast_sockaddr_is_ipv4(&ha->addr)?

Yep, that's a good idea. Adding the flag to this struct was the first change I made in this diff, and I never gave things a re-evaluation to determine if I could remove it.

While I'm here, I'll also address OEJ's comment on the developers' list. He suggested to use an enum to determine the address type instead of restriction that an is_ipv4 member creates. I think I like Simon's idea more because any type of sockaddr structure supported by ast_sockaddr should have an ast_sockaddr_is_foo() function that can be used to determine the type of socket in use.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/791/#review2407
-----------------------------------------------------------


On 2010-07-15 16:31:07, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/791/
> -----------------------------------------------------------
> 
> (Updated 2010-07-15 16:31:07)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This change allows for IPv6 ACLs to be used in Asterisk.
> 
> ACLs are classified either as being IPv4 or IPv6 based on the network address in the ACL. The distinction means that if an ACL is created that specifies a rule regarding an IPv4 network, it will not be applied to an IPv6 address and vice-versa.
> 
> If someone specifies an IPv4-mapped IPv6 address in an ACL, we will convert this to be an IPv4 address and issue a NOTICE alerting them of the change. In order to do such a conversion, I had to make the ast_sockaddr_ipv4_mapped() function in main/netsock2.c public.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_iax2.c 276530 
>   /trunk/channels/chan_sip.c 276570 
>   /trunk/channels/chan_skinny.c 276530 
>   /trunk/include/asterisk/acl.h 276530 
>   /trunk/include/asterisk/netsock2.h 276530 
>   /trunk/main/acl.c 276570 
>   /trunk/main/manager.c 276530 
>   /trunk/main/netsock2.c 276530 
>   /trunk/tests/test_acl.c 276530 
> 
> Diff: https://reviewboard.asterisk.org/r/791/diff
> 
> 
> Testing
> -------
> 
> I have added tests to the invalid_acl and acl tests in tests/test_acl.c. They all pass as expected. I also ran the tests under valgrind and ensured there were no inappropriate memory accesses occurring.
> 
> 
> Thanks,
> 
> Mark
> 
>