After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 615239 - imapx doesn't work with gssapi
imapx doesn't work with gssapi
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
2.30.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
evolution[imapx]
: 615196 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-04-09 00:32 UTC by Brian J. Murrell
Modified: 2010-06-14 11:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
just an incomplete try (2.55 KB, text/plain)
2010-05-04 18:04 UTC, Milan Crha
  Details
updated patch (against 2.30 branch) (3.60 KB, patch)
2010-06-11 16:00 UTC, David Woodhouse
committed Details | Review
for HEAD (3.40 KB, patch)
2010-06-11 16:44 UTC, David Woodhouse
none Details | Review
updated patch for master branch (3.66 KB, patch)
2010-06-12 23:28 UTC, David Woodhouse
committed Details | Review

Description Brian J. Murrell 2010-04-09 00:32:24 UTC
I have converted one of my IMAP accounts to use imapx to get some of the new features of it.  Right away it failed to open the account because I selected GSSAPI authentication.  Basically it seems to get stuck into a loop of opening a connection to the IMAP server, asking the capabilities and then evolution[-data-server] closes the connection and re-opens it.  My IMAP server responds to the capabilities request with:

CAPABILITY IMAP4 IMAP4rev1 ACL QUOTA LITERAL+ NAMESPACE UIDPLUS ID NO_ATOMIC_RENAME UNSELECT CHILDREN MULTIAPPEND BINARY SORT THREAD=ORDEREDSUBJECT THREAD=REFERENCES ANNOTATEMORE IDLE STARTTLS AUTH=GSSAPI SASL-IR

So something in there is upsetting imapx.
Comment 1 Akhil Laddha 2010-04-09 04:00:13 UTC
*** Bug 615196 has been marked as a duplicate of this bug. ***
Comment 2 Bojan Smojver 2010-04-30 01:37:04 UTC
Same here.
Comment 3 Milan Crha 2010-05-04 18:04:31 UTC
Created attachment 160280 [details]
just an incomplete try

This is something I believe should be there in the imapx code. The problem is it is not complete, and I don't get what is missing. What I found:
- There was a bug, the camel_sasl_new was called with service NULL.
- Also the question for the password should be done when not requested by the
  authentication method.
- The "AUTHENTICATE GSSAPI" returns three lines for me:
  + \n
  + some base64 code
  + another base64 code
  The problem seems to be that the camel_imapx_stream_token stops on the first
  "+ \n" and doesn't want to move forward. Neither when I call it again. If I
  try to read from the stream by camel_stream_read, then it gets some data,
  thus the main problem seems to be that during authentication is not read
  whole response from the server. Or something like that, I honestly do not
  understand what's going wrong here. I'll ask Chen to take a look, I
  believe it's something simple what I'm missing here.
- The last chunk in imapx_reconnect comments a free command, because it is
  freed twice, second time later in the same function in the disconnect call
Comment 4 Chenthill P 2010-05-20 11:39:41 UTC
I would need a test account to have a go at this. 

Akhil, would be great if we can get this setup when ur back..
We would also need to enable kerberos for exchange..
Comment 5 David Woodhouse 2010-06-11 09:03:17 UTC
Comparing your patch in comment 3 with my (working) hacks, it looks like you're just missing this bit (which I'll paste rather than attaching since it isn't really ready to be applied as-is anyway):

