Re: opensmtpd-extra: simplify getaddrinfo() usage
[2024-01-24 00:09] Omar Polo
> On 2024/01/23 19:49:34 +0100, Philipp wrote:
> > [2024-01-23 11:39] Omar Polo
> > > spotted while reading Philipp' ldaps diff. it's really ugly to reach
> > > into the struct sockaddrs when using getaddrinfo()...
> >
> > Nice this makes the libtls integration simpler. Also some comments inline.
> >
> > > however, I don't use ldap so this could use at least some testing :)
> >
> > No problem I can test this.
>
> thanks :)
No problem. I'm currently working on a new mailserver, I need these changes
for. My current plan is to test all this tomorrow.
> even if we'll change the transport away from imsg, it could still be
> useful to improve these extras I think. (again, don't know much of ldap,
> nor can judge the state of tha table-ldap; just scratching an itch after
> taking a look at your diff.)
>
> > > @@ -85,8 +85,8 @@ ldap_connect(const char *addr)
> > > {
> > > struct aldap_url lu;
> > > struct addrinfo hints, *res0, *res;
> > > - char*buf;
> > > - int error, fd = -1;
> > > + char*buf, port[32];
> >
> > nitpick: the port is max 65535, so 8 byte would be enough.
>
> right, i picked the first power of two that came to mind. it could be
> interesting however to change the parsing function to keep the port as
> string, to avoid this conversion here.
Nice Idea, patch is attached.
> > [...]
> > > + if (connect(fd, res->ai_addr, res->ai_addrlen) == 0)
> > > + return aldap_init(fd);
> >
> > Here a aldap_free_url() is missing, therefor lu.buffer is leaking.
> > But currently aldap_free_url() is buggy, it frees lu->buffer and
> > lu->filter but this is the same object.
>
> Yes, but it's the same behaviour as before. I didn't want to change
> many things at a time.
>
> if you're interested in this however, we can also avoid the strdup()
> here since aldap_parse_url() already strdup()s the string for parsing
> (but still frees the passed argument...)
I have written two patches for this, one adding the free and one to
avoid the unnecessary strdup.
Ass you might guess from the filenames, there are a few more patches. I'll
send the rest after I have tested all my patches.
From fa4cdb0a74c3b5d17cdc93b6285d765fda084740 Mon Sep 17 00:00:00 2001
From: Philipp Takacs
Date: Wed, 24 Jan 2024 01:16:56 +0100
Subject: [PATCH 11/11] table-ldap aldap_parse_url now saves the port as string
---
extras/tables/table-ldap/aldap.c | 3 ++-
extras/tables/table-ldap/aldap.h | 6 +++---
extras/tables/table-ldap/table_ldap.c | 11 ++-
3 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/extras/tables/table-ldap/aldap.c b/extras/tables/table-ldap/aldap.c
index 5907c6e..7058d81 100644
--- a/extras/tables/table-ldap/aldap.c
+++ b/extras/tables/table-ldap/aldap.c
@@ -601,9 +601,10 @@ aldap_parse_url(const char *url, struct aldap_url *lu)
/* if a port is given */
if (*(forward2+1) != '\0') {
#define PORT_MAX UINT16_MAX
- lu->port = strtonum(++forward2, 0, PORT_MAX, );
+ strtonum(++forward2, 0, PORT_MAX, );
if (errstr)
goto fail;
+ lu->port = forward2;
}
} else {
lu->port = LDAP_PORT;
diff --git a/extras/tables/table-ldap/aldap.h b/extras/tables/table-ldap/aldap.h
index 088ee60..60159b4 100644
--- a/extras/tables/table-ldap/aldap.h
+++ b/extras/tables/table-ldap/aldap.h
@@ -19,9 +19,9 @@
#include "ber.h"
#define LDAP_URL "ldap://;
-#define LDAP_PORT 389
+#define LDAP_PORT "389"
#define LDAPS_URL "ldaps://"
-#define LDAPS_PORT 636
+#define LDAPS_PORT "636"
#define LDAP_PAGED_OID "1.2.840.113556.1.4.319"
struct aldap {
@@ -73,7 +73,7 @@ enum aldap_protocol {
struct aldap_url {
enum aldap_protocol protocol;
char *host;
- in_port_t port;
+ char *port;
char *dn;
#define MAXATTR 1024
char *attributes[MAXATTR];
diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c
index 0fb5fe3..3c6437a 100644
--- a/extras/tables/table-ldap/table_ldap.c
+++ b/extras/tables/table-ldap/table_ldap.c
@@ -118,7 +118,6 @@ ldap_connect(const char *addr)
{
struct aldap_url lu;
struct addrinfo hints, *res0, *res;
- char port[32];
int error, r, fd = -1;
if (aldap_parse_url(addr, ) != 1) {
@@ -126,22 +125,16 @@ ldap_connect(const char *addr)
return NULL;
}
- r = snprintf(port, sizeof(port), "%d", lu.port);
- if (r < 0 || (size_t)r >= sizeof(port)) {
- log_warnx("snprintf");
- return NULL;
- }
-
memset(, 0, sizeof(hints));
hints.ai_family = PF_UNSPEC;
hints.ai_socktype = SOCK_STREAM;
hints.ai_flags = AI_NUMERICSERV;
- error = getaddrinfo(lu.host, port, , );
+ error = getaddrinfo(lu.host, lu.port, , );
if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME)
return NULL;
if (error) {
log_warnx("warn: could not parse \"%s:%s\": %s", lu.host,
- port, gai_strerror(error));
+ lu.port, gai_strerror(error));
return NULL;
}
--
2.39.2
From
Re: need help with honeypot
чт, 2 нояб. 2023 г. в 09:51, kasak : > > пн, 23 окт. 2023 г. в 09:11, kasak : > > > > пт, 20 окт. 2023 г. в 19:52, Юрий Иванов : > > > > > > Sorry for off top... > > > ...but thanks for your OpenBSD PF book Peter :-) > > > > > > От: kasak > > > Отправлено: 20 октября 2023 г. 8:48 > > > Кому: Peter N. M. Hansteen > > > Копия: misc@opensmtpd.org > > > Тема: Re: need help with honeypot > > > > > > чт, 19 окт. 2023 г. в 19:52, Peter N. M. Hansteen : > > > > > > > > On Thu, Oct 19, 2023 at 09:05:56AM +0300, kasak wrote: > > > > > In traps file I have list of spoiled addresses for example > > > > > aa...@tvema.ru > > > > > But mail is not accepted :( > > > > > > > > This sounds like you are more or less trying to imitate the greytrapping > > > > feature of OpenBSD spamd. > > > > > > Yes, it is something similar, but not quite. I'm no need of > > > greytrapping, I want to teach rspamd. > > > Hope I can do this without spamd. > > > > > > > You might want to read this article of mine (gosh, it's been 11 years) > > > > and links therein for inspiration: > > > > https://bsdly.blogspot.com/2012/05/in-name-of-sane-email-setting-up-spamd.html > > > > (also newly available trackerless but with even uglier formatting as > > > > https://nxdomain.no/~peter/in_the_name_of_sane_email.html), > > > > assuming, as usual that your system runs OpenBSD (also applicable with > > > > minor adjustments on FreeBSD or NetBSD) > > > > > > > > - Peter > > > > > > > > -- > > > > Peter N. M. Hansteen, member of the first RFC 1149 implementation team > > > > https://bsdly.blogspot.com/ https://www.bsdly.net/ https://www.nuug.no/ > > > > "Remember to set the evil bit on all malicious network traffic" > > > > delilah spamd[29949]: 85.152.224.147: disconnected after 42673 seconds. > > > > > > > I did some experiment here, and looks like this is not "match" > > problem, this is "action" problem. > > If I change action "trap" to some real delivery method, it works. > > So, I need to fix it with rspamc somehow. > > If someone interested, I have figured out how to do what I wanted to do. > > Here is howto: > At first, collect spoiled email addresses to some list, for example > /etc/mail/traps. > Second, we must map these addresses to some local user, because > otherwize, smtpd will not know where to put them. > To do this, we must make another table, with mapping all addresses to > _rspamd user. > You can do this with a simple sed pattern: > cat /etc/mail/traps | sed 's/$/ _rspamd/' > /etc/mail/virtualtraps > Now we have two tables, traps for matching and virtualtraps for action. > > add something like this to smtpd.conf: > > table traps file:/etc/mail/traps > table virtualtraps file:/etc/mail/virtualtraps > > action "trap" mda "/usr/local/bin/rspamc -f 1 -w 10 fuzzy_add" virtual > > > match from any for rcpt-to action "trap" > > > The match directive should be placed above the main domain match. > And voila! > You can monitor teaching with grep: > grep -F -f /etc/mail/traps /var/log/maillog Folks, I just suddenly discovered that rspamd has a module to achieve exactly similar functionality. https://rspamd.com/doc/modules/spamtrap.html I already tested it and it works like a charm! How did I not notice it before?
Re: opensmtpd-extra: simplify getaddrinfo() usage
On 2024/01/23 19:49:34 +0100, Philipp wrote:
> [2024-01-23 11:39] Omar Polo
> > spotted while reading Philipp' ldaps diff. it's really ugly to reach
> > into the struct sockaddrs when using getaddrinfo()...
>
> Nice this makes the libtls integration simpler. Also some comments inline.
>
> > however, I don't use ldap so this could use at least some testing :)
>
> No problem I can test this.
thanks :)
even if we'll change the transport away from imsg, it could still be
useful to improve these extras I think. (again, don't know much of ldap,
nor can judge the state of tha table-ldap; just scratching an itch after
taking a look at your diff.)
> > @@ -85,8 +85,8 @@ ldap_connect(const char *addr)
> > {
> > struct aldap_url lu;
> > struct addrinfo hints, *res0, *res;
> > - char*buf;
> > - int error, fd = -1;
> > + char*buf, port[32];
>
> nitpick: the port is max 65535, so 8 byte would be enough.
right, i picked the first power of two that came to mind. it could be
interesting however to change the parsing function to keep the port as
string, to avoid this conversion here.
> [...]
> > + if (connect(fd, res->ai_addr, res->ai_addrlen) == 0)
> > + return aldap_init(fd);
>
> Here a aldap_free_url() is missing, therefor lu.buffer is leaking.
> But currently aldap_free_url() is buggy, it frees lu->buffer and
> lu->filter but this is the same object.
Yes, but it's the same behaviour as before. I didn't want to change
many things at a time.
if you're interested in this however, we can also avoid the strdup()
here since aldap_parse_url() already strdup()s the string for parsing
(but still frees the passed argument...)
Re: opensmtpd-extra: simplify getaddrinfo() usage
[2024-01-23 11:39] Omar Polo
> spotted while reading Philipp' ldaps diff. it's really ugly to reach
> into the struct sockaddrs when using getaddrinfo()...
Nice this makes the libtls integration simpler. Also some comments inline.
> however, I don't use ldap so this could use at least some testing :)
No problem I can test this.
> (would also be interesting to provide some more logging if socket/connect
> fails somehow, and also aldap_parse_url could use some simplifying.
> don't want to fall too much into this rabbit hole though)
>
> diff /home/op/w/opensmtpd-extras
> commit - 5715b1ff87eafd465592df5c2cf4b2f171e60bbc
> path + /home/op/w/opensmtpd-extras
> blob - 090cfb467a79c71c8d28ad9f75e6a0faf859cdd8
> file + extras/tables/table-ldap/table_ldap.c
> --- extras/tables/table-ldap/table_ldap.c
> +++ extras/tables/table-ldap/table_ldap.c
> @@ -85,8 +85,8 @@ ldap_connect(const char *addr)
> {
> struct aldap_url lu;
> struct addrinfo hints, *res0, *res;
> - char*buf;
> - int error, fd = -1;
> + char*buf, port[32];
nitpick: the port is max 65535, so 8 byte would be enough.
> + int error, r, fd = -1;
>
> if ((buf = strdup(addr)) == NULL)
> return NULL;
> @@ -98,37 +98,32 @@ ldap_connect(const char *addr)
> return NULL;
> }
>
> + r = snprintf(port, sizeof(port), "%d", lu.port);
> + if (r < 0 || (size_t)r >= sizeof(port)) {
> + log_warnx("snprintf");
> + return NULL;
> + }
> +
> memset(, 0, sizeof(hints));
> hints.ai_family = PF_UNSPEC;
> - hints.ai_socktype = SOCK_STREAM; /* DUMMY */
> - error = getaddrinfo(lu.host, NULL, , );
> + hints.ai_socktype = SOCK_STREAM;
> + hints.ai_flags = AI_NUMERICSERV;
> + error = getaddrinfo(lu.host, port, , );
> if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME)
> return NULL;
> if (error) {
> - log_warnx("warn: could not parse \"%s\": %s", lu.host,
> - gai_strerror(error));
> + log_warnx("warn: could not parse \"%s:%s\": %s", lu.host,
> + port, gai_strerror(error));
> return NULL;
> }
>
> for (res = res0; res; res = res->ai_next) {
> - if (res->ai_family != AF_INET && res->ai_family != AF_INET6)
> - continue;
> -
> fd = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
> if (fd == -1)
> continue;
>
> - if (res->ai_family == AF_INET) {
> - struct sockaddr_in sin4 = *(struct sockaddr_in
> *)res->ai_addr;
> - sin4.sin_port = htons(lu.port);
> - if (connect(fd, (struct sockaddr *),
> res->ai_addrlen) == 0)
> - return aldap_init(fd);
> - } else if (res->ai_family == AF_INET6) {
> - struct sockaddr_in6 sin6 = *(struct sockaddr_in6
> *)res->ai_addr;
> - sin6.sin6_port = htons(lu.port);
> - if (connect(fd, (struct sockaddr *),
> res->ai_addrlen) == 0)
> - return aldap_init(fd);
> - }
> + if (connect(fd, res->ai_addr, res->ai_addrlen) == 0)
> + return aldap_init(fd);
Here a aldap_free_url() is missing, therefor lu.buffer is leaking.
But currently aldap_free_url() is buggy, it frees lu->buffer and
lu->filter but this is the same object.
>
> close(fd);
> fd = -1;
>
Re: ldaps support for table-ldap
On 2024/01/23 11:26:59 +0100, Omar Polo wrote: > On 2024/01/23 01:24:57 +0100, Philipp wrote: > > Hi > > > > I have had a bit of time and implemented ldaps support for table-ldap. > > It is currently untested and has some todos. But I would say it's > > complete enough to share. So other can comment on the code. A patch > > is attached > > I don't use ldap and completely lack the experience with it, so I can > only provide some feedback on the code itself, not if it makes sense to > have TLS in here nor provide testing. > > > In general it would be nice, if the extras repo could get reactivated. > > Most imported would be some documentation for the existing staff. > > I agree with that sentiment. There's a lot of useful stuff in there. ah, forgot to Cc aisha. i remember a few diffs regarding ldap on tech@ ages ago. Pinging just in case they're still interested :)
opensmtpd-extra: simplify getaddrinfo() usage
spotted while reading Philipp' ldaps diff. it's really ugly to reach
into the struct sockaddrs when using getaddrinfo()...
however, I don't use ldap so this could use at least some testing :)
(would also be interesting to provide some more logging if socket/connect
fails somehow, and also aldap_parse_url could use some simplifying.
don't want to fall too much into this rabbit hole though)
diff /home/op/w/opensmtpd-extras
commit - 5715b1ff87eafd465592df5c2cf4b2f171e60bbc
path + /home/op/w/opensmtpd-extras
blob - 090cfb467a79c71c8d28ad9f75e6a0faf859cdd8
file + extras/tables/table-ldap/table_ldap.c
--- extras/tables/table-ldap/table_ldap.c
+++ extras/tables/table-ldap/table_ldap.c
@@ -85,8 +85,8 @@ ldap_connect(const char *addr)
{
struct aldap_url lu;
struct addrinfo hints, *res0, *res;
- char*buf;
- int error, fd = -1;
+ char*buf, port[32];
+ int error, r, fd = -1;
if ((buf = strdup(addr)) == NULL)
return NULL;
@@ -98,37 +98,32 @@ ldap_connect(const char *addr)
return NULL;
}
+ r = snprintf(port, sizeof(port), "%d", lu.port);
+ if (r < 0 || (size_t)r >= sizeof(port)) {
+ log_warnx("snprintf");
+ return NULL;
+ }
+
memset(, 0, sizeof(hints));
hints.ai_family = PF_UNSPEC;
- hints.ai_socktype = SOCK_STREAM; /* DUMMY */
- error = getaddrinfo(lu.host, NULL, , );
+ hints.ai_socktype = SOCK_STREAM;
+ hints.ai_flags = AI_NUMERICSERV;
+ error = getaddrinfo(lu.host, port, , );
if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME)
return NULL;
if (error) {
- log_warnx("warn: could not parse \"%s\": %s", lu.host,
- gai_strerror(error));
+ log_warnx("warn: could not parse \"%s:%s\": %s", lu.host,
+ port, gai_strerror(error));
return NULL;
}
for (res = res0; res; res = res->ai_next) {
- if (res->ai_family != AF_INET && res->ai_family != AF_INET6)
- continue;
-
fd = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
if (fd == -1)
continue;
- if (res->ai_family == AF_INET) {
- struct sockaddr_in sin4 = *(struct sockaddr_in
*)res->ai_addr;
- sin4.sin_port = htons(lu.port);
- if (connect(fd, (struct sockaddr *),
res->ai_addrlen) == 0)
- return aldap_init(fd);
- } else if (res->ai_family == AF_INET6) {
- struct sockaddr_in6 sin6 = *(struct sockaddr_in6
*)res->ai_addr;
- sin6.sin6_port = htons(lu.port);
- if (connect(fd, (struct sockaddr *),
res->ai_addrlen) == 0)
- return aldap_init(fd);
- }
+ if (connect(fd, res->ai_addr, res->ai_addrlen) == 0)
+ return aldap_init(fd);
close(fd);
fd = -1;
Re: ldaps support for table-ldap
On 2024/01/23 01:24:57 +0100, Philipp wrote:
> Hi
>
> I have had a bit of time and implemented ldaps support for table-ldap.
> It is currently untested and has some todos. But I would say it's
> complete enough to share. So other can comment on the code. A patch
> is attached
I don't use ldap and completely lack the experience with it, so I can
only provide some feedback on the code itself, not if it makes sense to
have TLS in here nor provide testing.
> In general it would be nice, if the extras repo could get reactivated.
> Most imported would be some documentation for the existing staff.
I agree with that sentiment. There's a lot of useful stuff in there.
> From 2a2671ea2a88868ec91e24803db30935d18c081d Mon Sep 17 00:00:00 2001
> From: Philipp Takacs
> Date: Tue, 23 Jan 2024 00:55:23 +0100
> Subject: [PATCH] table-ldap add ldaps support
>
> untested
>
> based on libtls, autohell is ugly ass hell
don't worry about the autoconf/automake part of this. I can fix it as a
follow-up eventually (still have to take a proper look at how this repo
is built.)
> need also some log messages
> ---
> configure.ac | 2 +-
> extras/tables/table-ldap/aldap.c | 32 ++---
> extras/tables/table-ldap/aldap.h | 20 ++-
> extras/tables/table-ldap/ber.c| 23 ++---
> extras/tables/table-ldap/ber.h| 1 +
> extras/tables/table-ldap/table_ldap.c | 49 ---
> 6 files changed, 103 insertions(+), 24 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 410a61b..14608b1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -577,7 +577,7 @@ AC_ARG_WITH([libssl],
> ]
> )
> ## XXX chl -lssl manually added
> -LIBS="-lcrypto -lssl $LIBS"
> +LIBS="-lcrypto -lssl -ltls $LIBS"
> AC_TRY_LINK_FUNC([RAND_add], [AC_DEFINE([HAVE_OPENSSL], [1],
> [Define if your ssl headers are included
> with #include ])],
> diff --git a/extras/tables/table-ldap/aldap.c
> b/extras/tables/table-ldap/aldap.c
> index d54a90c..9cefe1c 100644
> --- a/extras/tables/table-ldap/aldap.c
> +++ b/extras/tables/table-ldap/aldap.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
nit: I'd keep the include sorted when possible, so tls.h should go
between stdlib.h and unistd.h
> #include "aldap.h"
>
> @@ -55,6 +56,12 @@ voidldap_debug_elements(struct
> ber_element *);
> int
> aldap_close(struct aldap *al)
> {
> + if (al->ber.tls_ctx) {
> + if (tls_close(al->ber.tls_ctx) == -1)
> + return (-1);
> + tls_free(al->ber.tls_ctx);
> + }
> +
> if (close(al->ber.fd) == -1)
> return (-1);
>
> @@ -65,13 +72,14 @@ aldap_close(struct aldap *al)
> }
>
> struct aldap *
> -aldap_init(int fd)
> +aldap_init(int fd, struct tls *ctx)
> {
> struct aldap *a;
>
> if ((a = calloc(1, sizeof(*a))) == NULL)
> return NULL;
> a->ber.fd = fd;
> + a->ber.tls_ctx = ctx;
>
> return a;
> }
> @@ -575,10 +583,15 @@ aldap_parse_url(char *url, struct aldap_url *lu)
> p = lu->buffer;
>
> /* protocol */
> - if (strncasecmp(LDAP_URL, p, strlen(LDAP_URL)) != 0)
> + if (strncasecmp(LDAP_URL, p, strlen(LDAP_URL)) == 0) {
> + lu->protocol = LDAP;
> + p += strlen(LDAP_URL);
> + } else if (strncasecmp(LDAPS_URL, p, strlen(LDAP_URL)) == 0) {
> + lu->protocol = LDAPS;
> + p += strlen(LDAPS_URL);
> + } else {
> goto fail;
> - lu->protocol = LDAP;
> - p += strlen(LDAP_URL);
> + }
>
> /* host and optional port */
> if ((forward = strchr(p, '/')) != NULL)
> @@ -594,7 +607,16 @@ aldap_parse_url(char *url, struct aldap_url *lu)
> goto fail;
> }
> } else {
> - lu->port = LDAP_PORT;
nit: a switch here is a bit over overly-verbose. just
lu->port = LDAP_PORT;
if (lu->protocol == LDAPS)
lp->port = LDAPS_PORT;
should do it.
> + switch (lu->protocol) {
> + case LDAP:
> + lu->port = LDAP_PORT;
> + break;
> + case LDAPS:
> + lu->port = LDAPS_PORT;
> + break;
> + default:
> + goto fail;
> + }
> }
> /* fail if no host is given */
> if (strlen(p) == 0)
> diff --git a/extras/tables/table-ldap/aldap.h
> b/extras/tables/table-ldap/aldap.h
> index 7cfd637..fec106b 100644
> --- a/extras/tables/table-ldap/aldap.h
> +++ b/extras/tables/table-ldap/aldap.h
> @@ -20,6 +20,8 @@
>
> #define LDAP_URL "ldap://;
> #define LDAP_PORT 389
> +#define LDAPS_URL "ldaps://"
> +#define LDAPS_PORT 636
> #define LDAP_PAGED_OID "1.2.840.113556.1.4.319"
>
> struct aldap {
> @@ -69,15 +71,15 @@ enum aldap_protocol {
> };
>
