[PATCH] Add openconnect_get_client_cert() to API

[David Woodhouse]

On Tue, 2011-09-27 at 16:18 +0300, Jussi Kukkonen wrote:
> On 09/19/2011 10:48 AM, David Woodhouse wrote:
> > On Mon, 2011-09-19 at 09:45 +0300, Jussi Kukkonen wrote:
> >> I still think it would make sense to make the certificate expiry date
> >> available to the application if possible (I suggested _get_client_cert()
> >> because I imagined other details in the cert could be useful as well).
> >> Creating user messages without the date is doable but not really optimal.
> > 
> > Yeah, I'm more than happy adding _get_client_cert(), which could even
> > call load_certificate() if the cert hasn't already been loaded. So you
> > *could* call it before connection if you really wanted to, or you can
> > call it when you receive a certificate warning message.
> 
> Sorry for the delay.
> 
> I'm not totally sure how to do this: load_certificate() assumes non-NULL
> vpninfo->https_ctx (and will segfault without one) and vpninfo only sets
> that in openconnect_open_https(), where it checks CA files and cert
> expiry... Should I make load_certificate() usable without a SSL_CTX as a
> special case for CERT_TYPE_PEM or create the SSL_CTX on demand as well?

We could rip the if (!vpninfo->https_ctx) { ... }  bit out of
openconnect_open_https() and stick it in a helper function which is
called from both places? Called setup_ssl_context() or something like
that?

> It's also possible that I'm just getting stuck on an insignificant
> detail here and we should just work on the generic case where errors and
> notices are shown after a connection attempt...

Something like http://david.woodhou.se/openconnect-show-notice.patch ?

I think we need to do that *anyway*, even if we also do something
special for certificate expiry. So perhaps we should do that first, and
then see if we can live with the resulting behaviour for expired certs?

We should probably add a 'PRG_NOTICE' log level; currently we only have
ERR, INFO, DEBUG and TRACE. The UI can then show NOTICE messages with
GTK_STOCK_DIALOG_WARNING and ERR messages with GTK_STOCK_DIALOG_ERROR.

So... we need an API break (well, a new openconnect_vpn_new_foo function
at least; the old ones can continue to work with the old PRG_* values
being translated for them). Next question: what *else* do we want to
break at the same time? Do we really want to add a message-id to each
message so that the UI can 'interpret' it and do something appropriate?
That seems... wrong to me somehow. Or do we want some kind of 'flags',
where we can say 'this is a certificate error' or 'this is a...'

...hm, what else *would* we want to handle specially in the UI?

And for that matter, what exactly are we going to do even for the
certificate? The message says:
 "Client certificate expires soon at 2011-09-28 11:00 GMT"

Do we really need more than that, shown with a warning triangle as
you're entering your password?

>  Feel free to tell me if it looks that way. It's just quite tempting
> to fail as fast as possible from UI POV.

Well, it shouldn't necessarily be 'fail'. I wouldn't want to abort the
connection and refuse to connect. The server *might* accept expired
certs anyway, or might not actually need a cert at all any more.

And I wouldn't necessarily want to demand the certificate passphrase
before we've tried to connect to the server, either.

You could automatically fail the user interaction if it's needed at that
point, I suppose, but then you end up with inconsistent behaviour; you
report it at a different time depending on whether the cert needs a
passphrase or not.

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