Am 27.08.20 um 01:57 schrieb Eric Thorpe: > Hi Arne, > >> That code/commit message is explicitly talking about renegotiation. So >> if that is also broken, there seems to be something else wrong. > You are quite correct, I've muddled this up with a different issue I've > been working through with Deferred Auth, management should not be a part > of this discussion. > >> - Do we send the AUTH_FAILED over the new or the old tls session? > The AUTH_FAILED is sent in the old (current) tls session, that is of > course assuming that it is still active at the time of reneg. Normally > if reneg is done with only keys or an auth-token, there isn't the risk > of authentication being too slow. If a user takes too long to enter a > password, or deferred auth is in use, the server can sigterm the user on > it's end before authentication is complete and we have the same result. > With this said though, the configuration can be adjusted to give the > user a grace period on the server, so that specific scenario isn't > something that needs to be handled in my opinion as it can be tuned in > the configuration. > >> - Do we establish the data channel (key_method_2_write) before writing >> the key? > This is the question I specifically ignored as I'm not sure why it's > relevant to plugin/script authentication. key_method_2_write, unless I'm > mistaken (and I'm happy to be corrected here),
client starts with key_method2_write, then server does _read and then _write again and after that client does _read. See also (https://github.com/OpenVPN/openvpn/blob/master/doc/doxygen/doc_key_generation.h) After the write2 from the server and read2 from client is done, the client can assume that the keys from that TLS session are valid. > is client side. The > client knows it's time to renegotiate as part of it's regular wake up > routines or server trigger, of which key_method_2_write is called and > queries for the user/pass (either by management, file, terminal, etc), > and then pushes it all to the server. Unlike the initial connection > process however, a push_request does not follow this information push, > so if authentication fails at the server end, there is no indication of > such and we end up with desync. Yeah, that sounds about right. >> understand the implications and whether the patch is really >> fixing the root cause or just band aiding the symptoms. > This I can't answer. I don't think it's a band aid, however there are > still scenarios this will not cover as mentioned above. Someone with > more in depth knowledge will need to help review this patch to determine > if this is a fix, or indeed a band aid to a deeper problem that needs to > be reviewed. > > With this in mind, I'm afraid I have spent enough time on this. We will > continue to maintain this patch in our fork. In the mean time, we've > tried to do the right thing by submitting a patch for an issue we've > discovered and brought to the mailing lists attention a long time ago > now, and then do the right thing again updating it when requested. I > will leave the conversation here and leave it to the repo maintainers > from here whether they wish to use this patch as is, modify it to their > needs or NAK it for a better future solution. I will keep an eye out for > a new patch and assist in it's testing if someone wishes to improve upon > this. I will put this on my TODO list. Maybe I can also cleanup the code to make this a litte bit less of a mess. Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
