Kapetanakis Giannis <[email protected]> writes:

> On 04/07/17 23:56, Sebastian Benoit wrote:
>> Florian Obser([email protected]) on 2017.07.04 19:27:15 +0000:
>>> On Fri, Jun 23, 2017 at 01:52:52PM +0300, Kapetanakis Giannis wrote:
>>>> Hi,
>>>>
>>>> Using relayd's redirect/forward on ipv6 addresses I discovered problems 
>>>> relating to setting TTL.
>>>>
>>>> There is no check for address family and setsockopt tries to apply IP_TTL 
>>>> always.
>>>>
>>>> Without ip ttl on ipv6 table, check_icmp gives
>>>> send_icmp: getsockopt: Invalid argument
>>>>
>>>> I've removed the IP_IPDEFTTL check. Was this ok?
>>>
>>> Nope, relayd reuses the raw socket between config reloads (I think),
>>> if the ttl gets removed from the config we need to reset to the
>>> default. Don't think there is a getsockopt for v6, you can take a look
>> 
>> i think jca@ once had a diff for somethin called IPV6_MINHOPLIMIT? Unsure if
>> thats what we need here though.

IPV6_MINHOPCOUNT support has been added to the kernel and relayd.
As tsg@ points out, this is about IPV6_UNICAST_HOPS here.

>>> at the sysctl(3) song and dance in traceroute(8) how to do this
>>> somewhat AF independet.
>>>
>>> Also please make sure to not exceed 80 cols
>
> Thanks for the commit on check_tcp.
>
> My tabstop was set to 3 and not 8. fixed that, but it looks ugly.
>
> According to ip6(4):
> IPV6_UNICAST_HOPS int *
>              Get or set the default hop limit header field for outgoing
>              unicast datagrams sent on this socket.  A value of -1 resets to
>              the default value.
>
> So I changed the diff and use this. Couldn't make it work with sysctl.

Using -1 for IPV6_UNICAST_HOPS is correct.

Note that you can also use -1 for IP_TTL on OpenBSD, sadly some systems
out there don't support it.

> comments?

ok jca@ with the nits below.

It would be nice to factor this out in a helper function and use it
elsewhere in relayd.

> Giannis
> ps. There is still a patch on @tech for alternative socket name.
> Could you also have a look there when you have some time?
> thanks
>
> Index: check_icmp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/check_icmp.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 check_icmp.c
> --- check_icmp.c      28 May 2017 10:39:15 -0000      1.45
> +++ check_icmp.c      5 Jul 2017 14:35:03 -0000
> @@ -168,6 +168,7 @@ send_icmp(int s, short event, void *arg)
>       socklen_t                slen, len;
>       int                      i = 0, ttl;
>       u_int32_t                id;
> +     int                      ip6_def_hlim = -1;

No need for this extra variable, you can just force ttl to -1 in the
IPv6 case.

>       if (event == EV_TIMEOUT) {
>               icmp_checks_timeout(cie, HCE_ICMP_WRITE_TIMEOUT);
> @@ -220,18 +221,46 @@ send_icmp(int s, short event, void *arg)
>                                   sizeof(packet));
>                       }
>  
> -                     if ((ttl = host->conf.ttl) > 0)
> -                             (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> -                                 &host->conf.ttl, sizeof(int));
> -                     else {
> -                             /* Revert to default TTL */
> -                             len = sizeof(ttl);
> -                             if (getsockopt(s, IPPROTO_IP, IP_IPDEFTTL,
> -                                 &ttl, &len) == 0)
> -                                     (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> -                                         &ttl, len);
> -                             else
> -                                 log_warn("%s: getsockopt",__func__);
> +                     switch(cie->af) {
> +                     case AF_INET:
> +                             if ((ttl = host->conf.ttl) > 0) {
> +                                     if (setsockopt(s, IPPROTO_IP, IP_TTL,
> +                                         &host->conf.ttl, sizeof(int)) == -1)
> +                                             log_warn("%s: setsockopt",
> +                                                 __func__);
> +                             }

extra newline

> +                             else {
> +                                     /* Revert to default TTL */
> +                                     len = sizeof(ttl);
> +                                     if (getsockopt(s, IPPROTO_IP,
> +                                         IP_IPDEFTTL, &ttl, &len) == 0) {
> +                                             if (setsockopt(s, IPPROTO_IP,
> +                                                 IP_TTL, &ttl, len) == -1)
> +                                                     log_warn(
> +                                                         "%s: setsockopt",
> +                                                         __func__);
> +                                     }
> +                                     else
> +                                         log_warn("%s: getsockopt",__func__);

4 spaces -> TAB

> +                             }
> +                             break;
> +                     case AF_INET6:
> +                             if ((ttl = host->conf.ttl) > 0) {
> +                                     if (setsockopt(s, IPPROTO_IPV6,
> +                                         IPV6_UNICAST_HOPS, &host->conf.ttl,
> +                                         sizeof(int)) == -1)
> +                                             log_warn("%s: setsockopt",
> +                                                 __func__);
> +                             }

extra newline

> +                             else {
> +                                     /* Revert to default hop limit */
> +                                     if (setsockopt(s, IPPROTO_IPV6,
> +                                         IPV6_UNICAST_HOPS, &ip6_def_hlim,
> +                                         sizeof(int)) == -1)
> +                                             log_warn("%s: setsockopt",
> +                                                 __func__);
> +                             }
> +                             break;
>                       }
>  
>                       r = sendto(s, packet, sizeof(packet), 0, to, slen);
>
>
>


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to