Skip to content

Commit

Permalink
* Test case and work around for domain names > 64 octets. Fixes #227.
Browse files Browse the repository at this point in the history
   When the first DNS name of an MD is longer than 63 octets, the certificate
   request will not contain a CN field, but leave it up to the CA to choose one.
   Currently, Lets Encrypt looks for a shorter name in the SAN list given and
   fails the request if none is found. But it is really up to the CA (and what
   browsers/libs accept here) and may change over the years. That is why
   the decision is best made at the CA.
  • Loading branch information
Stefan Eissing committed Jan 5, 2021
1 parent 4b9e1e5 commit bc4fd86
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 4 deletions.
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
* Test case and work around for domain names > 64 octets. Fixes #227.
When the first DNS name of an MD is longer than 63 octets, the certificate
request will not contain a CN field, but leave it up to the CA to choose one.
Currently, Lets Encrypt looks for a shorter name in the SAN list given and
fails the request if none is found. But it is really up to the CA (and what
browsers/libs accept here) and may change over the years. That is why
the decision is best made at the CA.
* Reverted setting the environment variables for MDMessageCmd and MDNotifyCmd. This
prevented the inheritance of existing environment variables as there seems to be
no portable way to iterate those on all platforms. This led to a regression on
Expand Down
16 changes: 13 additions & 3 deletions src/md_crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1686,8 +1686,18 @@ apr_status_t md_cert_req_create(const char **pcsr_der_64, const char *name,

/* subject name == first domain */
domain = APR_ARRAY_IDX(domains, 0, const unsigned char *);
if (!X509_NAME_add_entry_by_txt(n, "CN", MBSTRING_ASC, domain, -1, -1, 0)
|| !X509_REQ_set_subject_name(csr, n)) {
/* Do not set the domain in the CN if it is longer than 64 octets.
* Instead, let the CA choose a 'proper' name. At the moment (2021-01), LE will
* inspect all SAN names and use one < 64 chars if it can be found. It will fail
* otherwise.
* The reason we do not check this beforehand is that the restrictions on CNs
* are in flux. They used to be authoritative, now browsers no longer do that, but
* no one wants to hand out a cert with "google.com" as CN either. So, we leave
* it for the CA to decide if and how it hands out a cert for this or fails.
* This solves issue where the name is too long, see #227 */
if (strlen((const char*)domain) < 64
&& (!X509_NAME_add_entry_by_txt(n, "CN", MBSTRING_ASC, domain, -1, -1, 0)
|| !X509_REQ_set_subject_name(csr, n))) {
md_log_perror(MD_LOG_MARK, MD_LOG_ERR, 0, p, "%s: REQ name add entry", name);
rv = APR_EGENERAL; goto out;
}
Expand Down Expand Up @@ -1880,7 +1890,7 @@ apr_status_t md_cert_make_tls_alpn_01(md_cert_t **pcert, const char *domain,
const char *alts;
apr_status_t rv;

if (APR_SUCCESS != (rv = mk_x509(&x, pkey, domain, valid_for, p))) goto out;
if (APR_SUCCESS != (rv = mk_x509(&x, pkey, "tls-alpn-01-challenge", valid_for, p))) goto out;

/* add the domain as alt name */
alts = apr_psprintf(p, "DNS:%s", domain);
Expand Down
2 changes: 1 addition & 1 deletion test/TestEnv.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ def await_error(cls, domain, timeout=60):
return False
md = cls.get_md_status(domain)
if md:
if md['state'] == TestEnv.MD_S_ERROR:
if 'state' in md and md['state'] == TestEnv.MD_S_ERROR:
return md
if 'renewal' in md and 'errors' in md['renewal'] and md['renewal']['errors'] > 0:
return md
Expand Down
44 changes: 44 additions & 0 deletions test/test_0702_auto.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,50 @@ def test_702_052(self):
assert stat["proto"]["acme-tls/1"] == [domain]
assert TestEnv.await_completion([domain])

# Test a domain name longer than 64 chars, but components < 64, see #227
# Background: DNS has an official limit of 253 ASCII chars and components must be
# of length [1, 63].
# However the CN in a certificate is restricted too, see
# <https://github.com/letsencrypt/boulder/issues/2093>.
@pytest.mark.parametrize("challenge_type", [
"tls-alpn-01", "http-01"
])
def test_702_060(self, challenge_type):
domain = self.test_domain
# use only too long names, this is expected to fail:
# see <https://github.com/jetstack/cert-manager/issues/1462>
long_domain = ("x" * (65 - len(domain))) + domain
domains = [long_domain, "www." + long_domain]
conf = HttpdConf()
conf.add_admin("admin@" + domain)
conf.add_line("Protocols http/1.1 acme-tls/1")
conf.add_drive_mode("auto")
conf.add_ca_challenges([challenge_type])
conf.add_md(domains)
conf.add_vhost(domains)
conf.install()
assert TestEnv.apache_restart() == 0
TestEnv.check_md(domains)
assert TestEnv.await_error(long_domain)
# add a short domain to the SAN list, the CA should now use that one
# and issue a cert.
domains = [long_domain, "www." + long_domain, "xxx." + domain]
conf = HttpdConf()
conf.add_admin("admin@" + domain)
conf.add_line("Protocols http/1.1 acme-tls/1")
conf.add_drive_mode("auto")
conf.add_ca_challenges([challenge_type])
conf.add_md(domains)
conf.add_vhost(domains)
conf.install()
assert TestEnv.apache_restart() == 0
assert TestEnv.await_completion([long_domain])
TestEnv.check_md_complete(long_domain)
#
# check SSL running OK
cert = TestEnv.get_cert(long_domain)
assert long_domain in cert.get_san_list()

# --------- _utils_ ---------

def _write_res_file(self, doc_root, name, content):
Expand Down

0 comments on commit bc4fd86

Please sign in to comment.