Gert Doering <> on Wed, 2016/12/28 19:57:
> Hi,
> On Wed, Dec 28, 2016 at 02:07:21PM +0100, Christian Hesse wrote:
> > @@ -73,6 +77,21 @@ tunnel_point_to_point(struct context *c)
> >          return;
> >      }
> >  
> > +#ifdef ENABLE_SYSTEMD
> > +    /* In non-TLS configuration we wait for the remote peer to connect
> > +     * before issuing "Initialization Sequence Completed". So prevent to
> > +     * time out by telling systemd service manager we are ready for now.
> > +     * Status will be "Non-TLS mode, ready for now. Waiting for peer..."
> > +     * and changes once the remote peer connects. */
> > +    if (c->options.tls_client == false
> > +        && c->options.tls_server == false)
> > +    {
> > +        sd_notifyf(0, "READY=1\n"
> > +                   "STATUS=Non-TLS mode, ready for now. Waiting for
> > peer...\n"
> > +                   "MAINPID=%lu", (unsigned long) getpid());
> > +    }
> > +#endif  
> We definitely need a better approach than "litter ENABLE_SYSTEMD all
> over the code".

Well, openvpn supports a number of modes of operation... Some of these have
other requirements than others.

In general we have three options:

1. Use Type=forking in unit files. We decided not to go this way.

2. Use Type=notify and tell systemd we are ready about when we would have
   forked. This works as well as Type=forking does, but has some unhandled
   corner cases.

3. Use Type=notify and "litter ENABLE_SYSTEMD all over the code". This
   provides the best error handling.

(There is Type=simple, but that is just fire-and-forget - could not be worse.)

IMHO we should go with option three. Given the number of #ifdefs all over the
code - is this really an issue? My patch increases the number of sd_notify()
calls from four to five.

For informational purpose we could add even more calls. That would allow to
set intermediate status message, something like: "Up and running, currently
serving 25 client connections."

> (Also, this is the wrong check anyway.  p2p mode can go along with
> TLS just fine - what you need to check for is --server or --client,
> which is something else than --tls-server / --tls-client)

No, the check is correct.

In server move openvpn reports ready when it finally is ready to handle
incoming connections. That is fine.

In TLS client mode openvpn reports ready when it connected to a server
successfully. It fails if it can not connect. That is the desired behavior.
So that is fine as well.

The patch handles non-TLS P2P mode: Two nodes have a shared secret (or
connect unencrypted in pure tunnel mode) and connect to each other.
The culprit: There is no node that is ready all the time. As we do not want
to bail out before the other is connects we have to report ready after
initialization but before the remote connects. This really is about non-TLS,
not client/server. No?
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}

Attachment: pgpAoSJ3N2PI_.pgp
Description: OpenPGP digital signature

Check out the vibrant tech community on one of the world's most 
engaging tech sites,!
Openvpn-devel mailing list

Reply via email to