Move DTLS secret initialisation to openconnect_setup_dtls()

[David Woodhouse]

On Thu, 2014-06-26 at 09:30 -0700, Kevin Cernekee wrote:
> > This was added in commit ec2408e5 ("dtls: Align new-tunnel rekey behavior
> > with Cisco clients"), and was causing the NetworkManager authentication
> > dialog to crash because it was calling openconnect_random() before the SSL
> > library was initialised by openconnect_init_ssl().
> >
> > The auth dialog didn't need it anyway. Move it to openconnect_setup_dtls()
> > where it belongs.
> 
> When I tried running with this patch, it caused
> start_cstp_connection() to send a dtls_secret value that was all
> zeroes.

Oops, sorry about that. Hopefully a better fix in commit 105b5a5. And
I've added a sanity check too.

Hm, would be useful if --dump-http-traffic also dumped the CONNECT
request. I might change process_http_response() to take the request as a
parameter too, and actually send it for itself. And then the existing
/* FIXME: Use process_http_response() ... */ in  start_cstp_connection()
will cover that without having to export dump_buf() from http.c

> > Clear got_cancel_cmd when returning from openconnect_obtain_cookie()
> >
> > Otherwise, nothing ever clears it and next time the auth dialog calls
> > openconnect_obtain_cookie() to attempt a connection, it will immediately
> > abort.
> 
> Do we have any guidelines on when it is legal to reuse a "dirty"
> library instance left over from a failed connection?
> 
> On Android I am assuming the worst - once anything has disconnected
> for any reason, I create a new instance.  This might be too
> pessimistic.

I'm not sure about "guidelines". Remember, the NM authentication dialog
was incestuously developed along with OpenConnect long before we split
things out into a shared library and started trying to be sensible about
it.

If it cancels a connection attempt, it does attempt to try again with
the same vpninfo instance — that's what openconnect_reset_ssl() is for.

To reconnect, the NM auth dialog will read any pending bytes from the
(legacy) cancel_fd for itself, to prevent them from triggering another
cancellation. Then it'll call openconnect_reset_ssl(),
openconnect_set_hostname(), and finally openconnect_obtain_cookie()
again.

Actually, now I come to think about it, I suspect we should be clearing
->got_cancel_cmd in openconnect_reset_ssl() instead of when we return
from openconnect_obtain_cookie(). I've just pushed that change.

-- 
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20140627/dda3f658/attachment.bin>