if a plugin fails in ENABLE_PF, openvpn sigsegv's

[Gert Döring]

somewhat recent MASTER, trying to fix the code in defer/simple.c, I run it without "TEST_PACKET_FILTER" defined. So the plugin code fails ENABLE_PF:

            if (context->test_packet_filter)
            {
                return OPENVPN_PLUGIN_FUNC_SUCCESS;
            }
            else
            {
                return OPENVPN_PLUGIN_FUNC_ERROR;
            }

which leads to a SIGSEGV in OpenVPN, trying to setenv_link_socket_actual() with a NULL pointer for "info":

2021-01-21 10:21:41 us=297873 194.97.140.21:13279 PLUGIN_CALL: plugin function PLUGIN_ENABLE_PF failed with status 1: /root/t_server/tun-udp-p2mp-global-authpam/defer-simple.so
2021-01-21 10:21:41 us=297893 194.97.140.21:13279 WARNING: failed to init PF plugin, rejecting client.

Program received signal SIGSEGV, Segmentation fault.
setenv_trusted (es=0x555555668170, info=0x0) at socket.c:2449
2449        setenv_link_socket_actual(es, "trusted", &info->lsa->actual, SA_IP_PORT);
(gdb) where
#0  setenv_trusted (es=0x555555668170, info=0x0) at socket.c:2449
#1  0x000055555558baa1 in multi_client_disconnect_setenv (mi=0x555555666e10)
    at multi.c:562
#2  multi_client_disconnect_script (mi=mi@entry=0x555555666e10) at multi.c:574
#3  0x000055555558e308 in multi_close_instance (m=0x7fffffffc4b0, 
    mi=0x555555666e10, shutdown=<optimized out>) at multi.c:680
#4  0x0000555555591b2d in multi_create_instance (m=m@entry=0x7fffffffc4b0, 
    real=real@entry=0x7fffffffc350) at multi.c:837
#5  0x000055555558b467 in multi_get_create_instance_udp (m=0x7fffffffc4b0, 
    floated=floated@entry=0x7fffffffc3df) at mudp.c:103
#6  0x0000555555592032 in multi_process_incoming_link (
    m=m@entry=0x7fffffffc4b0, instance=instance@entry=0x0, 
    mpp_flags=mpp_flags@entry=5) at multi.c:3145
#7  0x000055555558b05c in multi_process_io_udp (m=0x7fffffffc4b0) at mudp.c:231
#8  tunnel_server_udp_single_threaded (top=top@entry=0x7fffffffd520)
    at mudp.c:356
#9  0x000055555558b595 in tunnel_server_udp (top=top@entry=0x7fffffffd520)
    at mudp.c:382
#10 0x0000555555592f7f in tunnel_server (top=top@entry=0x7fffffffd520)
    at multi.c:4051
#11 0x0000555555596e89 in openvpn_main (argc=2, argv=0x7fffffffe548)
    at openvpn.c:287

[Gert Döring]

Chased it down...

pf_init_context() is called in this call chain

#0  pf_init_context (c=0x555555650fd0) at pf.c:620
#1  0x000055555557add9 in init_instance (c=c@entry=0x555555650fd0, 
    env=<optimized out>, flags=flags@entry=10) at init.c:4430
#2  0x000055555557bc6f in inherit_context_child (
    dest=dest@entry=0x555555650fd0, src=src@entry=0x7fffffffc598)
    at init.c:4583
#3  0x0000555555590f03 in multi_create_instance (m=m@entry=0x7fffffffc4d0, 
    real=real@entry=0x7fffffffc370) at multi.c:782
#4  0x000055555558aca7 in multi_get_create_instance_udp (m=0x7fffffffc4d0, 
    floated=floated@entry=0x7fffffffc3ff) at mudp.c:103
#5  0x0000555555591312 in multi_process_incoming_link (
    m=m@entry=0x7fffffffc4d0, instance=instance@entry=0x0, 
    mpp_flags=mpp_flags@entry=5) at multi.c:3145

and if pf_init_context() sets up a signal (which it does, on failure) it hits

    init_instance(dest, src->c2.es, CC_NO_CLOSE | CC_USR1_TO_HUP);
    if (IS_SIG(dest))
    {
        return;
    }

which comes right before

    /* UDP inherits some extra things which TCP does not */
    if (dest->mode == CM_CHILD_UDP)
    {
        /* inherit buffers */
        dest->c2.buffers = src->c2.buffers;

        /* inherit parent link_socket and tuntap */
        dest->c2.link_socket = src->c2.link_socket;

        ALLOC_OBJ_GC(dest->c2.link_socket_info, struct link_socket_info, &dest->gc);
        *dest->c2.link_socket_info = src->c2.link_socket->info;

        /* locally override some link_socket_info fields */
        dest->c2.link_socket_info->lsa = &dest->c1.link_socket_addr;
...

• so, link_socket_info is NULL, when trying to finalize this instance.

Quote plaisthos

13:37 <@plaisthos> yepp
13:37 <@plaisthos> I am not sure this is worth trying to fix
13:37 <@plaisthos> as a proper fix would mean a refactoring of the pf related

functions

[Gert Döring]

​ https://patchwork.openvpn.net/patch/1564/

this is a very simple approach: make ENABLE_PF failures M_FATAL, and document why.

Then, ripp out PF in 2.6 :-)

[Gert Döring]

commit 6a0c51baaa4d2b329183601ec35d3d16f127519e (master)
commit cbbdcd4f97bb19e5be6c11cf94397b38e869a0ee (release/2.5)
Author: Gert Doering
Date: Thu Jan 21 14:39:29 2021 +0100

Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL

for reference, the commit that actually broke it is

commit c252dcc073155567c1982611ec6f065342909287
Author: Arne Schwabe <arne@…>
Date: Fri Jul 3 11:55:06 2020 +0200

Remove did_open_context, defined and connection_established_flag

and the mail thread in the patchwork URL above has some more thoughts on how to fix this, if it turns out someone really wants to re-vive PF and bring it back to fully maintained.