568929 - TLS connection to IMAP server does not prompt for client certificate password

Status: RESOLVED FIXED

(Thunderbird :: Security, defect)

Description

[Stacy Millions]

User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.2.3) Gecko/20100408 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.2.4) Gecko/20100528 Lightning/1.0b2pre Thunderbird/3.1

when connecting to an imap server using STARTTLS and using a client certificate I am not prompted with the expected "Please enter the master password for the Software Security Device." so that the TLS connection can be authenticated using the client certificate.

Reproducible: Always

Steps to Reproduce:
1. install a client certificate in the certificate manager
2. select STARTTLS for the connection security in the server settings
3. connect to the server

Actual Results:  
you are prompted to "Choose a certificate to present as identification"
but there is no prompt for the "master password"
and the TLS connection is initiated without authentication (no client cert)

Expected Results:  
prompted to "Choose a certificate to present as identification"
prompted for the "master password"
resulting in an authenticated TLS session

if you do something that prompts you for the master password before making the TLS connection (examples are SMTP connection using TLS or going to the certificate manager and backing up your certificate) then the TLS connection will be properly authenticated.

the following error is printed:
###!!! ASSERTION: callbacks does not implement nsIPrompt: 'PR_FALSE', file nsNSSCallbacks.cpp, line 800

This is not a FreeBSD specific issue, I have tested on Ubuntu 10.04 as well.

Comment 1

[David :Bienvenu]

The master password protects passwords, not certificates, as I understand it. I think that's also true for Firefox.

Comment 2

[Matthias Versen [:Matti]]

a client certificate should be protected with the masterpassword...
My email cert in SM requires a masterpassword for example.

Comment 3

[Stacy Millions]

