530319 - Kerberos/GSSAPI auth fails when the hostname is changed after account creation

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Ben Bucksch (:BenB)]

Reproduction:
0. Set up /etc/krb5.conf with realms CORRECT.COM and OLD.COM
1. Create account for username you at nonsense.old.com ,
   with Kerberos / secure auth .
2. Restart Thunderbird to make sure the prefs are persisted
3. Change the account to point to username you at yourserver.correct.com
4. kinit [e-mail] on console
5. Click on the Imbox of that account in Thunderbird
6. Check the KDC authentication request logs on the Kerberos server
7. Try the same with a fresh profile, with the correct hostname from the start.

Actual result:
Step 5 fails
Step 6 shows that the client [e-mail] tries to get a ticket for krbtgt/[e-mail], which of course fails.
Step 7 works

Expected result:
Step 5 matches Step 7.

I am not totally sure these exactl steps reproduct it, I am just guessing it from what I see here, it's hard to test because Kerberos is so complex, but I think that this will reproduce the problem. For the same reason (hard to diagnose), the bug is bad for users and admins using Kerberos.

Comment 1

[David :Bienvenu]

This code might need to be using server->GetRealHostName(realHostName) instead of GetImapHostName().

    nsCAutoString service("imap@");
    service.Append(GetImapHostName());
    gssrv = DoGSSAPIStep1(service.get(), userName, response);
    NS_ENSURE_SUCCESS(gssrv, gssrv);

Comment 2

[Ben Bucksch (:BenB)]

Yup, I guessed that was the problem. Thanks for the specific pointer.

I ran into the same issue (just with realUsername) with the ACL extension I wrote. Maybe we should change the API to let "username" and "HostName" return the new username and hostname, and leave the old username/hostname only for the mail directory pathname (which is already a pref on its own)?

Comment 3

[Ben Bucksch (:BenB)]

Attachment: Fix, v1

This does what bienvenu said above.

I could verify using printf() that indeed GetImapHostName() returns the old and wrong hostname, while m_server->GetRealHostName() returns the correct new one.

I also verified using testing that this fixes the problem.

Comment 4

[David :Bienvenu]

Comment on attachment 413830 [details] [diff] [review]
Fix, v1

I should have pointed out before - you can't do this on the UI thread because you will be accessing prefs off the ui thread. My first inclination would be to make GetImapHostName use the real host name, but I think that won't work. You could make SetupWithUrl (which runs on the UI thread) get the real host name and store it in the protocol object, and access it here.

Comment 5

[Nikolay Shopik]

Does this affect IMAP only? Seen it when using IMAP but I never used pop3 with Kerberos.

Comment 6

[David :Bienvenu]

pop3 uses the realhostname correctly.

Comment 8

[Nikolay Shopik]

I really would like to see this fix on 3.0 tree. Because currently new wizard always detect wrong server name for me and manually changing it doesn't do anything at all.

Comment 9

[Ben Bucksch (:BenB)]

Workaround: Remove TB account, Re-create. For IMAP, that's fairly easy.
Filters: If you save ImapMail/<oldservername>/msgFilterRules.dat and put it in the new account directory, you don't have to redo your filters either.

Comment 10

[Ben Bucksch (:BenB)]

Note: I probably won't be able to work on this until Jan 12.

Comment 12

[Mark Banner (:standard8)]

How severe is this issue? This doesn't seem to fit the policy of affecting a lot of users, although admittedly it is obviously not good for those users that use GSSAPI.

Comment 13

[David :Bienvenu]

There are ways to work around the issue. I don't know how common it is, but once it happens, it can be confusing for the user. The fix should be fairly trivial.  You could make SetupWithUrl (which runs on the UI thread) get the real host name
and store it in the protocol object, and use that.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Blocking because this has the potential to hurt a whole bunch of users at once, particularly in environments where the autoconfig wizard guesses wrong.  For what it's worth, if this were the last bug standing, I'd hold the release a day to get it, but I'm not sure I'd hold it a week.

Comment 15

[David :Bienvenu]

Ben, if you're not going to get to this in the next week or so, please let me know and I'll finish it off (though I'd ask you for a review if I write the patch :-) )

Comment 16

[Ben Bucksch (:BenB)]

Yeah, go ahead, if you want, and I'll be happy to review, if I may.

Comment 17

[David :Bienvenu]

Attachment: proposed fix

Looks like you included the prev patch in your mondo auth patch, and, shame on me, I missed it. So we're accessing prefs from the IMAP thread, which can cause issues. This rolls that back, and accesses the real host name pref on the ui thread...

We don't have GSSAPI auth tests, for understandable reasons, but this passes all the other auth tests...

Comment 18

[Ben Bucksch (:BenB)]

> Looks like you included the prev patch in your mondo auth patch

Oops? Sorry, that wasn't intentional.

From a quick glance, the patch looks OK to me. I'll give it a closer look and will try to test it later.

Comment 19

[Ben Bucksch (:BenB)]

Comment on attachment 442403 [details] [diff] [review]
proposed fix

Haven't tested yet, but code looks good to me. r=BenB

Thanks for the patch! :)

Comment 20

[[e-mail]]

Comment on attachment 442403 [details] [diff] [review]
proposed fix

>-        rv = NS_ExamineForProxy("imap", hostName.get(), port, getter_AddRefs(proxyInfo));
>+        rv = NS_ExamineForProxy("imap", m_realHostName.get(), port, getter_AddRefs(proxyInfo));
Please switch this to (external API safe) MsgExamineForProxy while you're here.

Comment 21

[David :Bienvenu]

fixed on trunk and 1.9.2 branch.

Comment 22

[Nikolay Shopik]

much appreciated, works well
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.5pre) Gecko/20100511 Lanikai/3.1pre

Comment 23

[Mark Banner (:standard8)]

If we want this on the 1.9.1 branch, then we'll need a version of the patch for that branch.

Comment 24

[Mark Banner (:standard8)]

We never got a branch patch for this and 3.1 has been out for a while with the fix so that is the workaround/upgrade path to get the fix.