@@ -1490,10 +1490,14 @@ imapx_continuation(CamelIMAPXServer *ima
 	case CAMEL_IMAPX_COMMAND_AUTH: {
 		gchar *resp;
 		guchar *token;
-		gint tok;
 		guint len;
 
-		tok = camel_imapx_stream_token(imap->stream, &token, &len, ex);
+		camel_imapx_stream_gets (imap->stream, &token, &len);
+		token[len] = 0;
+		while (*token == ' ')
+			token++;
+		
+		printf("token is '%s'\n", token);
 		resp = camel_sasl_challenge_base64((CamelSasl *)cp->ob, (const gchar *) token, ex);
 		if (camel_exception_is_set(ex))
 			return -1;
@@ -1505,7 +1509,7 @@ imapx_continuation(CamelIMAPXServer *ima
 		/* we want to keep getting called until we get a status reponse from the server
 		   ignore what sasl tells us */
 		newliteral = ic;
-
+		goto noskip;
 		break; }
 	case CAMEL_IMAPX_COMMAND_FILE: {
 		CamelStream *file;
@@ -1531,7 +1535,7 @@ imapx_continuation(CamelIMAPXServer *ima
 	}
 
 	camel_imapx_stream_skip(imap->stream, ex);
-
+ noskip:
 	cp = cp->next;
 	if (cp->next) {
 		ic->current = cp;
Comment 6 Brian J. Murrell 2010-06-11 11:51:46 UTC
David,

If you have working gssapi for imapx, even if you only consider it "hacks" how about posting them here?  At least then perhaps there is a starting point for somebody to refine them and hopefully the result is this task might get some attention because it becomes a lot less work to just refine something that does the job rather than having to start from scratch.
Comment 7 David Woodhouse 2010-06-11 14:59:06 UTC
(In reply to comment #6)
> If you have working gssapi for imapx, even if you only consider it "hacks" how
> about posting them here? 

http://david.woodhou.se/imapx-krb5.patch -- but mcrha's patch is much better than mine in every respect (he made it not ask for your password when you're using an authmethod that doesn't require it, etc.). Only the bit I added in comment 5 should be required, on top of mcrha's patch. I'm about to combine them and do some testing... and fix the SEGV that happens when you don't have a Kerberos ticket and the SASL routine returns NULL. Feeding that into camel_imapx_stream_write() is kind of unhelpful.
Comment 8 David Woodhouse 2010-06-11 16:00:42 UTC
Created attachment 163397 [details] [review]
updated patch (against 2.30 branch)

New patch based on mcrha's original. The auth continuation handling now looks like this:
-		tok = camel_imapx_stream_token(imap->stream, &token, &len, ex);
+		camel_imapx_stream_text (imap->stream, &token, ex);
+		if (camel_exception_is_set(ex))
+			return -1;
+		    
 		resp = camel_sasl_challenge_base64((CamelSasl *)cp->ob, (const gchar *) token, ex);
+		g_free(token);

I took a different approach to the double-free SEGV which mcrha fixed too; I think just removing the original free() was going to cause a memory leak in the case where the failing command _wasn't_ is->literal. Instead, I have added a check in imapx_command_run() to clear is->literal if it is the current command, after the loop.

This patch also fixes a SEGV in unrelated code in a debug message.
Comment 9 David Woodhouse 2010-06-11 16:44:14 UTC
Created attachment 163399 [details] [review]
for HEAD
Comment 10 David Woodhouse 2010-06-12 23:28:39 UTC
Created attachment 163491 [details] [review]
updated patch for master branch

Updated to check for NULL return from camel_sasl_challenge_base64(), which seems to happen when my Kerberos ticket is expired. Not sure why it doesn't raise a proper exception for that, but if even it _does_ get fixed on the SASL side, the imapx code should be robust and not crash.
Comment 11 Milan Crha 2010-06-14 11:07:50 UTC
I tested the last patch and it works for me, I can connect to imapx server with gssapi authentication.
Comment 12 David Woodhouse 2010-06-14 11:34:39 UTC
To ssh://dwmw2@git.gnome.org/git/evolution-data-server
   013d2bb..6c3e2e2  gnome-2-30 -> gnome-2-30
   14b5de1..35f61b3  master -> master

Thanks.

On the 2.30 branch, I left out the extra-paranoid check for NULL return from camel_sasl_challenge_base64(). That should never happen now that that bug is fixed, and it was introducing a new untranslated string.
Comment 13 Milan Crha 2010-06-14 11:59:06 UTC
Closing per comment #12 and setting proper patches status