Use of only CN is not allowed according to Baseline Requirements 1.8.0 from CA Browser Forum. Using CN is not prohibited, but if it is present, value in it must also be present in SAN. And SAN is always required.
While currently Let's Encrypt does accept requests without CN, in the interest of future-proofing, this commit modifies acme-client to always generate SAN and include CN in it. --- I recommend viewing the diff with whitespace differences ignored (-w), helps to show how minimal it is. I've tested this and it issues valid certificate that includes both CN and SAN (check current cert of wolfsden.cz if you want to make sure). Baseline Requirements can be found here [0], CN is described in 7.1.4.2.2. Why this is even a problem: My version of acme-client has some tests and pebble (let's encrypt's test server) already does not accept certificates without correct SAN [1]. 0: https://cabforum.org/wp-content/uploads/CA-Browser-Forum-BR-1.8.0.pdf 1: https://github.com/letsencrypt/pebble/issues/233 usr.sbin/acme-client/keyproc.c | 90 +++++++++++++++++----------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/usr.sbin/acme-client/keyproc.c b/usr.sbin/acme-client/keyproc.c index b4cc6184e26..3a1d68526d9 100644 --- a/usr.sbin/acme-client/keyproc.c +++ b/usr.sbin/acme-client/keyproc.c @@ -175,53 +175,51 @@ keyproc(int netsock, const char *keyfile, const char **alts, size_t altsz, * TODO: is this the best way of doing this? */ - if (altsz > 1) { - nid = NID_subject_alt_name; - if ((exts = sk_X509_EXTENSION_new_null()) == NULL) { - warnx("sk_X509_EXTENSION_new_null"); - goto out; - } - /* Initialise to empty string. */ - if ((sans = strdup("")) == NULL) { - warn("strdup"); - goto out; - } - sansz = strlen(sans) + 1; - - /* - * For each SAN entry, append it to the string. - * We need a single SAN entry for all of the SAN - * domains: NOT an entry per domain! - */ - - for (i = 1; i < altsz; i++) { - cc = asprintf(&san, "%sDNS:%s", - i > 1 ? "," : "", alts[i]); - if (cc == -1) { - warn("asprintf"); - goto out; - } - pp = recallocarray(sans, sansz, sansz + strlen(san), 1); - if (pp == NULL) { - warn("recallocarray"); - goto out; - } - sans = pp; - sansz += strlen(san); - strlcat(sans, san, sansz); - free(san); - san = NULL; - } - - if (!add_ext(exts, nid, sans)) { - warnx("add_ext"); - goto out; - } else if (!X509_REQ_add_extensions(x, exts)) { - warnx("X509_REQ_add_extensions"); - goto out; - } - sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); + nid = NID_subject_alt_name; + if ((exts = sk_X509_EXTENSION_new_null()) == NULL) { + warnx("sk_X509_EXTENSION_new_null"); + goto out; } + /* Initialise to empty string. */ + if ((sans = strdup("")) == NULL) { + warn("strdup"); + goto out; + } + sansz = strlen(sans) + 1; + + /* + * For each SAN entry, append it to the string. + * We need a single SAN entry for all of the SAN + * domains: NOT an entry per domain! + */ + + for (i = 0; i < altsz; i++) { + cc = asprintf(&san, "%sDNS:%s", + i ? "," : "", alts[i]); + if (cc == -1) { + warn("asprintf"); + goto out; + } + pp = recallocarray(sans, sansz, sansz + strlen(san), 1); + if (pp == NULL) { + warn("recallocarray"); + goto out; + } + sans = pp; + sansz += strlen(san); + strlcat(sans, san, sansz); + free(san); + san = NULL; + } + + if (!add_ext(exts, nid, sans)) { + warnx("add_ext"); + goto out; + } else if (!X509_REQ_add_extensions(x, exts)) { + warnx("X509_REQ_add_extensions"); + goto out; + } + sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free); /* Sign the X509 request using SHA256. */ -- 2.33.0
