Re: Submission service lookup support

2022-09-08 Thread Wietse Venema
Tomas Korbar:
> Hi guys,
> I created a new version of this feature which does not include
> changes in connection management code.
> Furthermore this new version adds configuration parameter
> "-o smtp_srv_resolution_allowed=yes" which enables it, so
> only users interested in SRV resolution can use it.
> 
> Thanks for any review or help that you can provide.

Thanks, I'll check it out.

Wietse

> On Tue, Aug 9, 2022 at 1:53 PM Tomas Korbar  wrote:
> >
> > Hi guys,
> > Thanks for your opinions and hints. I will try to come up with
> > a implementation that does not involve changes in
> > Postfix connection management code. smtp process was
> > the first place where I thought that this feature could be
> > implemented.
> >
> > On Tue, Aug 9, 2022 at 11:56 AM Wietse Venema  wrote:
> > >
> > > Viktor Dukhovni:
> > > > On Mon, Aug 08, 2022 at 05:06:22PM -0400, Viktor Dukhovni wrote:
> > > >
> > > > > > We're discussing support for an MUA-specific feature, not 
> > > > > > high-volime
> > > > > > MTA-to-MTA support. Connection reuse is less important, as long as
> > > > > > Postfix does not mix traffic with different authentication 
> > > > > > properties,
> > > > > > and that is what SMTP_HOST_KEY is for. So if sharing is a consern,
> > > > > > just add a "comes from SRV lookup" flag to the connection cache
> > > > > > lookup key.
> > > > > >
> > > > > > > Are keys along the lines of "domain:submission+srv" too clumsy?
> > > > >
> > > > > I meant TLS policy lookup keys (smtp_tls_policy_maps).  The session 
> > > > > and
> > > > > connection caches are already fine, since transport name is part of 
> > > > > the
> > > > > cache key.
> > > >
> > > > Also, for the caches, in addition to not getting false positives from
> > > > imprecise keys, we presumably actually want to get cache hits on the
> > > > logical destination for connection reuse, which is less likely to happen
> > > > if it splits into multiple separate nexthop values.
> > >
> > > Seriously, this is MUA submission, we don't need to optimize
> > > connection reuse for that.
> > >
> > > > And perhaps reuse may not be appropriate when the logical nexthop
> > > > destinations have different TLS policies, or different SASL settings,
> > > > ... and yet share underlying submission servers.
> > >
> > > Some kind of grouping metadata can take care of that.
> > >
> > > Wietse
> > >

[ Attachment, skipping... ]


Re: Submission service lookup support

2022-09-08 Thread Tomas Korbar
Hi guys,
I created a new version of this feature which does not include
changes in connection management code.
Furthermore this new version adds configuration parameter
"-o smtp_srv_resolution_allowed=yes" which enables it, so
only users interested in SRV resolution can use it.

Thanks for any review or help that you can provide.

On Tue, Aug 9, 2022 at 1:53 PM Tomas Korbar  wrote:
>
> Hi guys,
> Thanks for your opinions and hints. I will try to come up with
> a implementation that does not involve changes in
> Postfix connection management code. smtp process was
> the first place where I thought that this feature could be
> implemented.
>
> On Tue, Aug 9, 2022 at 11:56 AM Wietse Venema  wrote:
> >
> > Viktor Dukhovni:
> > > On Mon, Aug 08, 2022 at 05:06:22PM -0400, Viktor Dukhovni wrote:
> > >
> > > > > We're discussing support for an MUA-specific feature, not high-volime
> > > > > MTA-to-MTA support. Connection reuse is less important, as long as
> > > > > Postfix does not mix traffic with different authentication properties,
> > > > > and that is what SMTP_HOST_KEY is for. So if sharing is a consern,
> > > > > just add a "comes from SRV lookup" flag to the connection cache
> > > > > lookup key.
> > > > >
> > > > > > Are keys along the lines of "domain:submission+srv" too clumsy?
> > > >
> > > > I meant TLS policy lookup keys (smtp_tls_policy_maps).  The session and
> > > > connection caches are already fine, since transport name is part of the
> > > > cache key.
> > >
> > > Also, for the caches, in addition to not getting false positives from
> > > imprecise keys, we presumably actually want to get cache hits on the
> > > logical destination for connection reuse, which is less likely to happen
> > > if it splits into multiple separate nexthop values.
> >
> > Seriously, this is MUA submission, we don't need to optimize
> > connection reuse for that.
> >
> > > And perhaps reuse may not be appropriate when the logical nexthop
> > > destinations have different TLS policies, or different SASL settings,
> > > ... and yet share underlying submission servers.
> >
> > Some kind of grouping metadata can take care of that.
> >
> > Wietse
> >
commit 222deebf825e12f7620abfbe84e94f47831e34cd
Author: Tomas Korbar 
Date:   Thu Sep 1 15:32:02 2022 +0200

