[Patch] IMAP Code Simplifications

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Patch] IMAP Code Simplifications

Albrecht Dreß
Hi all,

attached is a patch which tries to eliminate some duplicated code from libimap:
- replace the custom implementation for base64 encoding and decoding by the respective glib functions
- use libnetclient to calculate the CRAM-MD5 authentication message.

I verified the changed implementation against a local dovecot instance, for both "cram-md5" and "plain" authentication.

Opinions?

Cheers,
Albrecht.

---
Patch details:
- Makefile.am: ensure libnetclient is built before libbalsa, so the imap test code can link properly
- libbalsa/imap/Makefile.am: add libnetclient to include path, link test app against it
- imap/auth-cram.c: use net_client_cram_calc(), and wipe sensitive data before free
- libbalsa/imap/imap-auth.c: g_base64_encode() instead of custom function
- libbalsa/imap/util.[hc]: remove custom base64 encode and decode stuff
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

libimap-cleanups.diff (8K) Download Attachment
attachment1 (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Peter Bloomfield
Hi Albrecht,

On Apr 10, 2017, at  1:21 PM, Albrecht Dreß wrote:

> Hi all,
>
> attached is a patch which tries to eliminate some duplicated code from  
> libimap:
> - replace the custom implementation for base64 encoding and decoding by  
> the respective glib functions
> - use libnetclient to calculate the CRAM-MD5 authentication message.
>
> I verified the changed implementation against a local dovecot instance,  
> for both "cram-md5" and "plain" authentication.
>
> Opinions?
Thanks! Testing it right now, no hiccups yet…

But it fails to build --with-gss.

Best,

Peter
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (188 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Albrecht Dreß
Hi Peter:

Am 11.04.17 19:01 schrieb(en) Peter Bloomfield:
> But it fails to build --with-gss.

Ooooops - I forgot to enable this option, and to grep through the sources...  Thanks a lot for testing, and for pointing me to it!

The attached patch should fix the issue.  It is somewhat inelegant as the newly allocated GLib results are copied to the fixed buffers, but I didn't want to refactor the whole code.  Unfortunately, I have no chance at all to test it, but as the changes are limited, they look logical to me.

Anyone out there using an imap server w/ gssapi authentication, who could verify the changes?

Cheers,
Albrecht.
diff --git a/libbalsa/imap/auth-gssapi.c b/libbalsa/imap/auth-gssapi.c
index 5a19707..7da29b3 100644
--- a/libbalsa/imap/auth-gssapi.c
+++ b/libbalsa/imap/auth-gssapi.c
@@ -218,6 +218,7 @@ ag_get_token(gss_ctx_id_t *context, gss_name_t target, gss_buffer_t sec_token,
     OM_uint32 state, min_stat;
     gss_buffer_desc send_token;
     unsigned cflags;
+    gchar *b64buf;
     
     *client_token = '\0';
     state = gss_init_sec_context
@@ -229,8 +230,9 @@ ag_get_token(gss_ctx_id_t *context, gss_name_t target, gss_buffer_t sec_token,
     if (state != GSS_S_COMPLETE && state != GSS_S_CONTINUE_NEEDED)
         return state;
 
-    lit_conv_to_base64(client_token, send_token.value, send_token.length,
-                       token_sz);
+    b64buf = g_base64_encode(send_token.value, send_token.length);
+    strncpy(client_token, b64buf, (size_t) token_sz);
+    g_free(b64buf);
     gss_release_buffer(&min_stat, &send_token);
     return state;
 }
@@ -240,8 +242,12 @@ ag_parse_request(ImapMboxHandle *handle, char *buf, ssize_t buf_sz,
                  gss_buffer_desc *request)
 {
     char line[LONG_STRING];
+    guchar *rawbuf;
+
     sio_gets(handle->sio, line, LONG_STRING); /* FIXME: error checking */
-    request->length = lit_conv_from_base64(buf, line);
+    rawbuf = g_base64_decode(line, &request->length);
+    memcpy(buf, rawbuf, request->length);
+    g_free(rawbuf);
     request->value = buf;
 }
 
@@ -261,6 +267,7 @@ ag_negotiate_parameters(ImapMboxHandle *handle, const char * user,
     char server_conf_flags;
     unsigned char *t;
     unsigned long buf_size;
+    gchar *b64buf;
 
     ag_parse_request(handle, buf, sizeof(buf), &request_buf);
     state = gss_unwrap(&min_stat, context, &request_buf, &send_token,
@@ -307,9 +314,9 @@ ag_negotiate_parameters(ImapMboxHandle *handle, const char * user,
         return FALSE;
     }
     
-    lit_conv_to_base64(buf, send_token.value, send_token.length,
-                       sizeof(buf));
-    sio_printf(handle->sio, "%s\r\n", buf); imap_handle_flush(handle);
+    b64buf = g_base64_encode(send_token.value, send_token.length);
+    sio_printf(handle->sio, "%s\r\n", b64buf); imap_handle_flush(handle);
+    g_free(b64buf);
     
     WAIT_FOR_PROMPT(*rc,handle,cmdno,buf,sizeof(buf));
     if (*rc == IMR_RESPOND)

_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Peter Bloomfield
Hi Albrecht:

On 04/11/2017 04:05:52 PM Tue, Albrecht Dreß wrote:
> Hi Peter:
>
> Am 11.04.17 19:01 schrieb(en) Peter Bloomfield:
>> But it fails to build --with-gss.
>
> Ooooops - I forgot to enable this option, and to grep through the sources..  Thanks a lot for testing, and for pointing me to it!
>
> The attached patch should fix the issue.  It is somewhat inelegant as the newly allocated GLib results are copied to the fixed buffers, but I didn't want to refactor the whole code.  Unfortunately, I have no chance at all to test it, but as the changes are limited, they look logical to me.

Thanks--I've pushed both patches as a single commit to master.

> Anyone out there using an imap server w/ gssapi authentication, who could verify the changes?

Yes, that would be great--I can't exercise that code either!

Peter
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Jack
On 2017.04.11 17:59, Peter Bloomfield wrote:

> Hi Albrecht:
>
> On 04/11/2017 04:05:52 PM Tue, Albrecht Dreß wrote:
>> Hi Peter:
>>
>> Am 11.04.17 19:01 schrieb(en) Peter Bloomfield:
>>> But it fails to build --with-gss.
>>
>> Ooooops - I forgot to enable this option, and to grep through the  
>> sources.  Thanks a lot for testing, and for pointing me to it!
>>
>> The attached patch should fix the issue.  It is somewhat inelegant  
>> as the newly allocated GLib results are copied to the fixed buffers,  
>> but I didn't want to refactor the whole code.  Unfortunately, I have  
>> no chance at all to test it, but as the changes are limited, they  
>> look logical to me.
>
> Thanks--I've pushed both patches as a single commit to master.
>
>> Anyone out there using an imap server w/ gssapi authentication, who  
>> could verify the changes?
>
> Yes, that would be great--I can't exercise that code either!

It might be useful to accumulate a list of major email providers and  
which features they use.  I'll be glad to test, but I have gmail and  
yahoo accounts, and have no idea whether they use gssapi.

Jack
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Peter Bloomfield
In reply to this post by Peter Bloomfield
Hi Albrecht,

On 04/11/2017 05:59:55 PM Tue, Peter Bloomfield wrote:

> Hi Albrecht:
>
> On 04/11/2017 04:05:52 PM Tue, Albrecht Dreß wrote:
>> Hi Peter:
>>
>> Am 11.04.17 19:01 schrieb(en) Peter Bloomfield:
>>> But it fails to build --with-gss.
>>
>> Ooooops - I forgot to enable this option, and to grep through the sources.  Thanks a lot for testing, and for pointing me to it!
>>
>> The attached patch should fix the issue.  It is somewhat inelegant as the newly allocated GLib results are copied to the fixed buffers, but I didn't want to refactor the whole code.  Unfortunately, I have no chance at all to test it, but as the changes are limited, they look logical to me.
Attached is a variant that uses the existing buffers without major refactoriing. I believe it does essentially the same thing. What do you think?

Best,

Peter
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

b64-use-buffers.diff (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Anatole Denis
In reply to this post by Peter Bloomfield
On mar., avril 11, 2017 at 11:59 , Peter Bloomfield
<[hidden email]> wrote:
>
>> Anyone out there using an imap server w/ gssapi authentication, who
>> could verify the changes?
>
> Yes, that would be great--I can't exercise that code either!
>
> Peter

Hi,

I happen to have an email server with GSSAPI auth (which I had never
tried with balsa before). I'm happy to report that with balsa
recompiled with gss from current master I am succesfully able to
authenticate with GSS over IMAP.

Maybe this isn't relevant here, but while it did start automatically
authenticating with method=GSSAPI over IMAP without any configuration
as soon as I had a ticket for the kerberos domain, I can't figure out
how to configure it to use GSS for SMTP.

By the way, since this is my first post to the list: thanks for Balsa !

--
Anatole

_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Peter Bloomfield
Hi Anatole,

Welcome to the list!

On 04/11/2017 09:56:04 PM Tue, Anatole Denis wrote:

> On mar., avril 11, 2017 at 11:59 , Peter Bloomfield <[hidden email]> wrote:
>>
>>> Anyone out there using an imap server w/ gssapi authentication, who could verify the changes?
>>
>> Yes, that would be great--I can't exercise that code either!
>>
>> Peter
>
> Hi,
>
> I happen to have an email server with GSSAPI auth (which I had never tried with balsa before). I'm happy to report that with balsa recompiled with gss from current master I am succesfully able to authenticate with GSS over IMAP.

Good news--thanks for testing!

> Maybe this isn't relevant here, but while it did start automatically authenticating with method=GSSAPI over IMAP without any configuration as soon as I had a ticket for the kerberos domain, I can't figure out how to configure it to use GSS for SMTP.

GSSAPI authentication has not been implemented for SMTP connections. But Albrecht has brought SMTP support into Balsa's code tree, so now it may be possible to add GSS to the list of supported auths (PLAIN, LOGIN, CRAM-MD5, CRAM-SHA1).

> By the way, since this is my first post to the list: thanks for Balsa !

You're welcome!

Peter
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Albrecht Dreß
In reply to this post by Peter Bloomfield
Hi Peter:

Am 11.04.17 23:59 schrieb(en) Peter Bloomfield:
> Thanks--I've pushed both patches as a single commit to master.

Cool, thanks a lot!

Cheers, Albrecht.
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Albrecht Dreß
In reply to this post by Peter Bloomfield
Hi Peter:

Am 12.04.17 03:43 schrieb(en) Peter Bloomfield:
>>> The attached patch should fix the issue.  It is somewhat inelegant as the newly allocated GLib results are copied to the fixed buffers, but I didn't want to refactor the whole code.  Unfortunately, I have no chance at all to test it, but as the changes are limited, they look logical to me.
>
> Attached is a variant that uses the existing buffers without major refactoriing. I believe it does essentially the same thing. What do you think?

I agree that your solution looks correct, and as it saves the memory allocation, I think it should be preferred!

Cheers,
Albrecht.
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Albrecht Dreß
In reply to this post by Anatole Denis
Hi Anatole:

Am 12.04.17 03:56 schrieb(en) Anatole Denis:
> I happen to have an email server with GSSAPI auth (which I had never tried with balsa before). I'm happy to report that with balsa recompiled with gss from current master I am succesfully able to authenticate with GSS over IMAP.

Great, thanks a lot for testing!

> Maybe this isn't relevant here, but while it did start automatically authenticating with method=GSSAPI over IMAP without any configuration as soon as I had a ticket for the kerberos domain, I can't figure out how to configure it to use GSS for SMTP.

Currently, the authentication methods are limited to LOGIN, PLAIN and CRAM-(MD5|SHA1) for SMTP (the same for POP3, plus APOP and USER).  I shifted the code to a library within Balsa recently, so it would be relatively easy to add GSS/Kerberos to both SMTP and POP3.

Unfortunately, all providers I have access to support LOGIN and PLAIN only (which is safe as they enforce encryption).  I made the tests with the other methods using INetSim (<http://www.inetsim.org/>).  Do you know a provider supporting GSS/Kerberos?  Which one?  Setting up Kerberos is a real PITA, so I would prefer to avoid it if possible, or maybe I find a pre-configured Docker container...

> By the way, since this is my first post to the list: thanks for Balsa !

Thanks; as always, any suggestion for improvements is highly welcome!

Cheers,
Albrecht.
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Peter Bloomfield
In reply to this post by Albrecht Dreß
Hi Albrecht:

On 04/12/2017 01:08:02 PM Wed, Albrecht Dreß wrote:
> Hi Peter:
>
> Am 12.04.17 03:43 schrieb(en) Peter Bloomfield:
>>>> The attached patch should fix the issue.  It is somewhat inelegant as the newly allocated GLib results are copied to the fixed buffers, but I didn't want to refactor the whole code.  Unfortunately, I have no chance at all to test it, but as the changes are limited, they look logical to me.
>>
>> Attached is a variant that uses the existing buffers without major refactoriing. I believe it does essentially the same thing. What do you think?
>
> I agree that your solution looks correct, and as it saves the memory allocation, I think it should be preferred!

OK--pushed to master. I also pushed the vfs fix for retaining ownership of the GFile when creating a GMimeStreamGIO.

Best,

Peter
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] IMAP Code Simplifications

Albrecht Dreß
In reply to this post by Anatole Denis
Hi Anatole:

Am 12.04.17 03:56 schrieb(en) Anatole Denis:
> Maybe this isn't relevant here, but while it did start automatically authenticating with method=GSSAPI over IMAP without any configuration as soon as I had a ticket for the kerberos domain, I can't figure out how to configure it to use GSS for SMTP.

I submitted a patch for SMTP and POP3 supporting GSSAPI authentication.  Peter already pushed it to git.

Unfortunately, I could only verify it against a rather simple test environment in a VM (Samba4 acting as AD-DC, plus Postfix and Dovecot).  If you have access to a "real-life" server supporting GSSAPI (which one?), it would be *really* helpful if you could check if it works for you.  You don't have to configure anything to use it, btw, as GSSAPI will be the first choice for authentication if it is supported.

Thanks in advance,
cheers,
Albrecht.
_______________________________________________
balsa-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/balsa-list

attachment0 (484 bytes) Download Attachment
Loading...