On 30/09/17 00:24, Steffan Karger wrote:
> This changes the behavior for pf plugins: instead of just not initializing
> the firewall rules and happily continuing, this now rejects the client in
> the case of an (unlikely) failure to initialize the pf.
> 
> Signed-off-by: Steffan Karger <stef...@karger.me>
> ---
>  src/openvpn/pf.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c
> index 5cb002bf..29231b67 100644
> --- a/src/openvpn/pf.c
> +++ b/src/openvpn/pf.c
> @@ -639,10 +639,11 @@ pf_init_context(struct context *c)
>                  }
>  #endif
>              }
> -            else
> -            {
> -                msg(M_WARN, "WARNING: OPENVPN_PLUGIN_ENABLE_PF disabled");
> -            }
> +        }
> +        if (!c->c2.pf.enabled)
> +        {
> +            msg(M_WARN, "WARNING: failed to init PF plugin, rejecting 
> client.");
> +            register_signal(c, SIGUSR1, "plugin-pf-init-failed");
>          }

after this part there is some code dealing with the management
interface: wouldn't it be better to have a return after registering the
signal so that we don't attempt any interaction with the management
interface? If the plugin initialization failed, we should exit right
away no?

(although this PF thing with the plugin being optional makes everything
more complicated every time :/)

Another thought: if we abort every time we can't initialize the pf
context, do you think we need the c->c2.pf.enabled member at all?

It looks to me that after this patch if PF is enabled, either a client
is connected and has a PF context, or its connection was rejected
entirely. Thus a overall state should be enough rather than a per-client
one. (Can probably be adjusted with another patch if you don't feel like
doing all this in here?)

Cheers,

>      }
>  #endif /* ifdef PLUGIN_PF */
> 

-- 
Antonio Quartulli

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to