(In reply to comment #1)
> The master password protects passwords, not certificates, as I understand it. I
> think that's also true for Firefox.

I realise that there is a "master password" that protects the saved passwords and that it is different from what protects the private keys, but the prompt says:
"Please enter the master password for the Software Security Device."
So I referred to it as a "master password", I believe what it is actually asking for is the pass phrase that protects the private keys in the PKCS11 module.

Comment 4

[David :Bienvenu]

Thunderbird doesn't know about any of that - it merely makes an ssl connection, and several layers below that, necko asks security library for a cert, all well below the covers. But Firefox does ask for the master password before using a client cert, so it must be possible for Thunderbird to do the same.

Comment 5

[Stacy Millions]

(In reply to comment #4)
> Thunderbird doesn't know about any of that - it merely makes an ssl
> connection, and several layers below that, necko asks security library for
> a cert, all well below the covers.

OK, I think I see where you are coming from on this... what Thunderbird needs to do is setup the callbacks correctly so that the lower layers of code can prompt for the pass phrase if they need to.

I believe that is the origins of the ASSERTION error; the NSS layer is trying to ask for the pass phrase, but the callback that he has does not support the required interface.

>                   But Firefox does ask for the master password before using a
> client cert, so it must be possible for Thunderbird to do the same.

Actually, Thunderbird does do it; for an SMTP connection that uses TLS and requires a client certificate it is all handled correctly.

Unfortunately, I have been unable to figure out what is missing in the IMAP code, the two pieces of code don't seem to have much in common when it comes to creating network connections.

Comment 6

[Ben Bucksch (:BenB)]

I think we're simply not supporting client cert authentication at all in IMAP. I think that should be AUTH EXTERNAL, and it should be explicit (by the server offering, and the client explicitly invoking, AUTH EXTERNAL), not implicitly on the SSL layer only.
For some strange reason, we support AUTH EXTERNAL (but without special password prompts) in SMTP, but not in IMAP.

Comment 7

[Ben Bucksch (:BenB)]

http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgProtocol.cpp#123
http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsSmtpProtocol.cpp#329
but that's where I am loosing the trail

Comment 8

[Stacy Millions]

(In reply to comment #6)
> I think we're simply not supporting client cert authentication at all in IMAP.

Whether intentional or accidental, the support is there; assuming that the private key is unlocked before the imap connection is made.

> I think that should be AUTH EXTERNAL, and it should be explicit (by the server
> offering, and the client explicitly invoking, AUTH EXTERNAL), not implicitly
> on the SSL layer only.

The problem with that is, the IMAP server can request that the TLS layer be authenticated with a client certificate even if it doesn't support AUTH EXTERNAL. AUTH EXTERNAL requires an authenticated TLS sessions, but an authenticated TLS sessions does not imply AUTH EXTERNAL.

> For some strange reason, we support AUTH EXTERNAL (but without special
> password prompts) in SMTP, but not in IMAP.

I'm not sure I understand what you mean by "without special password prompts". If the TLS layer is established without the private key for the client certificate then the TLS session will be unauthenticated. If the key was already unlocked then the TLS session will be authenticated with the client cert, but the same is true of IMAP.

BTW attachment 448079 [details] [diff] [review] adds support for AUTH EXTERNAL in IMAP, which is how I found this issue.

Comment 9

[Ben Bucksch (:BenB)]

> AUTH EXTERNAL requires an authenticated TLS sessions, but an
> authenticated TLS sessions does not imply AUTH EXTERNAL.

I claim the latter is broken. Authentication should be explicit and under client control, and not somewhere on SSL layer.
That's why AUTH EXTERNAL exists in the first place.

> I'm not sure I understand what you mean by "without special password prompts".

There's no explicit code which gets a password from user and hands it to the SSL layer, like it is for all the other auth schemes. It's the SSL layer requesting a prompt, and something, somewhere (I didn't find where, that "lost trail" I mentioned), popping up a dialog, entirely outside the normal code.

Comment 10

[Stacy Millions]

(In reply to comment #9)
> > AUTH EXTERNAL requires an authenticated TLS sessions, but an
> > authenticated TLS sessions does not imply AUTH EXTERNAL.
> 
> I claim the latter is broken.

I don't see why. GSSAPI over TLS is perfectly acceptable and there is no reason that the TLS session can't be authenticated as well.

>                         Authentication should be explicit and under
> client control, and not somewhere on SSL layer.

No, authentication should definitely be under the control of the server, not the client.

> That's why AUTH EXTERNAL exists in the first place.

AUTH EXTERNAL exists to pass the identity that was established by the TLS level to a higher level. If the higher level doesn't wish to use that identity, it is under now obligation to do so. I'm just looking to make it a viable option, but not a requirement.

> > I'm not sure I understand what you mean by "without special password 
> > prompts".
> 
> There's no explicit code which gets a password from user and hands it to the
> SSL layer, like it is for all the other auth schemes. It's the SSL layer
> requesting a prompt, and something, somewhere (I didn't find where, that "lost
> trail" I mentioned), popping up a dialog, entirely outside the normal code.

The good news is I have a patch that enables that same functionality for IMAP.

Comment 11

[Stacy Millions]

Attachment: patch that builds against 3.1rc1

Comment 12

[David :Bienvenu]

Comment on attachment 448248 [details] [diff] [review]
patch that builds against 3.1rc1

thx, that looks right - I figured it had to be the interface requestor stuff...I just need to maek sure our cert exception code still works after this, but it should.

Comment 13

[Ludovic Hirlimann [:Usul]]

(In reply to comment #12)
> (From update of attachment 448248 [details] [diff] [review])
> thx, that looks right - I figured it had to be the interface requestor
> stuff...I just need to maek sure our cert exception code still works after
> this, but it should.

Can we add a few xpshell test for that purpose ?

Comment 14

[David :Bienvenu]

(In reply to comment #13)
 
> Can we add a few xpshell test for that purpose ?

the cert exception stuff is in the front end, and xpcshell tests don't have real doc shells, so I'm not sure there's a test that would be super-meaningful.

Comment 15

[David :Bienvenu]

I need a patch that applies against the trunk, not 3.1rc1, since this feature isn't going to land for 3.1...and this patch doesn't seem to apply against the trunk, and if I apply it by hand, it doesn't compile, probably because of changes between moz central 1.9.2 and 1.9.3

Comment 16

[David :Bienvenu]

Attachment: trunk patch

this builds against the trunk.

Comment 17

[David :Bienvenu]

Comment on attachment 448440 [details] [diff] [review]
trunk patch

this seems to work fine.

Comment 18

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/13d1d63f3973

Comment 19

[David :Bienvenu]

Stacy, thanks for the patch!

Comment 20

[Mark Banner (:standard8)]

Bienvenu, is this bug/patch suitable for 3.1?

Comment 21

[David :Bienvenu]

(In reply to comment #20)
> Bienvenu, is this bug/patch suitable for 3.1?

The bug is suitable; the patch on the trunk won't build for 3.1, but Stacy attached an earlier patch that does apply and work for 3.1, which I've just un-obsoleted. It does need tabs removed, however. I'll attach a version w/o tabs in a minute.

Comment 22

[David :Bienvenu]

Attachment: 3.1 version -p1 patch w/o tabs

We could take this for 3.1.2 or 3.1.3...

Comment 23

[Stacy Millions]

(In reply to comment #22)
> We could take this for 3.1.2 or 3.1.3...

Any chance of bug 286581 accompanying it?

Comment 24

[David :Bienvenu]

No, bug 286581 has to land on the trunk first, before we can consider it for 3.1.x, or 3.2. I need to get it reviewed and landed on the trunk; I've been meaning to pick it up again...

Comment 25

[Mark Banner (:standard8)]

Stacy, do you know if this is a regression from a previous version of Thunderbird or something that never worked? If its a previous version, do you know roughly which one it regressed in (or what you had before it regressed)?

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 459017 [details] [diff] [review]
3.1 version -p1 patch w/o tabs

I think given what this touches (although small), I would rather we have a build with a beta period before we ship this. Therefore approved for 3.1.3 - please land on comm-1.9.2 default when ready so it gets into the nightlies.

Comment 27

[Stacy Millions]

(In reply to comment #25)
> Stacy, do you know if this is a regression from a previous version of
> Thunderbird or something that never worked? If its a previous version, do you
> know roughly which one it regressed in (or what you had before it regressed)?

More like part way between regression and never worked. It had worked previously, but had not been implemented correctly. There was a piece of code that did something like triggering the unlocking of the certificate database if TLS was enabled instead of setting up the call backs and letting the TLS layer take care of it if needed. If I remember correctly, that piece of code was removed between 3.0 and 3.1... I'm afraid I don't track the sources with much granularity.

Comment 28

[David :Bienvenu]

pushed to comm-central 1.9.2 changeset:   5745:90827ff08702

Comment 30

[Ludovic Hirlimann [:Usul]]

Would be nice to relnote that for 3.1.2