Re: [Evolution-hackers] Camel Authentication Changes
On Thu, 2011-10-20 at 01:14 +0100, David Woodhouse wrote: This looks like a good idea, and cleaning up the various ways that the providers were behaving differently in that authentication loop (and calling directly out to the GUI) is a good thing. I looked at those recently when implementing the -try_empty_password() SASL methods for automatic NTLM auth, and I take my hat off to you for finding a way through the mess. Having said that, I do think we need to adjust this approach to deal with authentication as a *reactive* thing, not *proactive*. It *used* to be reactive, but you changed it as a side-effect of your improvements. The point is that we authenticate ourselves *if* and *when* the server asks us to. We cannot know in advance whether we're going to need a password or not. We discussed this at length on IRC last night and I'm still digesting all the implications of auth cases I hadn't considered. I'm hoping that, at least for the time being, we can get by with the current API with only some tweaks to the authentication loop logic. Down the road I'm perfectly happy to revisit this, and even break APIs if needed to do it right, but right now Red Hat is pressuring me to get this account rewrite branch of mine finished as quickly as possible. So for now I really have to move on from this. That's fine at the moment, but the fact that we have to explicitly choose *one* mechanism in advance ought to be considered a bug. In the future, we ought to simply try *any* of the methods that the server offers today, in some order of preference. So we could try Kerberos, fall back to NTLM, etc. These changes seem to have made that harder to fix. I think we could achieve this with multiple calls to camel_session_authenticate_sync() with a different mechanism name each time (kerberos, ntlm, login), and making sure the session logic knows when to loop and when to bail out if an auth attempt fails. It already kinda does that by looking up the CamelServiceAuthType struct associated with the mechanism name (if one was given) and checking its 'need_password' flag. If FALSE, it does ONE authentication attempt and bails on failure (instead of looping). After which you could possibly call camel_session_authenticate_sync() again with a fallback mechanism name. Problem is when a mechanism name is NOT given, as is the case for most HTTP-based backends, or when 'need_password' is TRUE, it assumes you'll *will* need a password to authenticate and prompts if one is not found in the keyring (which I understand now to be a bad assumption). That's the part that needs tweaking, for now. It looks like it'll always ask for a password for HTTP-based backends, but those have similar options; they may authenticate with Kerberos, or with automatic-NTLM, or with cookies... or they *may* actually decide that they need a password today. Right. I've made similar changes to the addressbook and calendar backends on my branch, more of which are HTTP based, and what I did there was first try to run some initial inconsequential operation (list calendars or some such) with no auth header at all, and if that fails with a 401 Unauthorized *then* initiate authentication in a similar manner to camel_session_authenticate_sync(). I realize that doesn't cover all the use cases you were throwing at me, particularly the server password suddenly changing in the middle of a session, but perhaps that approach, in combination with the tweaking I mentioned above, would address the premature password prompt in EWS for the time being? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Thu, 2011-10-20 at 12:50 -0400, Matthew Barnes wrote: I think we could achieve this with multiple calls to camel_session_authenticate_sync() with a different mechanism name each time (kerberos, ntlm, login), and making sure the session logic knows when to loop and when to bail out if an auth attempt fails. It already kinda does that by looking up the CamelServiceAuthType struct associated with the mechanism name (if one was given) and checking its 'need_password' flag. If FALSE, it does ONE authentication attempt and bails on failure (instead of looping). After which you could possibly call camel_session_authenticate_sync() again with a fallback mechanism name. Problem is when a mechanism name is NOT given, as is the case for most HTTP-based backends, or when 'need_password' is TRUE, it assumes you'll *will* need a password to authenticate and prompts if one is not found in the keyring (which I understand now to be a bad assumption). That's the part that needs tweaking, for now. Yeah, the provider could just call camel_session_authenticate_sync() as and when it actually *needs* a password (which looks fairly much to be what imapx *is* doing, so I'm not really sure why it's demanding my password unnecessarily). The issue is the complexity. What happens is that the provider calls out to camel_session_authenticate_sync(), which then calls back to the provider to give it the information it wanted... but it provides it in a *synchronous* method and waits for a result, so we can't just hand off the password with a GAsyncResult to the code which called camel_session_authenticate_sync() in the first place; we have to do nasty things with threads and avoid deadlock while we make our provider's -authenticate_sync() actually wait for the completion of the function which *called* it. Forget EWS for now; let's just look at it in imapx. This bit in imapx_create_new_connection(), for example: /* XXX As part of the connect operation the CamelIMAPXServer will * have to call camel_session_authenticate_sync(), but it has * no way to pass itself through in that call so the service * knows which CamelIMAPXServer is trying to authenticate. * * IMAPX is the only provider that does multiple connections * like this, so I didn't want to pollute the CamelSession and * CamelService authentication APIs with an extra argument. * Instead we do this little hack so the service knows which * CamelIMAPXServer to act on in its authenticate_sync() method. * * Because we're holding the CAMEL_SERVICE_REC_CONNECT_LOCK * (and our own CON_LOCK for that matter) we should not have * multiple IMAPX connections trying to authenticate at once, * so this should be thread-safe. */ imapx_store-authenticating_server = g_object_ref (conn); success = camel_imapx_server_connect (conn, cancellable, error); g_object_unref (imapx_store-authenticating_server); Then in imapx_reconnect() you call camel_session_authenticate_sync() which ends up calling *back* to imapx's imapx_authenticate_sync() method... which uses that nastily-stashed 'server' object... which was RIGHT THERE in the imapx_reconnect() function. We could have just returned the password to imapx_reconnect() and all would have been well. I'm unlikely to get much done before I disappear to Prague on Saturday, and I'm on baby duty tomorrow because she's unwell and can't go to nursery, but I'll try to put together a patch which sorts this out. Maybe during next week. -- dwmw2 ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Sat, 2011-10-15 at 11:18 -0400, Matthew Barnes wrote: Just a heads up that I've changed Camel's authentication API to factor out the password loop that each and every provider currently replicates for themselves. Turns out they all have the same basic logic flow. These changes make password management more consistent and also make life easier for providers. Now when a provider reaches the step where it needs to authenticate, it simply calls: gboolean camel_session_authenticate_sync (CamelSession *session, CamelService *service, const gchar *mechanism, GCancellable *cancellable, GError **error); This looks like a good idea, and cleaning up the various ways that the providers were behaving differently in that authentication loop (and calling directly out to the GUI) is a good thing. I looked at those recently when implementing the -try_empty_password() SASL methods for automatic NTLM auth, and I take my hat off to you for finding a way through the mess. Having said that, I do think we need to adjust this approach to deal with authentication as a *reactive* thing, not *proactive*. It *used* to be reactive, but you changed it as a side-effect of your improvements. The point is that we authenticate ourselves *if* and *when* the server asks us to. We cannot know in advance whether we're going to need a password or not. I tried this code, and the first thing it did was ask me for a password for my IMAP connection, even though that IMAP account uses a custom connection command 'ssh $mailhost /usr/libexec/dovecot/imap' so it gets a *preauthenticated* connection. (At other times, the custom command might have been something different like 'ssh $bastionhost openssl s_client -connect $internalserver:993' and the authentication *would* still have been required. You just can't know until you connect.) Even if the server asks for a password, with things like automatic NTLM authentication you can't know whether the automatic challenge/response will work. The old loop in the providers would handle that — it would try first with the empty password, then it would only prompt the user for a password if it really needed one. I think you've broken that in master now, because session_authenticate_sync() will just return immediate success if the SASL -try_empty_password() method indicates that we can even *try* to authenticate without a user-provided password. The mechanism argument refers to the user's preferred SASL mechanism. This will usually be taken from url-authmech That's fine at the moment, but the fact that we have to explicitly choose *one* mechanism in advance ought to be considered a bug. In the future, we ought to simply try *any* of the methods that the server offers today, in some order of preference. So we could try Kerberos, fall back to NTLM, etc. These changes seem to have made that harder to fix. but it can also be NULL when not using SASL (such as for HTTP-based backends). It looks like it'll always ask for a password for HTTP-based backends, but those have similar options; they may authenticate with Kerberos, or with automatic-NTLM, or with cookies... or they *may* actually decide that they need a password today. All of the above issues have a single solution — we need to change things so that the password is requested *only* when we're actually connected to the server and we have discovered that we actually need it. I think the best way to handle it is to extend camel_service_get_password(), which is already called from the providers in the right place. If we make *it* trigger the password dialog, rather than doing the dialog in advance, then I think we solve all the issues. We'll also want to add a 'retrying' argument, for the caller to indicate that the last password it received was not correct. That'll prompt for actual user interaction, rather than just handing out the password from gkr, for example. The CamelSession (or subclass) now handles all the looping. So if the function returns TRUE, you're authenticated. If it returns FALSE, you treat it like any other failed operation. You don't loop. Aside from the correctness issues, doing it internally can be a lot more efficient. Imagine you're on a slow connection and each DNS lookup, TCP connection establishment and SSL negotiation takes about 30 seconds. Now, do you want to connect a first time and work out that your stored password is invalid, then tear it all down and make a *new* connection, only to find that the user fat-fingered the password and you need to try a *third* time? Or do you want to keep the same connection open and issue three consecutive AUTH commands, looping *inside* the IMAP provider? There are similar considerations when the password changes during a session. TCP connections may break and
Re: [Evolution-hackers] Camel Authentication Changes
Hi Matthew, Am Montag 17 Oktober 2011, um 23:48:01 schrieb Matthew Barnes: On Mon, 2011-10-17 at 15:21 +0200, Christian Hilberg wrote: Fair enough, thanks for clarifying. If that's the current status, then nothing is lost if we keep the implementation as-is for now and try it again later on. Still not sure how this whole security puzzle fits together yet, but this sounds like a piece of it: http://stef.thewalter.net/2011/10/redesigning-seahorse-experience.html Seahorse (or the library stack that Seahorse is built on) is supposedly adding support for NSS certificates by GNOME 3.4. This reads interesting, for sure. That means we should soon be able to plug into Seahorse for certificate management instead of talking to NSS directly some time next year. At least that's my hope. The email (backend) factory which is in the makings for carving out email handling from the Evo frontend would surely need a way to be fed with passwords, be it standard user auth or any more advanced thing like opening a security token with a PIN and reading authentication data (like client certificates) from there. Once configured in seahorse, Evo might not need to do more than presenting a dialogue for the general seahorse (stack) password, and everything is set from there on, since the email factory, and possibly other factories as well, will be granted access to the passwords or other authentication data they need, all handled by a service which is controlled/configured via seahorse. Maybe this is a perspective for solving that whole security puzzle? I highly encourage you to contact Stef about your smart card issue, as he can certainly paint a clearer picture than I can. I will do so. I've seen his posts on gnutls-devel regarding the PKCS#11 stuff, and it really seems things start working out in this area. I'm presently having the issue that OpenLDAP won't use a client certificate, since it builds against GnuTLS, and the security token handling is not transparent there for a lib like OpenLDAP. Instead, the application needs to handle all details by itself. My hope would be that this whole seahorse effort will solve most of the trouble... :) Thanks for the hint and kind regards, Christian -- kernel concepts GbRTel: +49-271-771091-14 Sieghuetter Hauptweg 48 D-57072 Siegen http://www.kernelconcepts.de/ signature.asc Description: This is a digitally signed message part. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Sat, 2011-10-15 at 11:18 -0400, Matthew Barnes wrote: Just a heads up that I've changed Camel's authentication API to factor out the password loop that each and every provider currently replicates for themselves. Turns out they all have the same basic logic flow. Hi, I'm currently trying to adapt evo-ews to current git master and this change doesn't make much sense there, at least for me, because on the first look there is used a libsoup for authentications, thus no such auth_loop or any same basic logic. I've a larger patch for the ews under go, which I will finish tomorrow, I only wanted to let you know about this and I hoped that you'll help with this particular thing. Thanks in advance, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Tue, 2011-10-18 at 21:29 +0200, Milan Crha wrote: I'm currently trying to adapt evo-ews to current git master and this change doesn't make much sense there, at least for me, because on the first look there is used a libsoup for authentications, thus no such auth_loop or any same basic logic. I've a larger patch for the ews under go, which I will finish tomorrow, I only wanted to let you know about this and I hoped that you'll help with this particular thing. You might have to implement authenticate() and authenticate_finish() instead of authenticate_sync(). Cache the password that authenticate() gives you, hand it to libsoup when it asks for it, and when libsoup responds with a status code, complete the GSimpleAsyncResult you created in authenticate(). ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Tue, 2011-10-18 at 15:58 -0400, Matthew Barnes wrote: On Tue, 2011-10-18 at 21:29 +0200, Milan Crha wrote: I'm currently trying to adapt evo-ews to current git master and this change doesn't make much sense there, at least for me, because on the first look there is used a libsoup for authentications, thus no such auth_loop or any same basic logic. I've a larger patch for the ews under go, which I will finish tomorrow, I only wanted to let you know about this and I hoped that you'll help with this particular thing. You might have to implement authenticate() and authenticate_finish() instead of authenticate_sync(). Cache the password that authenticate() gives you, hand it to libsoup when it asks for it, and when libsoup responds with a status code, complete the GSimpleAsyncResult you created in authenticate(). Note that this also needs to cope with the case where the password *becomes* invalid in the middle of an active session. The user should be prompted for a new password (repeatedly until they get it right), and then it should work for *all* of mail/cal/ebook access. And if they hit *cancel* instead of entering a valid password, hopefully it'll stop asking rather than popping up a dialog every few seconds until they change to a VT and run 'killall evolution' :) -- dwmw2 smime.p7s Description: S/MIME cryptographic signature ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
Hi Matthew, Am Samstag 15 Oktober 2011, um 17:18:27 schrieb Matthew Barnes: Just a heads up that I've changed Camel's authentication API to factor out the password loop that each and every provider currently replicates for themselves. Turns out they all have the same basic logic flow. [...] I take it this new password API deals with IMAP|POP|SMTP|... service passwords _only_, i.e. service user authentication? I'm asking to be sure about this, since I'm still thinking about the passing in of e.g. a security token PIN, be it a CamelService running inside Evolution (for which a PIN dialog implementation exists) or a CamelService running in our evolution-kolab backend (for which we found no clean way when implementing it half a year back). I still do not have grokked enough of the current Evo/EDS implementation and the design plans for the near future to come up with a better solution than the demonstrator we currently have ... which is, to pass the PIN from Evo to the backend via an account configuration detail (which gets stored on disk and therefore is not a solid implementation, but no more than our small, dirty hack around our lack of a better idea at the time we implemented it). Kind regards, Christian -- kernel concepts GbRTel: +49-271-771091-14 Sieghuetter Hauptweg 48 D-57072 Siegen http://www.kernelconcepts.de/ signature.asc Description: This is a digitally signed message part. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Mon, 2011-10-17 at 11:44 +0200, Christian Hilberg wrote: I take it this new password API deals with IMAP|POP|SMTP|... service passwords _only_, i.e. service user authentication? Correct. I don't have a good answer for you on the security token PIN use case at the moment. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
Hi there, Am Montag 17 Oktober 2011, um 14:51:08 schrieb Matthew Barnes: On Mon, 2011-10-17 at 11:44 +0200, Christian Hilberg wrote: I take it this new password API deals with IMAP|POP|SMTP|... service passwords _only_, i.e. service user authentication? Correct. I don't have a good answer for you on the security token PIN use case at the moment. Fair enough, thanks for clarifying. If that's the current status, then nothing is lost if we keep the implementation as-is for now and try it again later on. Kind regards, Christian -- kernel concepts GbRTel: +49-271-771091-14 Sieghuetter Hauptweg 48 D-57072 Siegen http://www.kernelconcepts.de/ signature.asc Description: This is a digitally signed message part. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Authentication Changes
On Mon, 2011-10-17 at 15:21 +0200, Christian Hilberg wrote: Fair enough, thanks for clarifying. If that's the current status, then nothing is lost if we keep the implementation as-is for now and try it again later on. Still not sure how this whole security puzzle fits together yet, but this sounds like a piece of it: http://stef.thewalter.net/2011/10/redesigning-seahorse-experience.html Seahorse (or the library stack that Seahorse is built on) is supposedly adding support for NSS certificates by GNOME 3.4. That means we should soon be able to plug into Seahorse for certificate management instead of talking to NSS directly some time next year. At least that's my hope. I highly encourage you to contact Stef about your smart card issue, as he can certainly paint a clearer picture than I can. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers