Peter Burden wrote:
> On 23/04/2008, Gordon Messmer <[EMAIL PROTECTED]> wrote:
>   
>> Peter Burden wrote:
>>  > Since then, I've cut some code and have a working signing filter.
>>  > If anybody wants to have a look and let me know about any bugs
>>  > (obvious or subtle), source code is at
>>  > http://web.ptwol.net/sigfilter/sigfilter.c
>>  > It's in standard C and uses a MySQL database.
>>  > There are some explanations of  how it works in the source code.
>>  >
>>
>>
>> I don't like criticizing you twice in one evening, but there are some
>>  pretty serious problems with this code:
>>
>>  * Filter is threaded, but you're not taking the required steps to make
>>  mysql thread-safe:
>>  http://dev.mysql.com/doc/refman/5.0/en/threaded-clients.html
>>     
>
> Since the filter doesn't try and share a connection between threads my
> interpretation of the reference is that, provided you link against
> libmysqlclient_r, you don't need to do anything further special.
>   

I don't think that's correct.  I could be wrong, since I rarely use the 
mysql C API (I'm actually not a big fan of MySQL in general), but it 
looks to me like you at *least* need to call mysql_library_init() before 
you create any threads.  I think you also need to call 
mysql_thread_end() to avoid memory leaks.  You should check with the 
mysql list for further confirmation, but I wouldn't trust this code to 
be thread safe as it is.

There's also a minor risk that "tp" will be changed in realmode before 
it's copied in procmsg.  You should pass that functions args by value 
only.  Either just pass fd as an arg, or allocate the memory in the 
realmode function and pass the address to procmsg.

Unrelated: I'm not confident that t->signature will have enough space if 
the field values are much longer than the names from the template.  It's 
kind of hard to tell how that works, but it looks like a likely buffer 
overflow to me.

>>  * Filter uses global variables with no mutex protection.  See above.
>>     
>
> The global variables that are accessed by the threads are only
> read, not written to. They are set up as part of the initialisation, the
> filter does not start listening or launching threads until initialisation
> is complete. I have never found any need for mutexes to control
> access to read-only data. Read-write is, of course, a completely
> different matter.
>   

Yeah... I'm not sure why I thought gettemplate was called by procmsg.

>>  * Filter seems to treat any "AUTH: LOGIN" as if it were a header
>>  * AUTH header won't always say "AUTH: LOGIN"
>>     
>
> Fair comment. For my server installation we currently only use LOGIN
> authentication but this should be easy to fix.
>
> I don't think I'm treating AUTH:LOGIN as if it were a header, based
> on my observation of what Courier seems to do, it appears that AUTH:LOGIN
> is interposed amongst the parts of the "Received:" header - usually on
> a continuation line.
>
> I would accept the criticism that the AUTH:LOGIN recognition code
> doesn't really understand headers and, worse, carries on looking for
> AUTH:LOGIN in the message body if it didn't find it in the headers.
>   

That's what I meant. :)

You should be looking for the first Received header, and examining its 
contents.  Stop there.

> Is there a definitive statement on the location/syntax of "AUTH:LOGIN" 
> anywhere?
> I can't see it in RFC2822/2821.
>   

I'm not sure.  I think I used Courier's code as a reference.

>>  * Finally, and most importantly IMO, you append a plaintext signature to
>>  any text part.  It looks like this includes attached files.  Regardless,
>>  by modifying the existing text parts, you invalidate PGP and SMIME
>>  signatures, which is bad.
>>     
>
> The point about PGP and SMIME is a fair comment, but I find it difficult
> to imagine a signing system that would both satisfy end user requirements
> and not break such things.
>   

I believe that it's possible to encapsulate the original, signed message 
in a new MIME message, and add the signature as a text/plain part at the 
end.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
courier-users mailing list
[email protected]
Unsubscribe: https://lists.sourceforge.net/lists/listinfo/courier-users

Reply via email to