Re: [Evolution-hackers] Camel Authentication Changes

2011-10-20 Thread Matthew Barnes
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

2011-10-20 Thread David Woodhouse
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

2011-10-19 Thread David Woodhouse
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

2011-10-18 Thread Christian Hilberg
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

2011-10-18 Thread Milan Crha
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

2011-10-18 Thread Matthew Barnes
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

2011-10-18 Thread David Woodhouse
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

2011-10-17 Thread Christian Hilberg
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

2011-10-17 Thread 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.

___
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

2011-10-17 Thread Christian Hilberg
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

2011-10-17 Thread 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.

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