fetchmail-devel

[fetchmail-devel] fetchmail and mapi II

[mojmir s. <moj...@2k...>]

good morning,

i'd like to supply more details for my previous mail:

3. missing mapi_session* argument
	affected functions are mapi_init, smtp_address
	(calls OpenMsgStore, GetGALTable)
	i had to introduce static global variable for that.

4. buffer overflows
	affected functions get_base64_attachment, octool_get_stream
	perhaps all these memcpys should be reviewed

6. \r characters and = spread all around in mail body
	caused by quoted_printable_encode, some weird stuff there.
	i fixed it to suite my needs, proper solution needed.

best regards,
mojmir

Re: [fetchmail-devel] fetchmail and mapi II

[Matthias A. <mat...@gm...>]

Am 10.06.2009, 08:23 Uhr, schrieb mojmir svoboda  
<moj...@2k...>:

> good morning,
>
> i'd like to supply more details for my previous mail:
>
> 3. missing mapi_session* argument
> 	affected functions are mapi_init, smtp_address
> 	(calls OpenMsgStore, GetGALTable)
> 	i had to introduce static global variable for that.
>
> 4. buffer overflows
> 	affected functions get_base64_attachment, octool_get_stream
> 	perhaps all these memcpys should be reviewed
>
> 6. \r characters and = spread all around in mail body
> 	caused by quoted_printable_encode, some weird stuff there.
> 	i fixed it to suite my needs, proper solution needed.

Dear Mojmir,

thanks for the details and taking interest here.

What is this quoted_printable_encode part doing wrong? Could you provide  
your solution to (6)? Perhaps we can build a full solution on top of yours.

Thanks.

Best regards

-- 
Matthias Andree

Re: [fetchmail-devel] fetchmail and mapi II

[mojmir s. <moj...@2k...>]

hello everyone,

first i'd like to note that i am not a mapi specialist.. or anything close to
that. i simply have to live with this exchange and not want to use outlook.


> > 3. missing mapi_session* argument
> > 	affected functions are mapi_init, smtp_address
> > 	(calls OpenMsgStore, GetGALTable)
> > 	i had to introduce static global variable for that.

you can find a patch in the attachment. this way it at least compiles and runs.

the things is: if i use s_mapi_session everywhere it stops to work properly in
mapi_getauth (MapiLogonProvider creates another session and if the
s_mapi_session passed as arg, it crashes later in ProcessNetworkProfile)

so the s_mapi_session is shared only between mapi_init and smptp_address for a
moment.

> > 4. buffer overflows
> > 	affected functions get_base64_attachment, octool_get_stream
> > 	perhaps all these memcpys should be reviewed

i will toy with this few days until i figure what the correct form should be. 

as a hotfix i simply allocated lot of memory and removed the loop that did
nothing. i assume you do not want that kind of solution (and neither do i..
in long term)

> > 6. \r characters and = spread all around in mail body
> What is this quoted_printable_encode part doing wrong? Could you provide
> your solution to (6)? Perhaps we can build a full solution on top of yours.

errr.. do not overestimate me... i simply removed the code that did not
suited me ;)

	for example: what is this supposed to do? 8->7 bit?
			if (is_safe_char(ch))
				line[line_count++] = ch;
			else {  
				line[line_count++] = '=';
				line[line_count++] = hex[(ch >> 4) & 15];
				line[line_count++] = hex[ch & 15];
			}
	if yes i wonder why mutt (set noallow_8bit) displays that incorrectly.

	and then there are these line[line_count++] = '\r';
	which are later visible in mutt.

	sincerely, i have no idea what proper solution should look like. 

there are more problems:
	- fetchmail modifies the "From: " header
	- fetchmail seems to drop any list-id and in-reply-to fields (and perhaps
	  other fields). this makes mailing lists (and threading) a little bit
	  uncomfortable

best regards and thanks for your replies,
mojmir

Re: [fetchmail-devel] fetchmail and mapi II

[Matthias A. <mat...@gm...>]

