On 11.11.2016 12:22, Arkadiusz Miśkiewicz wrote:
On Friday 11 of November 2016, Felipe Gasper wrote:
Hello,
We’re rolling out large SNI deployments for our mail servers. Each
domain
gets an entry like this in the config:
local_name mail.foo.com {
ssl_cert = </ssl/domain_tls/*.foo.com/combined
ssl_key = </ssl/domain_tls/*.foo.com/combined
}
Lack of glob/regexp support here is also a problem (for me). I could have 50%
smaller config if local_name supported regexp matching, so it would be
possible to do:
local_name ^(pop3|imap)\.foo\.com {
...
}
or even with glob like *.foo.com matching.
There are a couple problems we’re finding with this approach:
1) Dovecot wants to load everything at once, which has some machines taking
up many GiB of memory just for Dovecot. Is there any way to defer loading
of an SSL cert until a client actually requests it?
No - thread here http://www.dovecot.org/list/dovecot/2016-October/105855.html
Memory is one thing.
The other is that dovecot stops accepting clients when huge config reload
happens (I guess it's a design problem since it makes no sense to do that in
any case. Clients should be processed without gap using old config until new
config is loaded and ready to go).
And third problem is that there is hardcoded 10s limit for reloading which in
case thousands of certificates is way too short limit. Anyway if you hit that
limit it's already lost case due to earlier problem.
2) Any time we add or remove a domain, Dovecot’s SNI config matrix needs to
be rebuilt. Is there a way to handle SNI requests dynamically via some
sort of configuration plugin, so we wouldn’t need to rebuild the config on
domain add/remove? I looked through the docs but couldn’t see a way to do
this.
That's unavoidable for now :-(
Here we started analyzing maillog and put into dovecot config only these ssl
certs for domains that are actually used with TLS. It's very ugly and short-
sighted approach but hopefuly proper solution will be implemented by dovecot
team before all people start to use TLS.
Thank you in advance!
-Felipe Gasper
Mississauga, ON
If you are interested in testing, please find patch attached that allows
you to specify
local_name *.foo.bar {
}
or
local_name *.*.foo.bar {
}
so basically you can now use certificate name matching rules for
local_name. It made most sense.
This should apply cleanly to 2.2.26.0.
---
Aki Tuomi
Dovecot oy
>From 1016a956f6fb8a14e5d24372dc8ed3615b2ecc10 Mon Sep 17 00:00:00 2001
From: Aki Tuomi <[email protected]>
Date: Fri, 11 Nov 2016 13:13:29 +0200
Subject: [PATCH 1/3] lib-dns: Add DNS specific matching algorithms
RFC4343 and RFCRFC4592 compare and match algorithms
---
src/lib-dns/Makefile.am | 6 ++--
src/lib-dns/dns-util.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
src/lib-dns/dns-util.h | 36 +++++++++++++++++++++
3 files changed, 125 insertions(+), 2 deletions(-)
create mode 100644 src/lib-dns/dns-util.c
create mode 100644 src/lib-dns/dns-util.h
diff --git a/src/lib-dns/Makefile.am b/src/lib-dns/Makefile.am
index 751860a..4a0e8bc 100644
--- a/src/lib-dns/Makefile.am
+++ b/src/lib-dns/Makefile.am
@@ -4,10 +4,12 @@ AM_CPPFLAGS = \
-I$(top_srcdir)/src/lib
libdns_la_SOURCES = \
- dns-lookup.c
+ dns-lookup.c \
+ dns-util.c
headers = \
- dns-lookup.h
+ dns-lookup.h \
+ dns-util.h
pkginc_libdir=$(pkgincludedir)
pkginc_lib_HEADERS = $(headers)
diff --git a/src/lib-dns/dns-util.c b/src/lib-dns/dns-util.c
new file mode 100644
index 0000000..3ed7244
--- /dev/null
+++ b/src/lib-dns/dns-util.c
@@ -0,0 +1,85 @@
+/* Copyright (c) 2010-2016 Dovecot authors, see the included COPYING file */
+#include "lib.h"
+#include "dns-util.h"
+
+/**
+ return first position from b->a of c or a if not found
+ */
+static inline
+const char *strchr_ba(const char *a, const char *b, char c)
+{
+ for(;b>a && *b != c; b--);
+ return b;
+}
+
+int dns_ncompare(const char *a, const char *b, size_t n)
+{
+ if (a == NULL && b == NULL) return 0;
+ if (a == NULL && b != NULL) return 1;
+ if (a != NULL && b == NULL) return -1;
+
+ for(size_t i = 0; i < n &&
+ *a != '\0' &&
+ *b != '\0' &&
+ dns_tolower(*a) == dns_tolower(*b);
+ i++, a++, b++);
+
+ return dns_tolower(*a) - dns_tolower(*b);
+}
+
+int dns_compare(const char *a, const char *b)
+{
+ return dns_ncompare(a, b, (size_t)-1);
+}
+
+int dns_compare_labels(const char *a, const char *b)
+{
+ if (a == NULL && b == NULL) return 0;
+ if (a == NULL && b != NULL) return 1;
+ if (a != NULL && b == NULL) return -1;
+
+ const char *ptr_a = a + strlen(a);
+ const char *ptr_b = b + strlen(b);
+ const char *label_a = ptr_a, *label_b = ptr_b;
+ int comp = 0;
+
+ while(comp == 0 && ptr_a > a && ptr_b > b) {
+ /* look for start of label, including dot */
+ label_a = strchr_ba(a, ptr_a, '.');
+ label_b = strchr_ba(b, ptr_b, '.');
+ if (ptr_a - label_a != ptr_b - label_b)
+ /* compare labels up to minimum length
+ but include \0 to make sure that we
+ don't consider alpha and alphabet
+ equal */
+ return dns_ncompare(label_a, label_b,
+ I_MIN(ptr_a - label_a,
+ ptr_b - label_b)+1);
+ comp = dns_ncompare(label_a, label_b, ptr_a -label_a);
+ ptr_a = label_a - 1;
+ ptr_b = label_b - 1;
+ }
+
+ return dns_tolower(*label_a) - dns_tolower(*label_b);
+}
+
+int dns_match_wildcard(const char *name, const char *mask)
+{
+ i_assert(name != NULL && mask != NULL);
+
+ for(;*name != '\0' && *mask != '\0'; name++, mask++) {
+ switch(*mask) {
+ case '*':
+ for(; *name != '\0' && *name != '.'; name++);
+ if (*name == '.') mask++;
+ if (dns_tolower(*name) != dns_tolower(*mask)) return -1;
+ break;
+ case '?':
+ break;
+ default:
+ if (dns_tolower(*name) != dns_tolower(*mask)) return -1;
+ }
+ }
+ if (*mask == '*') mask++;
+ return dns_tolower(*name) == dns_tolower(*mask) ? 0 : -1;
+}
diff --git a/src/lib-dns/dns-util.h b/src/lib-dns/dns-util.h
new file mode 100644
index 0000000..cff52ff
--- /dev/null
+++ b/src/lib-dns/dns-util.h
@@ -0,0 +1,36 @@
+#ifndef DNS_UTIL_H
+#define DNS_UTIL_H 1
+
+static inline char
+dns_tolower(char c)
+{
+ if (c >= 'A' && c <= 'Z')
+ c+='a'-'A';
+ return c;
+}
+
+/**
+ * Will compare names in accordance with RFC4343
+ */
+int dns_compare(const char *a, const char *b) ATTR_PURE;
+int dns_ncompare(const char *a, const char *b, size_t n) ATTR_PURE;
+
+/**
+ * Same as above but done by labels from right to left
+ *
+ * www.example.org and www.example.net would be compared as
+ * org = net (return first difference)
+ * example = example
+ * www = www
+ */
+int dns_compare_labels(const char *a, const char *b) ATTR_PURE;
+
+/**
+ * Will match names in RFC4592 style
+ *
+ * this means *.foo.bar will match name.foo.bar
+ * but *DOES NOT* match something.name.foo.bar
+ */
+int dns_match_wildcard(const char *name, const char *mask) ATTR_PURE;
+
+#endif
--
2.1.4
>From da2908b37fc00d23018a2a455bad805bfd5ba006 Mon Sep 17 00:00:00 2001
From: Aki Tuomi <[email protected]>
Date: Fri, 11 Nov 2016 13:36:23 +0200
Subject: [PATCH 2/3] lib-dns: Add tests for dns-util
---
src/lib-dns/Makefile.am | 26 ++++++++++
src/lib-dns/test-dns-util.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 146 insertions(+)
create mode 100644 src/lib-dns/test-dns-util.c
diff --git a/src/lib-dns/Makefile.am b/src/lib-dns/Makefile.am
index 4a0e8bc..302e37c 100644
--- a/src/lib-dns/Makefile.am
+++ b/src/lib-dns/Makefile.am
@@ -11,5 +11,31 @@ headers = \
dns-lookup.h \
dns-util.h
+test_programs = \
+ test-dns-util
+
+noinst_PROGRAMS = $(test_programs)
+
+test_libs = \
+ libdns.la \
+ ../lib-test/libtest.la \
+ ../lib/liblib.la
+
+test_dns_util_SOURCE = test-dns-util.c
+test_dns_util_LDADD = $(test_libs)
+test_dns_util_CFLAGS = $(AM_CPPFLAGS) \
+ -I$(top_srcdir)/src/lib-test
+
+check: check-am check-test
+check-test: all-am
+ for bin in $(test_programs); do \
+ if test "$$bin" = "test-program-client-local"; then \
+ if ! env NOVALGRIND=yes $(RUN_TEST) ./$$bin; then exit 1; fi; \
+ else \
+ if ! $(RUN_TEST) ./$$bin; then exit 1; fi; \
+ fi \
+ done
+
+
pkginc_libdir=$(pkgincludedir)
pkginc_lib_HEADERS = $(headers)
diff --git a/src/lib-dns/test-dns-util.c b/src/lib-dns/test-dns-util.c
new file mode 100644
index 0000000..96c1be7
--- /dev/null
+++ b/src/lib-dns/test-dns-util.c
@@ -0,0 +1,120 @@
+#include "lib.h"
+#include "test-lib.h"
+#include "dns-util.h"
+#include "array.h"
+
+static void test_dns_compare(void)
+{
+ struct {
+ const char *a;
+ const char *b;
+ int res;
+ } tests[] =
+ {
+ { NULL, NULL, 0 },
+ { NULL, "", 1 },
+ { "", NULL, -1 },
+ { "", "", 0 },
+ { "a", "a", 0 },
+ { "a", "b", -1 },
+ { "b", "a", 1 },
+ { "A", "A", 0 },
+ { "A", "B", -1 },
+ { "B", "A", 1 },
+ { "A", "a", 0 },
+ { "a", "B", -1 },
+ { "B", "a", 1 },
+ { "test.invalid", "TeSt.InVaLid", 0 },
+ { "alphabet.com", "alpha.com", 52 },
+ { "com.alphabet", "com.alpha", 98 },
+ { "com.com", "com.comcom", -99 },
+ };
+
+ test_begin("test_dns_compare");
+
+ for(size_t i = 0; i < N_ELEMENTS(tests); i++) {
+ test_assert_idx(dns_compare(tests[i].a, tests[i].b) == tests[i].res, i);
+ test_assert_idx(dns_compare_labels(tests[i].a, tests[i].b) == tests[i].res, i);
+ }
+
+ test_end();
+}
+
+static void test_dns_match(void)
+{
+ struct {
+ const char *name;
+ const char *mask;
+ int res;
+ } tests[] =
+ {
+ { "", "", 0 },
+ { "", "*", 0 },
+ { "*", "", -1 },
+ { "TeSt.InVaLid", "test.invalid", 0 },
+ { "contoso.com", "test.invalid", -1 },
+ { "test.invalid", "test.unvalid", -1 },
+ { "name.test.invalid", "*.test.invalid", 0 },
+ { "real.name.test.invalid", "*.test.invalid", -1 },
+ { "real.name.test.invalid", "*.*.test.invalid", 0 },
+ { "name.test.invalid", "*name*.test.invalid", -1 },
+ };
+
+ test_begin("test_dns_match");
+
+ for(size_t i = 0; i < N_ELEMENTS(tests); i++) {
+ test_assert_idx(dns_match_wildcard(tests[i].name, tests[i].mask) == tests[i].res, i);
+ }
+
+ test_end();
+}
+
+static int
+arr_dns_compare(const char *const *a, const char *const *b)
+{
+ return dns_compare_labels(*a,*b);
+}
+
+static void test_dns_sort(void)
+{
+ const char *input[] = {
+ "test.invalid",
+ "test.com",
+ "test.contoso.com",
+ "test.alphabet.com",
+ "test.xxx",
+ };
+
+ const char *output[] = {
+ "test.alphabet.com",
+ "test.contoso.com",
+ "test.com",
+ "test.invalid",
+ "test.xxx",
+ };
+
+ test_begin("test_dns_sort");
+
+ ARRAY_TYPE(const_string) arr;
+ t_array_init(&arr, 8);
+
+ array_append(&arr, input, N_ELEMENTS(input));
+
+ array_sort(&arr, arr_dns_compare);
+
+ for(size_t i = 0; i < N_ELEMENTS(output); i++) {
+ test_assert_idx(dns_compare(*array_idx(&arr, i), output[i]) == 0, i);
+ }
+
+ test_end();
+}
+
+int main(void) {
+ void (*test_functions[])(void) = {
+ test_dns_compare,
+ test_dns_match,
+ test_dns_sort,
+ NULL
+ };
+ return test_run(test_functions);
+}
--
2.1.4
>From b2cd5c1cbfa91e7288a6ff41d51ced05c1c25f57 Mon Sep 17 00:00:00 2001
From: Aki Tuomi <[email protected]>
Date: Fri, 11 Nov 2016 13:40:55 +0200
Subject: [PATCH 3/3] config: Match local_name using dns-util
This way it correctly handles wildcards.
---
src/config/Makefile.am | 1 +
src/config/config-filter.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/config/Makefile.am b/src/config/Makefile.am
index 245ee56..a8cafa1 100644
--- a/src/config/Makefile.am
+++ b/src/config/Makefile.am
@@ -7,6 +7,7 @@ pkglibexec_PROGRAMS = config
AM_CPPFLAGS = \
-I$(top_srcdir)/src/lib \
+ -I$(top_srcdir)/src/lib-dns \
-I$(top_srcdir)/src/lib-settings \
-I$(top_srcdir)/src/lib-master \
-DPKG_RUNDIR=\""$(rundir)"\" \
diff --git a/src/config/config-filter.c b/src/config/config-filter.c
index 87a24da..426a326 100644
--- a/src/config/config-filter.c
+++ b/src/config/config-filter.c
@@ -6,6 +6,7 @@
#include "master-service-settings.h"
#include "config-parser.h"
#include "config-filter.h"
+#include "dns-util.h"
struct config_filter_context {
pool_t pool;
@@ -36,7 +37,7 @@ static bool config_filter_match_rest(const struct config_filter *mask,
if (mask->local_name != NULL) {
if (filter->local_name == NULL)
return FALSE;
- if (strcasecmp(filter->local_name, mask->local_name) != 0)
+ if (dns_match_wildcard(filter->local_name, mask->local_name) != 0)
return FALSE;
}
/* FIXME: it's not comparing full masks */
--
2.1.4
