#2 header list (h=) is not created and not evaluated properly

[Anonymous]

Hi,

I think I found some problem with NOFWS signing messages and created a patch for it.

Explanation

1) a list of headers used to calculate the signature (h=..) should always be created. Lots of mailer daemons, anti-virus scanners and mailing services add additional headers at the end of the list. eg: www.gmx.net and others. If the signature is checked against the altered mail it will certainly fail. So I added proper code and two functions to be able to read a list of headers used to calculate the signature.

2) During my tests validating the sent messages always failed. I found out that validation was successful if the "h=" list was removed. Scrutinizing the code I found the following:
* if a header list is set then this list is looped to find the proper header. these steps are taken:
a) get a header name from the "h=" list ("RECEIVED")
b) start searching for this name in the list of mail headers
c) use the FIRST occurence of this header
d) get back to a

you have a big problem if there are more than one header lines with the same name, as it usually is with "RECEIVED". During verification the first occurence will be used multiple times instead of each one only once. Therefore validation failes.

The solution would be to exchange the loops. Get a mail header line and search for it in the list of headers. Then erase the header name from the header list. I have done that and additionally have rewritten the code to fit into function "dkheaders" instead of "dkheaders_headers".

3) unfortunately there are a lot of mail filters that do not care about the order of the mail header lines. Especially qmail-scanner gives damn about it. If the mail is sent to another mail server using qmail-scanner, the previous "X-Spam-Status" line may be removed and relocated at the top of the mail, with new values. If this header has been used to calculate the signature then verifying the signature fails (of course properly). I think there are a lot more programs that reposition some header lines occasionally. In order to be able to cope with these without reprogramming every of them, the environment variable DKIGNORE is introduced. This contains a list of header names that should not be used for signing, separated by colons ':'. Now "X-Spam-Status" can be ignored when calculating the signature. eg: "X-Spam-Level:X-Spam-Status". Checks are made case insensitive.
I think the DomainKeys draft does not forbid ignoring some header if in "nofws" mode lines and I think it complies with validation standard procedure if a "h=" list of used headers is included in the signature.
To achive this major rewriting of "dkheaders" was necessary.

I have tested the included patch extensively but bugs may still exists. Please drop me a line if you find some.
In case you have any questions or need some further explanations feel free to contact me:
per (dot.) sil (at-@) gmx (dot.) it

Perolo Silantico

PS: Happy new year!!

[Nobody/Anonymous]

libdomainkeys.headerlines.2005-01-01.patch

[Perolo]

Logged In: YES
user_id=1188195

Thanks to Axis who found a bug in my patch. SInce I can not
replace the old patch, please apply this addidional patch
after the first one:

=========== SNIPP ==============
--- libdomainkeys-0.62_test/domainkeys.c 2005-01-01
11:04:41.354065576 +0100
+++ libdomainkeys-0.62/domainkeys.c 2005-01-01
10:56:29.295869792 +0100
@@ -378,16 +378,16 @@
list
*/
static int
-dkheaders_isinheaderlist(DK *dk, char* headername, int
consumeIt)
+dkheaders_isinheaderlist(char* headerslist, char*
headername, int consumeIt)
{
int namelen = 0;
int i, pos, len;
int headerslen;

- if (!dk || !headername || !dk->headers)
+ if (!headername || !headerslist)
return 0;

- headerslen = strlen(dk->headers);
+ headerslen = strlen(headerslist);
if (headerslen <= 0) return 0;

/* calculate length of the name of the header.
@@ -400,14 +400,14 @@
the header must exactly match the value in the header
list. */
pos = 0;
for (i = 0; i < headerslen; i++) {
- if (!dk->headers[i] || dk->headers[i] == ':') {
+ if (!headerslist[i] || headerslist[i] == ':') {
len = i - pos;
if ((i-pos > 0) && (namelen == i-pos) &&
- !strncmp(headername, dk->ignoreheaders[i], namelen)) {
+ !strncmp(headername, headerslist + i, namelen)) {

/* clean this header-name to avoid including it
more than once */
if (consumeIt)
- memset(dk->ignoreheaders[i], ':',
strlen(dk->ignoreheaders[i]));
+ memset(headerslist + i, ':', namelen);
return 1;
}

@@ -734,7 +734,7 @@
/* hash complete header line only if the header is
not to be ignored
or it is in the headers list when verifying */
if (!dk_isignoreheader(dk, dk->header +
lastheader_start) ||
- dkheaders_isinheaderlist(dk, dk->header +
lastheader_start, 1))
+ dkheaders_isinheaderlist(dk->headers, dk->header
+ lastheader_start, 1))
{
for (j = lastheader_start; j < dk->headerlen && j
< i; j++) {
if (dk->canon == DK_CANON_NOFWS &&
(dk->header[j] == '\r' || dk->header[j] == '\n')) ;
@@ -1203,12 +1203,19 @@
*/
DK_STAT dk_free(DK *dk)
{
+ int i;
+
if (!dk) return DK_STAT_ARGS;
if (dk->dkmarker != DKMARK) return DK_STAT_ARGS;
if (dk->from) free(dk->from);
if (dk->sender) free(dk->from);
if (dk->headers && dk->h_max) free(dk->headers);
- if (dk->ignoreheaders) free(dk->ignoreheaders);
+ if (dk->ignoreheaders) {
+ for (i = 0; i < dk->ignoreheaderslen; i++)
+ free(dk->ignoreheaders[i]);
+
+ free(dk->ignoreheaders);
+ }
free(dk->header); /* alloc'ing dk->header is
not optional. */
dk->dkmarker = ~DKMARK;
free(dk);
==============================

[Perolo]

Logged In: YES
user_id=1188195

OK, copying and pasting from my previous comment is not very
easy because of the line-wrapping.
Since I can not upload a new patch, it is available at:

http://perolo.vantage.at/patches/libdomainkeys.headerlines.2005-01-01.patch

Perhaps someone else can add another file to this report....

Perolo

[Timothy Der]

Logged In: YES
user_id=1254642

this has been addressed and should be fixed in 0.65