Am 16.06.2009, 12:27 Uhr, schrieb mojmir svoboda  
<moj...@2k...>:

> first i'd like to note that i am not a mapi specialist.. or anything  
> close to that. i simply have to live with this exchange and not want to  
> use outlook.

No need to justify your goals - your input and observations are useful and  
welcome, even if you cannot provide full solutions to issues that you see.

>> > 3. missing mapi_session* argument
>> > 	affected functions are mapi_init, smtp_address
>> > 	(calls OpenMsgStore, GetGALTable)
>> > 	i had to introduce static global variable for that.
>
> you can find a patch in the attachment. this way it at least compiles  
> and runs.
>
> the things is: if i use s_mapi_session everywhere it stops to work  
> properly in mapi_getauth (MapiLogonProvider creates another session and  
> if the
> s_mapi_session passed as arg, it crashes later in ProcessNetworkProfile)
>
> so the s_mapi_session is shared only between mapi_init and smptp_address  
> for a moment.

I'll save this part and look at it later; perhaps Jelmer can help here.

>> > 4. buffer overflows
>> > 	affected functions get_base64_attachment, octool_get_stream
>> > 	perhaps all these memcpys should be reviewed
>
> i will toy with this few days until i figure what the correct form  
> should be.
>
> as a hotfix i simply allocated lot of memory and removed the loop that  
> did nothing. i assume you do not want that kind of solution (and neither  
> do i.. in long term)

Ah, ok.

>> > 6. \r characters and = spread all around in mail body
>> What is this quoted_printable_encode part doing wrong? Could you provide
>> your solution to (6)? Perhaps we can build a full solution on top of  
>> yours.
>
> errr.. do not overestimate me... i simply removed the code that did not
> suited me ;)
>
> 	for example: what is this supposed to do? 8->7 bit?
> 			if (is_safe_char(ch))
> 				line[line_count++] = ch;
> 			else {
> 				line[line_count++] = '=';
> 				line[line_count++] = hex[(ch >> 4) & 15];
> 				line[line_count++] = hex[ch & 15];
> 			}

