On 10/5/20 6:15 PM, Wietse Venema wrote:
Demi M. Obenour:There was a recent vulnerability in OpenBSD due to libc malfunctioning in a set-uid-root program under very low resource limits. I would prefer to minimize the amount of third-party libraries that are used by postdrop. That said, another option would be to error out if the resource limits are below what we consider a reasonable minimum.Another good reason for not using set-uid root programs...
Indeed.
I don't think it is practical for Postfix to have universal minimal resource limts. You can add some custom OPENBSD code later.
That makes sense. The OpenBSD vulnerability has been fixed, and was merely used as an example. No OpenBSD-specific code will be needed, at least not for this purpose.
Surprise: Postfix has a strip_addr() function that can remove adress extensions before enforcing the ACL.
Good to know! That proved critical.
Is the code in smtpd_check.c a good place to start?Yes. It also helps you to become familiar with Postfix's approach to parsing.
Indeed it was helpful. Thanks for the tip! Patch (made against 3.5.7) attached. I lightly tested it locally and it seems to work, but there could very well be bugs. I am virtually certain that I violated the Postfix coding style somewhere, sorry. I can also send the patch inline if you prefer. For what it is worth, I found the Postfix source code to be very clean and easy to read. Writing this patch probably took about four hours of work, which is significantly less than I expected for a non-trivial feature. Thank you for all the work you have put into Postfix!
Wietse
Thank you, Demi
diff -ur postfix-3.5.7-old/src/global/mail_params.h postfix-3.5.7/src/global/mail_params.h
--- postfix-3.5.7-old/src/global/mail_params.h 2020-05-09 11:51:27.000000000 -0400
+++ postfix-3.5.7/src/global/mail_params.h 2020-10-05 18:10:32.641000000 -0400
@@ -1667,6 +1667,10 @@
#define DEF_SMTPD_SASL_TYPE DEF_SERVER_SASL_TYPE
extern char *var_smtpd_sasl_type;
+#define VAR_LOCAL_SND_AUTH_MAPS "local_sender_login_maps"
+#define DEF_LOCAL_SND_AUTH_MAPS "static:*"
+extern char *var_local_snd_auth_maps;
+
#define VAR_SMTPD_SND_AUTH_MAPS "smtpd_sender_login_maps"
#define DEF_SMTPD_SND_AUTH_MAPS ""
extern char *var_smtpd_snd_auth_maps;
diff -ur postfix-3.5.7-old/src/postdrop/postdrop.c postfix-3.5.7/src/postdrop/postdrop.c
--- postfix-3.5.7-old/src/postdrop/postdrop.c 2019-10-13 11:32:18.000000000 -0400
+++ postfix-3.5.7/src/postdrop/postdrop.c 2020-10-05 18:48:02.078000000 -0400
@@ -147,6 +147,10 @@
#include <rec_attr_map.h>
#include <mail_parm_split.h>
#include <maillog_client.h>
+#include <maps.h>
+#include <mail_addr_find.h>
+#include <strip_addr.h>
+#include <mypwd.h>
/* Application-specific. */
@@ -167,9 +171,10 @@
* Local mail submission access list.
*/
char *var_submit_acl;
-
+char *var_local_snd_auth_maps;
static const CONFIG_STR_TABLE str_table[] = {
VAR_SUBMIT_ACL, DEF_SUBMIT_ACL, &var_submit_acl, 0, 0,
+ VAR_LOCAL_SND_AUTH_MAPS, DEF_LOCAL_SND_AUTH_MAPS, &var_local_snd_auth_maps, 0, 0,
0,
};
@@ -231,6 +236,7 @@
int c;
VSTRING *buf;
int status;
+ MAPS *local_sender_login_maps;
MAIL_STREAM *dst;
int rec_type;
static char *segment_info[] = {
@@ -239,6 +245,7 @@
char **expected;
uid_t uid = getuid();
ARGV *import_env;
+ struct mypasswd *username;
const char *error_text;
char *attr_name;
char *attr_value;
@@ -316,16 +323,6 @@
get_mail_conf_str_table(str_table);
/*
- * Mail submission access control. Should this be in the user-land gate,
- * or in the daemon process?
- */
- mail_dict_init();
- if ((errstr = check_user_acl_byuid(VAR_SUBMIT_ACL, var_submit_acl,
- uid)) != 0)
- msg_fatal("User %s(%ld) is not allowed to submit mail",
- errstr, (long) uid);
-
- /*
* Stop run-away process accidents by limiting the queue file size. This
* is not a defense against DOS attack.
*/
@@ -369,11 +366,34 @@
/* End of initializations. */
/*
+ * Mail submission access control. Should this be in the user-land gate,
+ * or in the daemon process?
+ */
+ mail_dict_init();
+ if ((errstr = check_user_acl_byuid(VAR_SUBMIT_ACL, var_submit_acl,
+ uid)) != 0)
+ msg_fatal("User %s(%ld) is not allowed to submit mail",
+ errstr, (long) uid);
+
+ local_sender_login_maps = maps_create(VAR_SMTPD_SND_AUTH_MAPS,
+ var_local_snd_auth_maps,
+ DICT_FLAG_FOLD_FIX |
+ DICT_FLAG_UTF8_REQUEST |
+ DICT_FLAG_NO_PROXY |
+ DICT_FLAG_NO_UNAUTH);
+
+ /*
* Don't trust the caller's time information.
*/
GETTIMEOFDAY(&start);
/*
+ * Get the username.
+ */
+ if ((username = mypwuid(uid)) == NULL)
+ msg_fatal("Cannot get username for uid %ld", (long) uid);
+
+ /*
* Create queue file. mail_stream_file() never fails. Send the queue ID
* to the caller. Stash away a copy of the queue file name so we can
* clean up in case of a fatal error or an interrupt.
@@ -429,8 +449,34 @@
if (rec_type == REC_TYPE_TIME)
continue;
/* Check these at submission time instead of pickup time. */
- if (rec_type == REC_TYPE_FROM)
+ if (rec_type == REC_TYPE_FROM) {
+ const char *owners;
+ char *saved_owners, *cp, *name, *stripped, *stripped_copy;
+ int found = 0;
+
+ if (vstring_memchr(buf, '\0') != NULL)
+ msg_fatal("uid=%ld: NUL in FROM", (long) uid);
+ if ((owners = mail_addr_find(local_sender_login_maps, username->pw_name, NULL)) == NULL)
+ msg_fatal("uid=%ld: cannot check ownership", (long) uid);
+ if (strcmp(owners, "*") == 0)
+ goto allow_all;
+ if ((stripped = stripped_copy = strip_addr_internal(vstring_str(buf), NULL, "+")) == NULL)
+ stripped = vstring_str(buf);
+ cp = saved_owners = mystrdup(owners);
+ while ((name = mystrtok(&saved_owners, CHARS_COMMA_SP)) != 0) {
+ if (strcmp(stripped, name) == 0) {
+ found = 1;
+ break;
+ }
+ }
+ if (!found)
+ msg_fatal("uid=%ld: does not own address %s", (long) uid, vstring_str(buf));
+ if (stripped_copy != NULL)
+ myfree(stripped_copy);
+ myfree(cp);
+allow_all:
from_count++;
+ }
if (rec_type == REC_TYPE_RCPT)
rcpt_count++;
/* Limit the attribute types that users may specify. */
OpenPGP_0xB288B55FFF9C22C1.asc
Description: application/pgp-keys
OpenPGP_signature
Description: OpenPGP digital signature
