Helo, Omar
Thank you for the patch!
• Omar Polo [2023-12-14 20:01]:
[moving to tech@]
On 2023/12/13 20:37:09 +0100, Kirill Miazine <k...@krot.org> wrote:
I've spent several hours debugging an issue.
table(5) specifies addrname format as a mapping from inet4 or inet6
addresses to hostnames:
::1 localhost
127.0.0.1 localhost
88.190.23.165 www.opensmtpd.org
But I can't get IPv6 mappings to work. So I've given up, and use helo
instead of helo-src.
But helo-src is useful (not on this particular setup, though).
For a month or so I had a static addrname table like this:
table helo-names { \
65.108.153.125 = com.krot.org, \
2a01:4f9:c010:9411::1 = com.krot.org
}
I suppose this was the cause of the frequent "Failed to retrieve helo
string" errors I was getting -- smtpd would not get helo name for IPv6
address.
At first I thought it was space around equal signs, but table(5) uses
space in the example. So I had to dig further.
Doing tracing, I saw that for IPv6 it -- apparently -- would be looking
up IPv6 address enclosed in brackets:
lookup: lookup "[2a01:4f9:c010:9411::1]" as ADDRNAME in table
static:helo-names -> none
So, I went ahead and did:
table helo-names { \
65.108.153.125 = com.krot.org, \
"[2a01:4f9:c010:9411::1]" = com.krot.org \
}
(by the way, in -current the \ are no longer needed.)
This didn't help, either:
lookup: lookup "[2a01:4f9:c010:9411::1]" as ADDRNAME in table
static:helo-names -> "com.krot.org"
warn: failure during helo lookup helo-names:[2a01:4f9:c010:9411::1]
Now it did find "com.krot.org", but why did it report "failure during
helo lookup"? It found a match, but still reported failure...
The issue is that smtpd fails to parse the address.
First, the sockaddr is turned into a string using sa_to_text() that
wraps ipv6 addresses in brackets. The resulting string is then searched
in the table and (if found) passed to inet_pton to get back a sockaddr,
but the [] are left in there, and so it fails.
I don't think it's safe to change sa_to_text() to not wrap ipv6
addresses in brackets, but we can adjust parse_sockaddr().
See diff below.
Thinking that it could be a static-map issue, I did a file: map. First
as documented in the man page:
2a01:4f9:c010:9411::1 com.krot.org
65.108.153.125 com.krot.org
This didn't work:
lookup: lookup "[2a01:4f9:c010:9411::1]" as ADDRNAME in table
static:helo-names -> none
Then with square brackets:
65.108.153.125 com.krot.org
[2a01:4f9:c010:9411::1] com.krot.org
And this time square bracked didn't help.
lookup: lookup "[2a01:4f9:c010:9411::1]" as ADDRNAME in table
static:helo-names -> none
New try with db:
makemap -d hash -o helo-names.db -t set helo-names
When reading makemap(8), I noticed this:
In all cases, makemap reads lines consisting of words separated by
whitespace. The first word of a line is the database key; the remainder
represents the mapped value. The database key and value may optionally
be separated by the colon character.
Colon character! In makemap.c there's:
strsep(&valp, " \t:");
So it would be splitting IPv6 addresses, IIUC.
table(5) doesn't say anything about colons.
So I did a dump, and it indeed does separate the IPv6 address:
# makemap -U helo-names.db
65.108.153.125 com.krot.org
[2a01 4f9:c010:9411::1] com.krot.org
How would one put IPv6 addresses in such a map?
I don't think makemap as-is supports ipv6 addresses, but this should at
least fix the usage in other tables.
Can you please test it and report back if it works? I have to admit I
failed to test helo-src locally so I hacked up something in smtpd.c to
perform a lookup in order to test this.
(I *believe* the diff should apply to -portable as well as-is.)
cvs checkout took looong time, so I took src.tar.gz from 7.4, applied
patch there.
diff worked for static maps, at least. tested on this config:
<config>
table local-interfaces { ::1 127.0.0.1 }
table helo-names { \
127.0.0.1 = localhost, \
::1 = localhost, \
"[::1]" = localhost, \
ipv6:::1 = localhost \
}
listen on socket
listen on lo0
action mail_relay relay host smtp://[::1] \
src <local-interfaces> \
helo-src <helo-names>
match from local for any action mail_relay
</config>
from logs:
mproc: dispatcher -> lka : 56 IMSG_MTA_LOOKUP_HELO
imsg: lka <- dispatcher: IMSG_MTA_LOOKUP_HELO (len=56)
lookup: lookup "[::1]" as ADDRNAME in table static:helo-names -> "localhost"
mproc: lka -> dispatcher : 23 IMSG_MTA_LOOKUP_HELO
imsg: dispatcher <- lka: IMSG_MTA_LOOKUP_HELO (len=23)
[...]
d147a0751842134 smtp connected address=[::1] host=localhost.localdomain
smtp: 0x1d0c9690000: >>> 220 build ESMTP OpenSMTPD
smtp: 0x1d0c9690000: IO_LOWAT <io:0x1cff3822e00 fd=13 to=300000 fl=W
ib=0 ob=0>
mta: 0x1d084c9d6d0: IO_DATAIN <io:0x1d084c82d00 fd=12 to=300000 fl=R
ib=27 ob=0>
mta: 0x1d084c9d6d0: <<< 220 build ESMTP OpenSMTPD
mta: 0x1d084c9d6d0: MTA_BANNER -> MTA_EHLO
mta: 0x1d084c9d6d0: >>> EHLO localhost
so this is good!
but didn't work with "table helo-names file:/etc/mail/helo-names",
however, which contained:
127.0.0.1 localhost1
::1 localhost2
[::1] localhost3
ipv6:::1 localhost4
with file based map logs showed:
imsg: lka <- dispatcher: IMSG_MTA_LOOKUP_HELO (len=56)
lookup: lookup "[::1]" as ADDRNAME in table static:helo-names -> none
mproc: lka -> dispatcher : 12 IMSG_MTA_LOOKUP_HELO
imsg: dispatcher <- lka: IMSG_MTA_LOOKUP_HELO (len=12)
when I did delivery to 127.0.0.1, file based map returned proper name:
mproc: dispatcher -> lka : 44 IMSG_MTA_LOOKUP_HELO
imsg: lka <- dispatcher: IMSG_MTA_LOOKUP_HELO (len=44)
lookup: lookup "127.0.0.1" as ADDRNAME in table static:helo-names ->
"localhost1"
mproc: lka -> dispatcher : 24 IMSG_MTA_LOOKUP_HELO
imsg: dispatcher <- lka: IMSG_MTA_LOOKUP_HELO (len=24)
diff /tmp/smtpd
commit - 59829af3c4da38e511c4f8e3e4a38e45fcf3b082
path + /tmp/smtpd
blob - 7328cf5df6eb0034e9bb5c91b59c74bd3d402db4
file + table.c
--- table.c
+++ table.c
@@ -628,7 +628,8 @@ parse_sockaddr(struct sockaddr *sa, int family, const
struct in6_addr in6a;
struct sockaddr_in *sin;
struct sockaddr_in6 *sin6;
- char *cp, *str2;
+ char *cp;
+ char addr[NI_MAXHOST];
const char *errstr;
switch (family) {
@@ -649,23 +650,21 @@ parse_sockaddr(struct sockaddr *sa, int family, const
return (0);
case PF_INET6:
- if (strncasecmp("ipv6:", str, 5) == 0)
+ if (*str == '[')
+ str++;
+ if (!strncasecmp("ipv6:", str, 5))
str += 5;
- cp = strchr(str, SCOPE_DELIMITER);
- if (cp) {
- str2 = strdup(str);
- if (str2 == NULL)
- return (-1);
- str2[cp - str] = '\0';
- if (inet_pton(PF_INET6, str2, &in6a) != 1) {
- free(str2);
- return (-1);
- }
- cp++;
- free(str2);
- } else if (inet_pton(PF_INET6, str, &in6a) != 1)
+
+ if (strlcpy(addr, str, sizeof(addr)) >= sizeof(addr))
return (-1);
+ if ((cp = strchr(addr, ']')) != NULL)
+ *cp = '\0';
+ if ((cp = strchr(addr, SCOPE_DELIMITER)) != NULL)
+ *cp++ = '\0';
+ if (inet_pton(PF_INET6, addr, &in6a) != 1)
+ return (-1);
+
sin6 = (struct sockaddr_in6 *)sa;
memset(sin6, 0, sizeof *sin6);
sin6->sin6_len = sizeof(struct sockaddr_in6);