This looks like quoted printable encoding to me. hex is likely (i. e. I  
haven't looked at the code) a char array "0123456789ABCDEF" and if it's a  
safe character (no control character), it's emitted directly, otherwise,  
it's encoded as =XX, where XX is the associated ASCII value, expressed as  
a sedecimal number.

> 	if yes i wonder why mutt (set noallow_8bit) displays that incorrectly.

Perhaps headers do not properly declare content. Do you have an example of  
such a message (inside mutt, just pipe it into "cat - >/tmp/msg.bin", then  
mail msg.bin as application/octet-stream attachment).

> 	and then there are these line[line_count++] = '\r';
> 	which are later visible in mutt.

If that's correct depends on the location in the code where you've found  
this.

> 	sincerely, i have no idea what proper solution should look like.
>
> there are more problems:
> 	- fetchmail modifies the "From: " header
> 	- fetchmail seems to drop any list-id and in-reply-to fields (and  
> perhaps
> 	  other fields). this makes mailing lists (and threading) a little bit
> 	  uncomfortable

Is it fetchmail or OpenChange or Exchange that does this?

I recall an internship some 8 years ago where the company also Exchange  
(that was in early 2001), and Emacs+Gnus was rather confused by the output  
(which also included blank lines in places where you didn't expect them,  
and where PINE didn't show them), while PINE worked reasonably well. I  
don't recall how well mutt fared at the time.

HTH
Matthias

-- 
Matthias Andree

Re: [fetchmail-devel] fetchmail and mapi II

[mojmir s. <moj...@2k...>]

* Mat...@2k...rp <Mat...@2k...rp> [2009-06-16 14:11:37 +0000]:

> > as a hotfix i simply allocated lot of memory and removed the loop that
> > did nothing. i assume you do not want that kind of solution (and neither
> > do i.. in long term)
> 
> Ah, ok.

in attachment there is a patch fixing this (hopefuly) correctly.  still,
the postscript attachment i sent myself got little bit scrambled
(periodically at buffer boundaries). may be bug in stream, may be bug in
ldb_base64_encode.

btw: ldb_base64_encode can be removed from the source file mapi.c, as it
is already present in samba4.

> >> > 6. \r characters and = spread all around in mail body
> This looks like quoted printable encoding to me. hex is likely (i. e. I

i confirm. this seems to be hardcoded in mapi.c
perhaps there should be an option specifying output format? i.e. if
exchange sends it in 7bit, i'm fine with it. no need for another
translation.

> Perhaps headers do not properly declare content. Do you have an example of
> such a message (inside mutt, just pipe it into "cat - >/tmp/msg.bin", then
> mail msg.bin as application/octet-stream attachment).

in attachment. it's in czech, sorry - only sample i did not deleted

> > 	- fetchmail modifies the "From: " header
> Is it fetchmail or OpenChange or Exchange that does this?

this is what i have when i look out from outlook:
	From: "Matthias Andree" <matthias.andree@your.email>
	To: "mojmir svoboda" <mojmir.svoboda@my.email>, fet...@li...
	Content-Type: text/plain; format=flowed; delsp=yes; charset=iso-8859-15
	Content-Transfer-Encoding: 7bit

this is what i receive after fetchmailing via mapi:
	From: MatthiasAndree@exchange_server_address.here
	To: 2K Czech <MojmirSvoboda>;, fet...@li...
	Content-Type: text/plain; charset=us-ascii
	Content-Transfer-Encoding: quoted-printable

there is some header manipulation code in mapi_fetch_headers, i'd start
search there.

best regards,
mojmir

Re: [fetchmail-devel] fetchmail and mapi II

[Matthias A. <mat...@gm...>]

Am 16.06.2009, 17:55 Uhr, schrieb mojmir svoboda  
<moj...@2k...>:

> * Mat...@2k...rp  
> <Mat...@2k...rp> [2009-06-16 14:11:37 +0000]:
>
>> > as a hotfix i simply allocated lot of memory and removed the loop that
>> > did nothing. i assume you do not want that kind of solution (and  
>> neither
>> > do i.. in long term)
>>
>> Ah, ok.
>
> in attachment there is a patch fixing this (hopefuly) correctly.  still,
> the postscript attachment i sent myself got little bit scrambled
> (periodically at buffer boundaries). may be bug in stream, may be bug in
> ldb_base64_encode.
>
> btw: ldb_base64_encode can be removed from the source file mapi.c, as it
> is already present in samba4.
>
>> >> > 6. \r characters and = spread all around in mail body
>> This looks like quoted printable encoding to me. hex is likely (i. e. I
>
> i confirm. this seems to be hardcoded in mapi.c
> perhaps there should be an option specifying output format? i.e. if
> exchange sends it in 7bit, i'm fine with it. no need for another
> translation.
>
>> Perhaps headers do not properly declare content. Do you have an example  
>> of
>> such a message (inside mutt, just pipe it into "cat - >/tmp/msg.bin",  
>> then
>> mail msg.bin as application/octet-stream attachment).
>
> in attachment. it's in czech, sorry - only sample i did not deleted
>
>> > 	- fetchmail modifies the "From: " header
>> Is it fetchmail or OpenChange or Exchange that does this?
>
> this is what i have when i look out from outlook:
> 	From: "Matthias Andree" <matthias.andree@your.email>
> 	To: "mojmir svoboda" <mojmir.svoboda@my.email>,  
> fet...@li...
> 	Content-Type: text/plain; format=flowed; delsp=yes; charset=iso-8859-15
> 	Content-Transfer-Encoding: 7bit
>
> this is what i receive after fetchmailing via mapi:
> 	From: MatthiasAndree@exchange_server_address.here
> 	To: 2K Czech <MojmirSvoboda>;, fet...@li...
> 	Content-Type: text/plain; charset=us-ascii
> 	Content-Transfer-Encoding: quoted-printable
>
> there is some header manipulation code in mapi_fetch_headers, i'd start
> search there.
>
> best regards,
> mojmir

Hi Mojmir,

I have updated fetchmail's BRANCH_MAPI branch to release 6.3.10 including  
the post-release updates to Spanish and Chinese translations as of  
yesterday.

I have merged your mapi2.patch (thanks), but haven't yet had the time to  
look at the messages or other problems we discussed.

I fixed an issue in rcfile_y.y where the LCID parsing for strings that  
didn't start with 0x was utterly broken.

I fixed configure.ac logic so that --disable-MAPI (or not specifying this)  
works, and we only check for libmagic if needed.

So me new issues I discovered that also need addressing:

- all issues you've mentioned that haven't been addressed by your patch,  
for instance, message corruption as observed, or duplication of base64  
encoders.

- there are warnings about broken 4th arguments to one of the openchange  
functions

- mapi.c throws a zillion of warnings of mismatched signedness in  
comparisons, or unused variables. All have to be audited that they don't  
trigger integer overflows or negative indexes.

- the MAPI code adds quite a few if (blah.protocol != P_MAPI) ... #ifdef  
MAPI_ENABLE ... #endif that should be abstracted away rather than copy &  
paste code...

- the bug I fixed in rcfile_y.y was sort of target = sprintf("0x%04x",  
uint32_t); - oops. sprintf takes the output variable as argument and  
returns the length... I hope there isn't more of this bug.

- more will show up as fixup and integration proceed.


For better or for worse, the current BRANCH_MAPI code isn't currently fit  
for integration into the trunk, but needs considerable work. With the most  
recent commits, I can compile it on openSUSE 11.1 with the -devel packages  
 from  
http://download.opensuse.org/repositories/GNOME:/Evolution:/mapi/openSUSE_11.1  
and with file-devel installed (that is the openSUSE package containing  
libmagic development files), if I run

./autogen.sh
mkdir build
cd build
PKG_CONFIG_PATH=/usr/lib/samba4/lib/pkgconfig/ \
../configure -C --enable-MAPI --with-ssl \
CFLAGS="-O -pipe -ggdb3 -Wall -Wextra -W" \
&& make -sj4

You can check out sources from  
http://mknod.org/svn/fetchmail/branches/BRANCH_MAPI/


I'm not interested in doing any of the audit/cleanup/fixup work as  
assessed in this and related threads, but I am happy to help with the  
integration later and help with tracing the requirements.

Reasons are that I personally do not have interest in MAPI support, I have  
no Exchange server to test (Asif Iqbal offered help with testing, but I  
think the tests should be done by those who fix up the code). If someone  
is willing to pay pro rates, can provide an Exchange server to test  
against, be happy with low-profile effort (5 - 20 hours per month - not  
week!) that can start only in a few months' time, contact me off-list. No  
promises though.

So - any volunteers to audit and/or polish BRANCH_MAPI?

Thanks in advance

-- 
Matthias Andree

Re: [fetchmail-devel] fetchmail and mapi II

[Yangyan Li <yan...@gm...>]

Hi Matthias,

Sorry for the messing up of branch MAPI, I will try to fix these
issues one by one.

Roughly speaking, I will concentrate on:
1. a complete review of the existing code, probably a rewriting is required.
2. port this branch to match the latest libmapi release.

In fact, I test on the Exchange Server inside my Virtualbox, it will
be great if anyone can provide some testing environments.

I will seek more advice from you during my development and welcome opinions!

Best Regards!
Yangyan

On Tue, Jul 7, 2009 at 5:09 AM, Matthias Andree<mat...@gm...> wrote:
> Am 16.06.2009, 17:55 Uhr, schrieb mojmir svoboda
> <moj...@2k...>:
>
>> * Mat...@2k...rp
>> <Mat...@2k...rp> [2009-06-16 14:11:37 +0000]:
>>
>>> > as a hotfix i simply allocated lot of memory and removed the loop that
>>> > did nothing. i assume you do not want that kind of solution (and
>>> > neither
>>> > do i.. in long term)
>>>
>>> Ah, ok.
>>
>> in attachment there is a patch fixing this (hopefuly) correctly.  still,
>> the postscript attachment i sent myself got little bit scrambled
>> (periodically at buffer boundaries). may be bug in stream, may be bug in
>> ldb_base64_encode.
>>
>> btw: ldb_base64_encode can be removed from the source file mapi.c, as it
>> is already present in samba4.
>>
>>> >> > 6. \r characters and = spread all around in mail body
>>> This looks like quoted printable encoding to me. hex is likely (i. e. I
>>
>> i confirm. this seems to be hardcoded in mapi.c
>> perhaps there should be an option specifying output format? i.e. if
>> exchange sends it in 7bit, i'm fine with it. no need for another
>> translation.
>>
>>> Perhaps headers do not properly declare content. Do you have an example
>>> of
>>> such a message (inside mutt, just pipe it into "cat - >/tmp/msg.bin",
>>> then
>>> mail msg.bin as application/octet-stream attachment).
>>
>> in attachment. it's in czech, sorry - only sample i did not deleted
>>
>>> >       - fetchmail modifies the "From: " header
>>> Is it fetchmail or OpenChange or Exchange that does this?
>>
>> this is what i have when i look out from outlook:
>>        From: "Matthias Andree" <matthias.andree@your.email>
>>        To: "mojmir svoboda" <mojmir.svoboda@my.email>,
>> fet...@li...
>>        Content-Type: text/plain; format=flowed; delsp=yes;
>> charset=iso-8859-15
>>        Content-Transfer-Encoding: 7bit
>>
>> this is what i receive after fetchmailing via mapi:
>>        From: MatthiasAndree@exchange_server_address.here
>>        To: 2K Czech <MojmirSvoboda>;, fet...@li...
>>        Content-Type: text/plain; charset=us-ascii
>>        Content-Transfer-Encoding: quoted-printable
>>
>> there is some header manipulation code in mapi_fetch_headers, i'd start
>> search there.
>>
>> best regards,
>> mojmir
>
> Hi Mojmir,
>
> I have updated fetchmail's BRANCH_MAPI branch to release 6.3.10 including
> the post-release updates to Spanish and Chinese translations as of
> yesterday.
>
> I have merged your mapi2.patch (thanks), but haven't yet had the time to
> look at the messages or other problems we discussed.
>
> I fixed an issue in rcfile_y.y where the LCID parsing for strings that
> didn't start with 0x was utterly broken.
>
> I fixed configure.ac logic so that --disable-MAPI (or not specifying this)
> works, and we only check for libmagic if needed.
>
> So me new issues I discovered that also need addressing:
>
> - all issues you've mentioned that haven't been addressed by your patch, for
> instance, message corruption as observed, or duplication of base64 encoders.
>
> - there are warnings about broken 4th arguments to one of the openchange
> functions
>
> - mapi.c throws a zillion of warnings of mismatched signedness in
> comparisons, or unused variables. All have to be audited that they don't
> trigger integer overflows or negative indexes.
>
> - the MAPI code adds quite a few if (blah.protocol != P_MAPI) ... #ifdef
> MAPI_ENABLE ... #endif that should be abstracted away rather than copy &
> paste code...
>
> - the bug I fixed in rcfile_y.y was sort of target = sprintf("0x%04x",
> uint32_t); - oops. sprintf takes the output variable as argument and returns
> the length... I hope there isn't more of this bug.
>
> - more will show up as fixup and integration proceed.
>
>
> For better or for worse, the current BRANCH_MAPI code isn't currently fit
> for integration into the trunk, but needs considerable work. With the most
> recent commits, I can compile it on openSUSE 11.1 with the -devel packages
> from
> http://download.opensuse.org/repositories/GNOME:/Evolution:/mapi/openSUSE_11.1
> and with file-devel installed (that is the openSUSE package containing
> libmagic development files), if I run
>
> ./autogen.sh
> mkdir build
> cd build
> PKG_CONFIG_PATH=/usr/lib/samba4/lib/pkgconfig/ \
> ../configure -C --enable-MAPI --with-ssl \
> CFLAGS="-O -pipe -ggdb3 -Wall -Wextra -W" \
> && make -sj4
>
> You can check out sources from
> http://mknod.org/svn/fetchmail/branches/BRANCH_MAPI/
>
>
> I'm not interested in doing any of the audit/cleanup/fixup work as assessed
> in this and related threads, but I am happy to help with the integration
> later and help with tracing the requirements.
>
> Reasons are that I personally do not have interest in MAPI support, I have
> no Exchange server to test (Asif Iqbal offered help with testing, but I
> think the tests should be done by those who fix up the code). If someone is
> willing to pay pro rates, can provide an Exchange server to test against, be
> happy with low-profile effort (5 - 20 hours per month - not week!) that can
> start only in a few months' time, contact me off-list. No promises though.
>
> So - any volunteers to audit and/or polish BRANCH_MAPI?
>
> Thanks in advance
>
> --
> Matthias Andree
>

Re: [fetchmail-devel] fetchmail and mapi II

[Matthias A. <mat...@gm...>]

Am 09.07.2009, 06:43 Uhr, schrieb Yangyan Li <yan...@gm...>:

> Sorry for the messing up of branch MAPI, I will try to fix these
> issues one by one.

That sounds great. Thank you!

> Roughly speaking, I will concentrate on:
> 1. a complete review of the existing code, probably a rewriting is  
> required.
> 2. port this branch to match the latest libmapi release.
>
> In fact, I test on the Exchange Server inside my Virtualbox, it will
> be great if anyone can provide some testing environments.
>
> I will seek more advice from you during my development and welcome  
> opinions!

You're welcome to use the fetchmail-devel@ list for further questions. Use  
"MAPI" in the subject so that all interested parties will see your  
messages.

CONTENTS

I have merged the 6.3.10 release (from /branches/BRANCH_6-3) into  
BRANCH_MAPI, and merged Mojmir's patches where they looked sane, and fixed  
some minor configure.ac issues (see NEWS).

Do we really need libmagic for MAPI support? What exactly does it do?


SUBVERSION CHANGES

WRT the Subversion server:  I have had Graham (who maintains the server)  
enable mergeinfo tracking. The server now also locks out commits from SVN  
clients that are older than version 1.5.0, to make sure that no clients  
mess up the mergeinfo. (The server is version 1.6.X).

The bonus is that you no longer need to manually track which version you  
have merged, Subversion does that for you. It can't hurt to document the  
version, as you used to do :). Just make sure to only use "svn merge [-c  
M[,N...] | -r N:M ...] SOURCE[@REV] [WCPATH]" syntax.

If this feature is new for you, and you're looking for documentation, see  
for instance  
http://svnbook.red-bean.com/en/1.5/svn.branchmerge.basicmerging.html and  
the following sections and chapters.


COMPILE WITH WARNINGS

Could I ask you a favour? Can you configure with modified CFLAGS, for  
instance:

configure -C --with-ssl --enable-MAPI CFLAGS="-O -g -pipe -Wall -W -Wextra  
-pedantic"

(-W -Wextra is redundant, but covers older and newer GCC versions)

That enables some more warnings and helps find bugs - for instance, I  
found the sprintf() bug (around language code parsing) in rcfile_y.y that  
way.

Warnings of the "defined but not used" need not be fixed if you're sure  
that you haven't mistyped somewhere and actually meant to use the unused  
variables.

Signedness warnings should be double-checked and fixed by matching types -  
please do not add casts to silence the compiler!  These warnings are  
usually hints that something's wrong with the code, and a second thought  
needs to be spent especially on indexes and wraparound/overflow behaviour.


Thanks again!

Best regards

-- 
Matthias Andree