p2p tun configs break with new topology default in non-obvious ways about openvpn HOT 8 OPEN

So if init_tun() is called with strict_warn == true then we warn when the second argument doesn't look like a netmask.

from openvpn.

But the problem with that check is that it actually uses topology to detect whether we're in P2P mode:

bool
is_tun_p2p(const struct tuntap *tt)
{
    bool tun = false;

    if (tt->type == DEV_TYPE_TAP
        || (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
        || tt->type == DEV_TYPE_NULL)
    {
     	tun = false;
    }
    else if (tt->type == DEV_TYPE_TUN)
    {
        tun = true;
    }
    else
    {
        msg(M_FATAL, "Error: problem with tun vs. tap setting"); /* JYFIXME -- needs to be caught earlier, in ini\
t_tun? */

    }
    return tun;
}

So by changing the default we probably broke this check.

from openvpn.

Nevermind, in current master we do not actually use is_tun_p2p. This is only added by my patch https://gerrit.openvpn.net/c/openvpn/+/380. Master uses tt->type == DEV_TYPE_TUN which is just not correct, so the wrong checks are done probably.

from openvpn.

Changing the default only for --server would probably look something like this:

• Change default to TOP_UNDEF instead of TOP_SUBNET
• In helper_client_server check whether topology is still TOP_UNDEF, if yes change to TOP_SUBNET
• After helper_client_server set topology to TOP_NET30 if it is still TOP_UNDEF

from openvpn.

After helper_client_server set topology to TOP_NET30 if it is still TOP_UNDEF

honestly this feels a bit hacky: i.e. using what we have in a dirty way to properly achieve what we need.

How about explicitly adding a TOP_P2P ? after all this is not NET30, but it's a truly different way of assigning the IPs (local+remote vs /30).

This way we can then explicitly check if topology == TOP_P2P and act accordingly.

from openvpn.

As discussed on IRC: There is already a TOP_P2P which does something slightly different again, although most of the code treats it as alias for TOP_NET30.

from openvpn.

So the topology default has been taken care of, but the other aspect of passing on -1 as netbits after parsing is still not really helpful error handling.

from openvpn.

So the topology default has been taken care of, but the other aspect of passing on -1 as netbits after parsing is still not really helpful error handling.

@cron2 Please take a look at https://gerrit.openvpn.net/c/openvpn/+/380, my earlier comments seem to indicate that it should improve that part.

from openvpn.