commit 88a9e485e3709a77867542f5e9a5f7ac60238f49 Author: Jeremy Harris Date: Wed Nov 29 22:18:18 2017 +0000 TLS: Fix excessive calling of smtp_auth_acl under AUTH_TLS. Bug 2203 diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 0aabc535..76c72de9 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -187,7 +187,7 @@ static smtp_cmd_list cmd_list[] = { { "auth", sizeof("auth")-1, AUTH_CMD, TRUE, TRUE }, #ifdef SUPPORT_TLS { "starttls", sizeof("starttls")-1, STARTTLS_CMD, FALSE, FALSE }, - { "tls_auth", 0, TLS_AUTH_CMD, FALSE, TRUE }, + { "tls_auth", 0, TLS_AUTH_CMD, FALSE, FALSE }, #endif /* If you change anything above here, also fix the definitions below. */ @@ -2826,8 +2826,12 @@ if (check_proxy_protocol_host()) smtps port for use with older style SSL MTAs. */ #ifdef SUPPORT_TLS - if (tls_in.on_connect && tls_server_start(tls_require_ciphers, &user_msg) != OK) - return smtp_log_tls_fail(user_msg); + if (tls_in.on_connect) + { + if (tls_server_start(tls_require_ciphers, &user_msg) != OK) + return smtp_log_tls_fail(user_msg); + cmd_list[CMD_LIST_TLS_AUTH].is_mail_cmd = TRUE; + } #endif /* Run the connect ACL if it exists */ @@ -3800,7 +3804,6 @@ cmd_list[CMD_LIST_HELO].is_mail_cmd = TRUE; cmd_list[CMD_LIST_EHLO].is_mail_cmd = TRUE; #ifdef SUPPORT_TLS cmd_list[CMD_LIST_STARTTLS].is_mail_cmd = TRUE; -cmd_list[CMD_LIST_TLS_AUTH].is_mail_cmd = TRUE; #endif /* Set the local signal handler for SIGTERM - it tries to end off tidily */ commit 2d9c1e31d619a61ca09b30a3abfe73ddfed67ce5 Author: Jeremy Harris Date: Wed Nov 29 23:22:34 2017 +0000 TLS: avoid calling smtp_auth_acl on client cert when no tls authenticator is configured diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 76c72de9..00e9d41a 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -3849,24 +3849,24 @@ while (done <= 0) ) { cmd_list[CMD_LIST_TLS_AUTH].is_mail_cmd = FALSE; - if ( acl_smtp_auth - && (rc = acl_check(ACL_WHERE_AUTH, NULL, acl_smtp_auth, - &user_msg, &log_msg)) != OK - ) - { - done = smtp_handle_acl_fail(ACL_WHERE_AUTH, rc, user_msg, log_msg); - continue; - } for (au = auths; au; au = au->next) if (strcmpic(US"tls", au->driver_name) == 0) { - smtp_cmd_data = NULL; - - if (smtp_in_auth(au, &s, &ss) == OK) - { DEBUG(D_auth) debug_printf("tls auth succeeded\n"); } + if ( acl_smtp_auth + && (rc = acl_check(ACL_WHERE_AUTH, NULL, acl_smtp_auth, + &user_msg, &log_msg)) != OK + ) + done = smtp_handle_acl_fail(ACL_WHERE_AUTH, rc, user_msg, log_msg); else - { DEBUG(D_auth) debug_printf("tls auth not succeeded\n"); } + { + smtp_cmd_data = NULL; + + if (smtp_in_auth(au, &s, &ss) == OK) + { DEBUG(D_auth) debug_printf("tls auth succeeded\n"); } + else + { DEBUG(D_auth) debug_printf("tls auth not succeeded\n"); } + } break; } } commit f9d9829cee89e77043113209b0fb619d66274bed Author: Jeremy Harris Date: Fri Dec 1 22:43:19 2017 +0000 Debug: fix coding in dnssec reporting. Bug 2205 diff --git a/src/src/host.c b/src/src/host.c index 05bde3fb..1f0d9195 100644 --- a/src/src/host.c +++ b/src/src/host.c @@ -2612,8 +2612,8 @@ if ((whichrrs & HOST_FIND_BY_SRV) != 0) DEBUG(D_dns) if ((dnssec_request || dnssec_require) - & !dns_is_secure(&dnsa) - & dns_is_aa(&dnsa)) + && !dns_is_secure(&dnsa) + && dns_is_aa(&dnsa)) debug_printf("DNS lookup of %.256s (SRV) requested AD, but got AA\n", host->name); if (dnssec_request) commit c73a4d073e195c11bb5b03b91c61478ab6935593 Author: Heiko Schlittermann (HS12-RIPE) Date: Sun Dec 3 18:17:43 2017 +0100 DKIM: Ignore non-DKIM TXT records in DNS response. Bug 2207 diff --git a/src/src/dkim.c b/src/src/dkim.c index 5e97c1b7..9731a63d 100644 --- a/src/src/dkim.c +++ b/src/src/dkim.c @@ -73,6 +73,9 @@ for (rr = dns_next_rr(&dnsa, &dnss, RESET_ANSWERS); if (answer_offset >= PDKIM_DNS_TXT_MAX_RECLEN) return PDKIM_FAIL; /*XXX better error detail? logging? */ } + + /* check if this looks like a DKIM record */ + if (strncasecmp(answer, "v=dkim", 6) != 0) continue; return PDKIM_OK; } @@ -148,7 +151,7 @@ if (!(s = sig->domain)) s = US""; logmsg = string_append(logmsg, 2, "d=", s); if (!(s = sig->selector)) s = US""; logmsg = string_append(logmsg, 2, " s=", s); -logmsg = string_append(logmsg, 7, +logmsg = string_append(logmsg, 7, " c=", sig->canon_headers == PDKIM_CANON_SIMPLE ? "simple" : "relaxed", "/", sig->canon_body == PDKIM_CANON_SIMPLE ? "simple" : "relaxed", " a=", dkim_sig_to_a_tag(sig), @@ -371,7 +374,7 @@ for (sig = dkim_signatures; sig; sig = sig->next) dkim_verify_status = dkim_exim_expand_query(DKIM_VERIFY_STATUS); dkim_verify_reason = dkim_exim_expand_query(DKIM_VERIFY_REASON); - + if ((rc = dkim_acl_call(id, res_ptr, user_msgptr, log_msgptr)) != OK) return rc; } commit c1f0ee1044f2d39648ffdd96194566020d3d164e Author: Andreas Piesk Date: Fri Dec 22 10:05:02 2017 +0000 Fix build of nisplus lookup diff --git a/src/src/lookups/nisplus.c b/src/src/lookups/nisplus.c index e4184bbd..7cd00eed 100644 --- a/src/src/lookups/nisplus.c +++ b/src/src/lookups/nisplus.c @@ -54,7 +54,6 @@ nis_object *tno, *eno; struct entry_obj *eo; struct table_obj *ta; uschar *p = query + length; -uschar *yield = NULL; gstring * yield = NULL; do_cache = do_cache; /* Placate picky compilers */ @@ -173,7 +172,7 @@ for (i = 0; i < eo->en_cols.en_cols_len; i++) yield = string_catn(yield, US"\"", 1); } else - eyield = string_catn(yield, value, len); + yield = string_catn(yield, value, len); yield = string_catn(yield, US" ", 1); } commit 032e3aa3d12b3c939c8602ff801847a8305bd849 Author: Jeremy Harris Date: Fri Dec 22 10:25:56 2017 +0000 Fix const issue in nisplus lookup diff --git a/src/src/lookups/nisplus.c b/src/src/lookups/nisplus.c index 7cd00eed..1f226d52 100644 --- a/src/src/lookups/nisplus.c +++ b/src/src/lookups/nisplus.c @@ -42,18 +42,18 @@ yield is the concatenation of all the fields, preceded by their names and an equals sign. */ static int -nisplus_find(void *handle, uschar *filename, uschar *query, int length, +nisplus_find(void *handle, uschar *filename, const uschar *query, int length, uschar **result, uschar **errmsg, uint *do_cache) { int i; int error_error = FAIL; -uschar *field_name = NULL; +const uschar * field_name = NULL; nis_result *nrt = NULL; nis_result *nre = NULL; nis_object *tno, *eno; struct entry_obj *eo; struct table_obj *ta; -uschar *p = query + length; +const uschar * p = query + length; gstring * yield = NULL; do_cache = do_cache; /* Placate picky compilers */ @@ -63,12 +63,15 @@ has been given. */ while (p > query && p[-1] != ':') p--; -if (p > query) +if (p > query) /* get the query without the result-field */ { + uint len = p-1 - query; field_name = p; - p[-1] = 0; + query = string_copyn(query, len); + p = query + len; } -else p = query + length; +else + p = query + length; /* Now search backwards to find the comma that starts the table name. */ @@ -100,7 +103,7 @@ if (tno->zo_data.zo_type != TABLE_OBJ) *errmsg = string_sprintf("NIS+ error: %s is not a table", p); goto NISPLUS_EXIT; } -ta = &(tno->zo_data.objdata_u.ta_data); +ta = &tno->zo_data.objdata_u.ta_data; /* Now look up the entry in the table, check that we got precisely one object and that it is a table entry. */ @@ -152,7 +155,7 @@ for (i = 0; i < eo->en_cols.en_cols_len; i++) /* Concatenate all fields if no specific one selected */ - if (field_name == NULL) + if (!field_name) { yield = string_cat (yield, tc->tc_name); yield = string_catn(yield, US"=", 1); @@ -195,11 +198,9 @@ if (field_name) else store_reset(yield->s + yield->ptr + 1); -/* Restore the colon in the query, and free result store before -finishing. */ +/* Free result store before finishing. */ NISPLUS_EXIT: -if (field_name) field_name[-1] = ':'; if (nrt) nis_freeresult(nrt); if (nre) nis_freeresult(nre); commit 8143ff45b2b9edd3532fe4056d5736ecc4e0a766 Author: Jeremy Harris Date: Sun Dec 24 21:30:20 2017 +0000 Lookups: fix pgsql multiple-row, single-column return Report & fix from James ; additional tidying and testcase by JGH Broken-by: acec9514b1 diff --git a/src/src/lookups/pgsql.c b/src/src/lookups/pgsql.c index 0b771f59..cece43b6 100644 --- a/src/src/lookups/pgsql.c +++ b/src/src/lookups/pgsql.c @@ -140,7 +140,7 @@ has the password removed. This copy is also used for debugging output. */ for (i = 2; i >= 0; i--) { uschar *pp = Ustrrchr(server, '/'); - if (pp == NULL) + if (!pp) { *errmsg = string_sprintf("incomplete pgSQL server data: %s", (i == 2)? server : server_copy); @@ -156,18 +156,16 @@ for (i = 2; i >= 0; i--) start is the identification of the server (host or path). See if we have a cached connection to the server. */ -for (cn = pgsql_connections; cn != NULL; cn = cn->next) - { +for (cn = pgsql_connections; cn; cn = cn->next) if (Ustrcmp(cn->server, server_copy) == 0) { pg_conn = cn->handle; break; } - } /* If there is no cached connection, we must set one up. */ -if (cn == NULL) +if (!cn) { uschar *port = US""; @@ -178,7 +176,7 @@ if (cn == NULL) uschar *last_slash, *last_dot, *p; p = ++server; - while (*p != 0 && *p != ')') p++; + while (*p && *p != ')') p++; *p = 0; last_slash = Ustrrchr(server, '/'); @@ -191,10 +189,9 @@ if (cn == NULL) We have to call PQsetdbLogin with '/var/run/postgresql' as the hostname argument and put '5432' into the port variable. */ - if (last_slash == NULL || last_dot == NULL) + if (!last_slash || !last_dot) { - *errmsg = string_sprintf("PGSQL invalid filename for socket: %s", - server); + *errmsg = string_sprintf("PGSQL invalid filename for socket: %s", server); *defer_break = TRUE; return DEFER; } @@ -211,13 +208,13 @@ if (cn == NULL) else { uschar *p; - if ((p = Ustrchr(server, ':')) != NULL) + if ((p = Ustrchr(server, ':'))) { *p++ = 0; port = p; } - if (Ustrchr(server, '/') != NULL) + if (Ustrchr(server, '/')) { *errmsg = string_sprintf("unexpected slash in pgSQL server hostname: %s", server); @@ -280,37 +277,37 @@ else /* Run the query */ - pg_result = PQexec(pg_conn, CS query); - switch(PQresultStatus(pg_result)) - { - case PGRES_EMPTY_QUERY: - case PGRES_COMMAND_OK: - /* The command was successful but did not return any data since it was - not SELECT but either an INSERT, UPDATE or DELETE statement. Tell the - high level code to not cache this query, and clean the current cache for - this handle by setting *do_cache zero. */ - - result = string_cat(result, US PQcmdTuples(pg_result)); - *do_cache = 0; - DEBUG(D_lookup) debug_printf("PGSQL: command does not return any data " - "but was successful. Rows affected: %s\n", result->s); - break; +pg_result = PQexec(pg_conn, CS query); +switch(PQresultStatus(pg_result)) + { + case PGRES_EMPTY_QUERY: + case PGRES_COMMAND_OK: + /* The command was successful but did not return any data since it was + not SELECT but either an INSERT, UPDATE or DELETE statement. Tell the + high level code to not cache this query, and clean the current cache for + this handle by setting *do_cache zero. */ + + result = string_cat(result, US PQcmdTuples(pg_result)); + *do_cache = 0; + DEBUG(D_lookup) debug_printf("PGSQL: command does not return any data " + "but was successful. Rows affected: %s\n", string_from_gstring(result)); + break; - case PGRES_TUPLES_OK: - break; + case PGRES_TUPLES_OK: + break; - default: - /* This was the original code: - *errmsg = string_sprintf("PGSQL: query failed: %s\n", - PQresultErrorMessage(pg_result)); - This was suggested by a user: - */ - - *errmsg = string_sprintf("PGSQL: query failed: %s (%s) (%s)\n", - PQresultErrorMessage(pg_result), - PQresStatus(PQresultStatus(pg_result)), query); - goto PGSQL_EXIT; - } + default: + /* This was the original code: + *errmsg = string_sprintf("PGSQL: query failed: %s\n", + PQresultErrorMessage(pg_result)); + This was suggested by a user: + */ + + *errmsg = string_sprintf("PGSQL: query failed: %s (%s) (%s)\n", + PQresultErrorMessage(pg_result), + PQresStatus(PQresultStatus(pg_result)), query); + goto PGSQL_EXIT; + } /* Result is in pg_result. Find the number of fields returned. If this is one, we don't add field names to the data. Otherwise we do. If the query did not @@ -329,7 +326,7 @@ for (i = 0; i < num_tuples; i++) result = string_catn(result, US"\n", 1); if (num_fields == 1) - result = string_catn(NULL, + result = string_catn(result, US PQgetvalue(pg_result, i, 0), PQgetlength(pg_result, i, 0)); else { @@ -342,17 +339,13 @@ for (i = 0; i < num_tuples; i++) } } -/* If result is NULL then no data has been found and so we return FAIL. -Otherwise, we must terminate the string which has been built; string_cat() -always leaves enough room for a terminating zero. */ +/* If result is NULL then no data has been found and so we return FAIL. */ if (!result) { yield = FAIL; *errmsg = US"PGSQL: no data found"; } -else - store_reset(result->s + result->ptr + 1); /* Get here by goto from various error checks. */ @@ -367,6 +360,7 @@ if (pg_result) PQclear(pg_result); if (result) { + store_reset(result->s + result->ptr + 1); *resultptr = string_from_gstring(result); return OK; } commit db98649c6f4fc9ae797effb07302cb4f2dd48690 Author: Jeremy Harris Date: Thu Dec 28 20:51:28 2017 +0000 DKIM: tighter checking while parsing signature headers. Bug 2217 diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c index 20366a46..b884671d 100644 --- a/src/src/pdkim/pdkim.c +++ b/src/src/pdkim/pdkim.c @@ -490,7 +490,12 @@ for (p = raw_hdr; ; p++) if (c == ';' || c == '\0') { - if (cur_tag && cur_val) + /* We must have both tag and value, and tags must be one char except + for the possibility of "bh". */ + + if ( cur_tag && cur_val + && (cur_tag->ptr == 1 || *cur_tag->s == 'b') + ) { (void) string_from_gstring(cur_val); pdkim_strtrim(cur_val); @@ -500,8 +505,14 @@ for (p = raw_hdr; ; p++) switch (*cur_tag->s) { case 'b': - pdkim_decode_base64(cur_val->s, - cur_tag->s[1] == 'h' ? &sig->bodyhash : &sig->sighash); + switch (cur_tag->s[1]) + { + case '\0': pdkim_decode_base64(cur_val->s, &sig->sighash); break; + case 'h': if (cur_tag->ptr == 2) + pdkim_decode_base64(cur_val->s, &sig->bodyhash); + break; + default: break; + } break; case 'v': /* We only support version 1, and that is currently the