Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DKIM: Fix invalid signature verification issues #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

DKIM: Fix invalid signature verification issues #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/src/dkim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,9 @@ for (sig = dkim_signatures; sig; sig = sig->next)
sig->selector,
sig->canon_headers == PDKIM_CANON_SIMPLE ? "simple" : "relaxed",
sig->canon_body == PDKIM_CANON_SIMPLE ? "simple" : "relaxed",
sig->algo == PDKIM_ALGO_RSA_SHA256 ? "rsa-sha256" : "rsa-sha1",
sig->sigdata.len * 8
sig->algo == PDKIM_ALGO_RSA_SHA256 ? "rsa-sha256" :
(sig->algo == PDKIM_ALGO_RSA_SHA1 ? "rsa-sha1" : "err"),
(int)sig->sigdata.len > -1 ? sig->sigdata.len * 8 : 0
),

sig->identity ? string_sprintf("i=%s ", sig->identity) : US"",
Expand Down Expand Up @@ -186,6 +187,16 @@ for (sig = dkim_signatures; sig; sig = sig->next)
"syntax error in public key record]");
break;

case PDKIM_VERIFY_INVALID_SIGNATURE_ERROR:
logmsg = string_append(logmsg, &size, &ptr, 1,
"signature tag missing or invalid]");
break;

case PDKIM_VERIFY_INVALID_DKIM_VERSION:
logmsg = string_append(logmsg, &size, &ptr, 1,
"unsupported DKIM version]");
break;

default:
logmsg = string_append(logmsg, &size, &ptr, 1,
"unspecified problem]");
Expand Down
45 changes: 41 additions & 4 deletions src/src/pdkim/pdkim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ pdkim_verify_ext_status_str(int ext_status)
case PDKIM_VERIFY_INVALID_BUFFER_SIZE: return "PDKIM_VERIFY_INVALID_BUFFER_SIZE";
case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD: return "PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD";
case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT: return "PDKIM_VERIFY_INVALID_PUBKEY_IMPORT";
case PDKIM_VERIFY_INVALID_SIGNATURE_ERROR: return "PDKIM_VERIFY_INVALID_SIGNATURE_ERROR";
case PDKIM_VERIFY_INVALID_DKIM_VERSION: return "PDKIM_VERIFY_INVALID_DKIM_VERSION";
default: return "PDKIM_VERIFY_UNKNOWN";
}
}
Expand Down Expand Up @@ -407,6 +409,10 @@ sig = store_get(sizeof(pdkim_signature));
memset(sig, 0, sizeof(pdkim_signature));
sig->bodylength = -1;

/* Set so invalid/missing data error display is accurate */
sig->algo = -1;
sig->version = 0;

q = sig->rawsig_no_b_val = store_get(Ustrlen(raw_hdr)+1);

for (p = raw_hdr; ; p++)
Expand Down Expand Up @@ -478,6 +484,8 @@ for (p = raw_hdr; ; p++)
only version there is. */
if (Ustrcmp(cur_val, PDKIM_SIGNATURE_VERSION) == 0)
sig->version = 1;
else
sig->version = -1;
break;
case 'a':
for (i = 0; pdkim_algos[i]; i++)
Expand Down Expand Up @@ -542,10 +550,6 @@ for (p = raw_hdr; ; p++)
*q++ = c;
}

/* Make sure the most important bits are there. */
if (!sig->version)
return NULL;

*q = '\0';
/* Chomp raw header. The final newline must not be added to the signature. */
while (--q > sig->rawsig_no_b_val && (*q == '\r' || *q == '\n'))
Expand Down Expand Up @@ -943,9 +947,12 @@ if (ctx->mode == PDKIM_MODE_VERIFY)
}
}
else
{
DEBUG(D_acl) debug_printf(
"Error while parsing signature header\n"
"PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
goto BAIL;
}
}

/* every other header is stored for signature verification */
Expand Down Expand Up @@ -1475,6 +1482,36 @@ while (sig)

uschar *dns_txt_name, *dns_txt_reply;

/* Make sure we have all required signature tags */
if (!(sig->domain && (*(sig->domain) != '\0') &&
sig->selector && (*(sig->selector) != '\0') &&
sig->headernames && (*(sig->headernames) != '\0') &&
sig->bodyhash.data &&
sig->sigdata.data &&
(sig->algo > -1) &&
sig->version))
{
sig->verify_status = PDKIM_VERIFY_INVALID;
sig->verify_ext_status = PDKIM_VERIFY_INVALID_SIGNATURE_ERROR;

DEBUG(D_acl) debug_printf(
" Error in DKIM-Signature header: tags missing or invalid\n"
"PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
goto NEXT_VERIFY;
}

/* Make sure sig uses supported DKIM version (only v1) */
if (sig->version != 1)
{
sig->verify_status = PDKIM_VERIFY_INVALID;
sig->verify_ext_status = PDKIM_VERIFY_INVALID_DKIM_VERSION;

DEBUG(D_acl) debug_printf(
" Error in DKIM-Signature header: unsupported DKIM version\n"
"PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
goto NEXT_VERIFY;
}

/* Fetch public key for signing domain, from DNS */

dns_txt_name = string_sprintf("%s._domainkey.%s.",
Expand Down
14 changes: 8 additions & 6 deletions src/src/pdkim/pdkim.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,14 @@
#define PDKIM_VERIFY_FAIL 2
#define PDKIM_VERIFY_PASS 3

#define PDKIM_VERIFY_FAIL_BODY 1
#define PDKIM_VERIFY_FAIL_MESSAGE 2
#define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE 3
#define PDKIM_VERIFY_INVALID_BUFFER_SIZE 4
#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD 5
#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT 6
#define PDKIM_VERIFY_FAIL_BODY 1
#define PDKIM_VERIFY_FAIL_MESSAGE 2
#define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE 3
#define PDKIM_VERIFY_INVALID_BUFFER_SIZE 4
#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD 5
#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT 6
#define PDKIM_VERIFY_INVALID_SIGNATURE_ERROR 7
#define PDKIM_VERIFY_INVALID_DKIM_VERSION 8

/* -------------------------------------------------------------------------- */
/* Some parameter values */
Expand Down