Implement SRV resolution feature

diff --git a/src/dns/dns.h b/src/dns/dns.h
index 5f53dbc..84f1f6b 100644
--- a/src/dns/dns.h
+++ b/src/dns/dns.h
@@ -158,7 +158,9 @@ typedef struct DNS_RR {
 unsigned short class;		/* C_IN, etc. */
 unsigned int ttl;			/* always */
 unsigned int dnssec_valid;		/* DNSSEC validated */
-unsigned short pref;		/* T_MX only */
+unsigned short pref;		/* T_MX and T_SRV record related */
+unsigned short weight;	/* T_SRV related, defined in rfc2782 */
+unsigned short port;		/* T_SRV related, defined in rfc2782 */
 struct DNS_RR *next;		/* linkage */
 size_t  data_len;			/* actual data size */
 chardata[1];			/* actually a bunch of data */
@@ -186,11 +188,13 @@ extern char *dns_strrecord(VSTRING *, DNS_RR *);
 extern DNS_RR *dns_rr_create(const char *, const char *,
 			 ushort, ushort,
 			 unsigned, unsigned,
+			 unsigned, unsigned,
 			 const char *, size_t);
 extern void dns_rr_free(DNS_RR *);
 extern DNS_RR *dns_rr_copy(DNS_RR *);
 extern DNS_RR *dns_rr_append(DNS_RR *, DNS_RR *);
 extern DNS_RR *dns_rr_sort(DNS_RR *, int (*) (DNS_RR *, DNS_RR *));
+extern DNS_RR *dns_srv_rr_sort(DNS_RR *);
 extern int dns_rr_compare_pref_ipv6(DNS_RR *, DNS_RR *);
 extern int dns_rr_compare_pref_ipv4(DNS_RR *, DNS_RR *);
 extern int dns_rr_compare_pref_any(DNS_RR *, DNS_RR *);
@@ -295,6 +299,7 @@ extern int dns_get_h_errno(void);
   * Below is the precedence order. The order between DNS_RETRY and DNS_NOTFOUND
   * is arbitrary.
   */
+#define DNS_NULLSRV	(-8)		/* query ok, submission service unavailable */
 #define DNS_RECURSE	(-7)		/* internal only: recursion needed */
 #define DNS_NOTFOUND	(-6)		/* query ok, data not found */
 #define DNS_NULLMX	(-5)		/* query ok, service unavailable */
diff --git a/src/dns/dns_lookup.c b/src/dns/dns_lookup.c
index 1c12a88..b7029a8 100644
--- a/src/dns/dns_lookup.c
+++ b/src/dns/dns_lookup.c
@@ -740,6 +740,8 @@ static int dns_get_rr(DNS_RR **list, const char *orig_name, DNS_REPLY *reply,
 int comp_len;
 ssize_t data_len;
 unsigned pref = 0;
+unsigned weight = 0;
+unsigned port = 0;
 unsigned char *src;
 unsigned char *dst;
 int ch;
@@ -765,6 +767,18 @@ static int dns_get_rr(DNS_RR **list, const char *orig_name, DNS_REPLY *reply,
 	return (DNS_INVAL);
 	data_len = strlen(temp) + 1;
 	break;
+	case T_SRV:
+	GETSHORT(pref, pos);
+	GETSHORT(weight, pos);
+	GETSHORT(port, pos);
+	if (dn_expand(reply->buf, reply->end, pos, temp, sizeof(temp)) < 0)
+	return (DNS_RETRY);
+	if (*temp == 0)
+	return (DNS_NULLSRV);
+	if (!valid_rr_name(temp, "resource data", fixed->type, reply))