From: Jeremy Harris Date: Sat, 6 Aug 2016 23:03:56 +0000 (+0100) Subject: CHUNKING/DKIM: fix handling of lines having a leading dot X-Git-Tag: exim-4_88_RC1~46 X-Git-Url: https://vcs.fsf.org/?p=exim.git;a=commitdiff_plain;h=e983e85a314998aed1d586990969fea128a8b4c7 CHUNKING/DKIM: fix handling of lines having a leading dot --- diff --git a/src/src/dkim.c b/src/src/dkim.c index f09d438d5..3fa11c800 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -69,7 +69,7 @@ pdkim_init(); void -dkim_exim_verify_init(void) +dkim_exim_verify_init(BOOL dot_stuffing) { /* There is a store-reset between header & body reception so cannot use the main pool. Any allocs done by Exim @@ -85,7 +85,7 @@ if (dkim_verify_ctx) /* Create new context */ -dkim_verify_ctx = pdkim_init_verify(&dkim_exim_query_dns_txt); +dkim_verify_ctx = pdkim_init_verify(&dkim_exim_query_dns_txt, dot_stuffing); dkim_collect_input = !!dkim_verify_ctx; /* Start feed up with any cached data */ @@ -102,7 +102,7 @@ int rc; store_pool = POOL_PERM; if ( dkim_collect_input - && (rc = pdkim_feed(dkim_verify_ctx, (char *)data, len)) != PDKIM_OK) + && (rc = pdkim_feed(dkim_verify_ctx, CS data, len)) != PDKIM_OK) { log_write(0, LOG_MAIN, "DKIM: validation error: %.100s", pdkim_errstr(rc)); @@ -460,10 +460,9 @@ switch (what) uschar * -dkim_exim_sign(int dkim_fd, uschar * dkim_private_key, - const uschar * dkim_domain, uschar * dkim_selector, - uschar * dkim_canon, uschar * dkim_sign_headers) +dkim_exim_sign(int dkim_fd, struct ob_dkim * dkim) { +const uschar * dkim_domain; int sep = 0; uschar *seen_items = NULL; int seen_items_size = 0; @@ -487,7 +486,7 @@ int old_pool = store_pool; store_pool = POOL_MAIN; -if (!(dkim_domain = expand_cstring(dkim_domain))) +if (!(dkim_domain = expand_cstring(dkim->dkim_domain))) { /* expansion error, do not send message. */ log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand " @@ -525,7 +524,7 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, /* Set up $dkim_selector expansion variable. */ - if (!(dkim_signing_selector = expand_string(dkim_selector))) + if (!(dkim_signing_selector = expand_string(dkim->dkim_selector))) { log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand " "dkim_selector: %s", expand_string_message); @@ -534,7 +533,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, /* Get canonicalization to use */ - dkim_canon_expanded = dkim_canon ? expand_string(dkim_canon) : US"relaxed"; + dkim_canon_expanded = dkim->dkim_canon + ? expand_string(dkim->dkim_canon) : US"relaxed"; if (!dkim_canon_expanded) { /* expansion error, do not send message. */ @@ -556,8 +556,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, } dkim_sign_headers_expanded = NULL; - if (dkim_sign_headers) - if (!(dkim_sign_headers_expanded = expand_string(dkim_sign_headers))) + if (dkim->dkim_sign_headers) + if (!(dkim_sign_headers_expanded = expand_string(dkim->dkim_sign_headers))) { log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand " "dkim_sign_headers: %s", expand_string_message); @@ -567,7 +567,7 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, /* Get private key to use. */ - if (!(dkim_private_key_expanded = expand_string(dkim_private_key))) + if (!(dkim_private_key_expanded = expand_string(dkim->dkim_private_key))) { log_write(0, LOG_MAIN | LOG_PANIC, "failed to expand " "dkim_private_key: %s", expand_string_message); @@ -587,8 +587,8 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, /* Looks like a filename, load the private key. */ memset(big_buffer, 0, big_buffer_size); - privkey_fd = open(CS dkim_private_key_expanded, O_RDONLY); - if (privkey_fd < 0) + + if ((privkey_fd = open(CS dkim_private_key_expanded, O_RDONLY)) < 0) { log_write(0, LOG_MAIN | LOG_PANIC, "unable to open " "private key file for reading: %s", @@ -610,16 +610,17 @@ while ((dkim_signing_domain = string_nextinlist(&dkim_domain, &sep, ctx = pdkim_init_sign( CS dkim_signing_domain, CS dkim_signing_selector, CS dkim_private_key_expanded, - PDKIM_ALGO_RSA_SHA256); + PDKIM_ALGO_RSA_SHA256, + dkim->dot_stuffed); pdkim_set_optional(ctx, - (char *) dkim_sign_headers_expanded, + CS dkim_sign_headers_expanded, NULL, pdkim_canon, pdkim_canon, -1, 0, 0); lseek(dkim_fd, 0, SEEK_SET); - while ((sread = read(dkim_fd, &buf, 4096)) > 0) + while ((sread = read(dkim_fd, &buf, sizeof(buf))) > 0) if ((pdkim_rc = pdkim_feed(ctx, buf, sread)) != PDKIM_OK) goto pk_bad; diff --git a/src/src/dkim.h b/src/src/dkim.h index 0dade972f..8474962fc 100644 --- a/src/src/dkim.h +++ b/src/src/dkim.h @@ -6,8 +6,8 @@ /* See the file NOTICE for conditions of use and distribution. */ void dkim_exim_init(void); -uschar *dkim_exim_sign(int, uschar *, const uschar *, uschar *, uschar *, uschar *); -void dkim_exim_verify_init(void); +uschar *dkim_exim_sign(int, struct ob_dkim *); +void dkim_exim_verify_init(BOOL); void dkim_exim_verify_feed(uschar *, int); void dkim_exim_verify_finish(void); void dkim_exim_acl_setup(uschar *); diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index f468d232b..915b90e5c 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -794,7 +794,7 @@ for (sig = ctx->sig; sig; sig = sig->next) } /* SIGNING -------------------------------------------------------------- */ - if (ctx->mode == PDKIM_MODE_SIGN) + if (ctx->flags & PDKIM_MODE_SIGN) { sig->bodyhash = bh; @@ -830,8 +830,32 @@ for (sig = ctx->sig; sig; sig = sig->next) +static int +pdkim_body_complete(pdkim_ctx * ctx) +{ +pdkim_signature * sig = ctx->sig; /*XXX assumes only one sig */ + +/* In simple body mode, if any empty lines were buffered, +replace with one. rfc 4871 3.4.3 */ +/*XXX checking the signed-body-bytes is a gross hack; I think +it indicates that all linebreaks should be buffered, including +the one terminating a text line */ + +if ( sig && sig->canon_body == PDKIM_CANON_SIMPLE + && sig->signed_body_bytes == 0 + && ctx->num_buffered_crlf > 0 + ) + pdkim_update_bodyhash(ctx, "\r\n", 2); + +ctx->flags |= PDKIM_SEEN_EOD; +ctx->linebuf_offset = 0; +return PDKIM_OK; +} + + + /* -------------------------------------------------------------------------- */ -/* Callback from pdkim_feed below for processing complete body lines */ +/* Call from pdkim_feed below for processing complete body lines */ static int pdkim_bodyline_complete(pdkim_ctx *ctx) @@ -841,33 +865,23 @@ int n = ctx->linebuf_offset; pdkim_signature *sig = ctx->sig; /*XXX assumes only one sig */ /* Ignore extra data if we've seen the end-of-data marker */ -if (ctx->seen_eod) goto BAIL; +if (ctx->flags & PDKIM_SEEN_EOD) goto BAIL; /* We've always got one extra byte to stuff a zero ... */ ctx->linebuf[ctx->linebuf_offset] = '\0'; /* Terminate on EOD marker */ -if (memcmp(p, ".\r\n", 3) == 0) +if (ctx->flags & PDKIM_DOT_TERM) { - /* In simple body mode, if any empty lines were buffered, - replace with one. rfc 4871 3.4.3 */ - /*XXX checking the signed-body-bytes is a gross hack; I think - it indicates that all linebreaks should be buffered, including - the one terminating a text line */ - if ( sig && sig->canon_body == PDKIM_CANON_SIMPLE - && sig->signed_body_bytes == 0 - && ctx->num_buffered_crlf > 0 - ) - pdkim_update_bodyhash(ctx, "\r\n", 2); + if ( memcmp(p, ".\r\n", 3) == 0) + return pdkim_body_complete(ctx); - ctx->seen_eod = TRUE; - goto BAIL; - } -/* Unstuff dots */ -if (memcmp(p, "..", 2) == 0) - { - p++; - n--; + /* Unstuff dots */ + if (memcmp(p, "..", 2) == 0) + { + p++; + n--; + } } /* Empty lines need to be buffered until we find a non-empty line */ @@ -927,7 +941,7 @@ ctx->num_headers++; if (ctx->num_headers > PDKIM_MAX_HEADERS) goto BAIL; /* SIGNING -------------------------------------------------------------- */ -if (ctx->mode == PDKIM_MODE_SIGN) +if (ctx->flags & PDKIM_MODE_SIGN) { pdkim_signature *sig; @@ -940,7 +954,7 @@ if (ctx->mode == PDKIM_MODE_SIGN) /* VERIFICATION ----------------------------------------------------------- */ /* DKIM-Signature: headers are added to the verification list */ -if (ctx->mode == PDKIM_MODE_VERIFY) +else { if (strncasecmp(CCS ctx->cur_header, DKIM_SIGNATURE_HEADERNAME, @@ -990,11 +1004,15 @@ pdkim_feed(pdkim_ctx *ctx, char *data, int len) { int p; +/* Alternate EOD signal, used in non-dotstuffing mode */ +if (!data) + pdkim_body_complete(ctx); + for (p = 0; ppast_headers) + if (ctx->flags & PDKIM_PAST_HDRS) { /* Processing body byte */ ctx->linebuf[ctx->linebuf_offset++] = c; @@ -1013,28 +1031,27 @@ for (p = 0; pseen_lf) + if (ctx->flags & PDKIM_SEEN_LF) { int rc = pdkim_header_complete(ctx); /* Seen last header line */ if (rc != PDKIM_OK) return rc; - ctx->past_headers = TRUE; - ctx->seen_lf = 0; + ctx->flags = ctx->flags & ~PDKIM_SEEN_LF | PDKIM_PAST_HDRS; DEBUG(D_acl) debug_printf( "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>\n"); continue; } else - ctx->seen_lf = TRUE; + ctx->flags |= PDKIM_SEEN_LF; } - else if (ctx->seen_lf) + else if (ctx->flags & PDKIM_SEEN_LF) { if (!(c == '\t' || c == ' ')) { int rc = pdkim_header_complete(ctx); /* End of header */ if (rc != PDKIM_OK) return rc; } - ctx->seen_lf = FALSE; + ctx->flags &= ~PDKIM_SEEN_LF; } } @@ -1328,7 +1345,7 @@ while (sig) Then append to that list any remaining header names for which there was no header to sign. */ - if (ctx->mode == PDKIM_MODE_SIGN) + if (ctx->flags & PDKIM_MODE_SIGN) { pdkim_stringlist *p; const uschar * l; @@ -1449,11 +1466,11 @@ while (sig) } /* Remember headers block for signing (when the library cannot do incremental) */ - if (ctx->mode == PDKIM_MODE_SIGN) + if (ctx->flags & PDKIM_MODE_SIGN) (void) exim_rsa_data_append(&hdata, &hdata_alloc, US sig_hdr); /* SIGNING ---------------------------------------------------------------- */ - if (ctx->mode == PDKIM_MODE_SIGN) + if (ctx->flags & PDKIM_MODE_SIGN) { es_ctx sctx; const uschar * errstr; @@ -1616,15 +1633,15 @@ return PDKIM_OK; /* -------------------------------------------------------------------------- */ DLLEXPORT pdkim_ctx * -pdkim_init_verify(int(*dns_txt_callback)(char *, char *)) +pdkim_init_verify(int(*dns_txt_callback)(char *, char *), BOOL dot_stuffing) { pdkim_ctx * ctx; ctx = store_get(sizeof(pdkim_ctx)); memset(ctx, 0, sizeof(pdkim_ctx)); +if (dot_stuffing) ctx->flags = PDKIM_DOT_TERM; ctx->linebuf = store_get(PDKIM_MAX_BODY_LINE_LEN); -ctx->mode = PDKIM_MODE_VERIFY; ctx->dns_txt_callback = dns_txt_callback; return ctx; @@ -1634,7 +1651,8 @@ return ctx; /* -------------------------------------------------------------------------- */ DLLEXPORT pdkim_ctx * -pdkim_init_sign(char *domain, char *selector, char *rsa_privkey, int algo) +pdkim_init_sign(char *domain, char *selector, char *rsa_privkey, int algo, + BOOL dot_stuffed) { pdkim_ctx *ctx; pdkim_signature *sig; @@ -1645,14 +1663,13 @@ if (!domain || !selector || !rsa_privkey) ctx = store_get(sizeof(pdkim_ctx)); memset(ctx, 0, sizeof(pdkim_ctx)); +ctx->flags = dot_stuffed ? PDKIM_MODE_SIGN | PDKIM_DOT_TERM : PDKIM_MODE_SIGN; ctx->linebuf = store_get(PDKIM_MAX_BODY_LINE_LEN); sig = store_get(sizeof(pdkim_signature)); memset(sig, 0, sizeof(pdkim_signature)); sig->bodylength = -1; - -ctx->mode = PDKIM_MODE_SIGN; ctx->sig = sig; sig->domain = string_copy(US domain); diff --git a/src/src/pdkim/pdkim.h b/src/src/pdkim/pdkim.h index 5d0157e63..536e5af69 100644 --- a/src/src/pdkim/pdkim.h +++ b/src/src/pdkim/pdkim.h @@ -245,12 +245,14 @@ typedef struct pdkim_signature { /* -------------------------------------------------------------------------- */ /* Context to keep state between all operations. */ -#define PDKIM_MODE_SIGN 0 -#define PDKIM_MODE_VERIFY 1 typedef struct pdkim_ctx { - /* PDKIM_MODE_VERIFY or PDKIM_MODE_SIGN */ - int mode; +#define PDKIM_MODE_SIGN BIT(0) /* if unset, mode==verify */ +#define PDKIM_DOT_TERM BIT(1) /* dot termination and unstuffing */ +#define PDKIM_SEEN_LF BIT(2) +#define PDKIM_SEEN_EOD BIT(3) +#define PDKIM_PAST_HDRS BIT(4) + unsigned flags; /* One (signing) or several chained (verification) signatures */ pdkim_signature *sig; @@ -264,9 +266,6 @@ typedef struct pdkim_ctx { int cur_header_len; char *linebuf; int linebuf_offset; - BOOL seen_lf; - BOOL seen_eod; - BOOL past_headers; int num_buffered_crlf; int num_headers; pdkim_stringlist *headers; /* Raw headers for verification */ @@ -285,10 +284,10 @@ extern "C" { void pdkim_init (void); DLLEXPORT -pdkim_ctx *pdkim_init_sign (char *, char *, char *, int); +pdkim_ctx *pdkim_init_sign (char *, char *, char *, int, BOOL); DLLEXPORT -pdkim_ctx *pdkim_init_verify (int(*)(char *, char *)); +pdkim_ctx *pdkim_init_verify (int(*)(char *, char *), BOOL); DLLEXPORT int pdkim_set_optional (pdkim_ctx *, char *, char *,int, int, diff --git a/src/src/receive.c b/src/src/receive.c index 6a8ce8841..3351ab1c5 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -1615,8 +1615,10 @@ message_linecount = body_linecount = body_zerocount = max_received_linelength = 0; #ifndef DISABLE_DKIM -/* Call into DKIM to set up the context. */ -if (smtp_input && !smtp_batched_input && !dkim_disable_verify) dkim_exim_verify_init(); +/* Call into DKIM to set up the context. In CHUNKING mode +we clear the dot-stuffing flag */ +if (smtp_input && !smtp_batched_input && !dkim_disable_verify) + dkim_exim_verify_init(chunking_state <= CHUNKING_OFFERED); #endif #ifdef EXPERIMENTAL_DMARC diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 6c9afc501..34481aef7 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -399,7 +399,7 @@ for(;;) if (chunking_state == CHUNKING_LAST) { #ifndef DISABLE_DKIM - dkim_exim_verify_feed(".\r\n", 3); /* for consistency with .-term MAIL */ + dkim_exim_verify_feed(NULL, 0); /* notify EOD */ #endif return EOD; } diff --git a/src/src/structs.h b/src/src/structs.h index 23c40ea3d..75eba6643 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -874,6 +874,7 @@ struct ob_dkim { uschar *dkim_canon; uschar *dkim_sign_headers; uschar *dkim_strict; -} dkim; + BOOL dot_stuffed; +}; /* End of structs.h */ diff --git a/src/src/transport.c b/src/src/transport.c index fdd97822a..8e69fb686 100644 --- a/src/src/transport.c +++ b/src/src/transport.c @@ -1081,7 +1081,8 @@ if ((dkim_fd = Uopen(dkim_spool_name, O_RDWR|O_CREAT|O_TRUNC, SPOOL_MODE)) < 0) goto CLEANUP; } -/* Call original function to write the -K file; does the CRLF expansion */ +/* Call original function to write the -K file; does the CRLF expansion +(but, in the CHUNKING case, not dot-stuffing and dot-termination). */ options = tctx->options; tctx->options &= ~topt_use_bdat; @@ -1096,14 +1097,9 @@ if (!rc) } /* Rewind file and feed it to the goats^W DKIM lib */ +dkim->dot_stuffed = !!(options & topt_end_dot); lseek(dkim_fd, 0, SEEK_SET); -dkim_signature = dkim_exim_sign(dkim_fd, - dkim->dkim_private_key, - dkim->dkim_domain, - dkim->dkim_selector, - dkim->dkim_canon, - dkim->dkim_sign_headers); -if (dkim_signature) +if ((dkim_signature = dkim_exim_sign(dkim_fd, dkim))) siglen = Ustrlen(dkim_signature); else if (dkim->dkim_strict) { diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 416ea6297..6b0781096 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -266,7 +266,8 @@ smtp_transport_options_block smtp_transport_option_defaults = { NULL, /* dkim_private_key */ NULL, /* dkim_selector */ NULL, /* dkim_sign_headers */ - NULL} /* dkim_strict */ + NULL, /* dkim_strict */ + FALSE} /* dot_stuffed */ #endif };