Skip to content

Send the chain the config names and validate it ourselves, since SSL_CTX_build_cert_chain() discards the reason it failed #89

Description

@novotimo

app/main.c:init_openssl() hands chain construction to
SSL_CTX_build_cert_chain(), with
SSL_BUILD_CHAIN_FLAG_UNTRUSTED | SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR on the
cert-chain path and no flags on the cacerts path, and decides from the
return value alone whether the chain is fit to serve. That return value does
not divide along the line we care about, and the reason a chain failed is gone
by the time we see it.

Measured on OpenSSL 3.5.7, reconstructing by hand the verification the function
performs internally, with an X509_STORE_CTX built from the same leaf, the
same untrusted stack and the same store:

listener config X509_verify_cert() error(s) SSL_CTX_build_cert_chain()
cert-chain: chain.pem, leaf+int+root 0 depth 2, 19 self-signed certificate in certificate chain 1
cert-chain: servcert.pem, leaf alone 0 depth 0, 20 unable to get local issuer certificate; 21 unable to verify the first certificate 2
cert-chain: a self-signed leaf 0 depth 0, 18 self-signed certificate 2
cert-chain: an expired self-signed leaf 0 depth 0, 18 self-signed certificate; 10 certificate has expired 2
cacerts: with servcert: 1 none 1

Every cert-chain listener fails verification, the ordinary three-certificate
one included, because that path installs no trust anchors: SSL_CTX_new()
starts with an empty store, nothing calls SSL_CTX_set_default_verify_paths(),
and only load_cacerts() ever calls SSL_CTX_set_cert_store(). The 1 in the
first row is not a pass. ssl_build_cert_chain() in ssl/ssl_cert.c sets
rv = 2 when IGNORE_ERROR rescues a failed verification, and then the
security level loop near the end assigns rv = ssl_security_cert(...) once per
CA left in the chain, overwriting the 2 with 1 whenever any CA survives. So the
2 reports "the chain came out empty", not "errors were ignored", and #68's WARN
fires on a deliberately self-signed certificate while staying silent on a
production chain with a broken link.

The reason is not recoverable after the fact either. With IGNORE_ERROR set,
the ERR_raise_data() carrying SSL_R_CERTIFICATE_VERIFY_FAILED sits in the
branch the flag skips, and the X509_STORE_CTX holding the error code is freed
before the function returns. ERR_peek_error() is 0 on return, measured, which
is why #68's warning logs error_desc="" every time.

What to do instead

Send what the operator wrote and form our own opinion of it, separately.

SSL_CTX_set1_chain() installs the file's certificates as the chain to send
without consulting any store, which is what haproxy and nginx both do:
haproxy's ssl_sock_load_cert_chain() installs an empty stack on purpose so
that OpenSSL will not build from the verify store, and nginx's
ngx_ssl_certificate() shifts the leaf off the PEM and passes the rest to
SSL_CTX_set0_chain(). Neither validates the chain it serves, and neither
calls SSL_CTX_build_cert_chain() outside one narrow case: haproxy uses it
only under ssl-skip-self-issued-ca, to drop a root, with the same
UNTRUSTED|IGNORE_ERROR we pass plus NO_ROOT, and reads its return as
!rv so that a 2 counts as success.

Validation then becomes ours: X509_STORE_CTX_new(),
X509_STORE_CTX_init(store, leaf, untrusted), X509_verify_cert(), and a
verify callback that returns 1 so the walk continues past the first failure and
every error at every depth is collected. X509_STORE_CTX_get_error_depth() and
X509_verify_cert_error_string() are what turn that into the line an operator
can act on, and the expired self-signed row above shows the difference: two
distinct errors on one certificate, of which the return value carries neither.

That reporting should go to stderr as well as the log, since a master started
without a logfile still needs to hear it, and src/config.c:check_keyfiles()
already has the shape: log_system_err_m_ex() for the log with fprintf(stderr, ...) beside it.

SSL_BUILD_CHAIN_FLAG_UNTRUSTED becomes moot once we stop building. For the
record of why it is where it is: it only sets untrusted = cpk->chain, so it
matters exactly when the intermediates arrived with the leaf and nowhere else,
which is the cert-chain case. On the cacerts path cpk->chain is empty and
the flag would bind an empty stack. Measured, chain.pem with UNTRUSTED
alone and with no flags both return 0 and leave the error queue dirty, so the
two flags together are the minimum that made that path start at all, rather
than a decision about laxity.

Two things to settle before writing any of it:

  • The cacerts path currently gets its chain built from the store, so dropping
    SSL_CTX_build_cert_chain() there means composing the chain to send from the
    cacerts list ourselves. Doing nothing would send a bare leaf and break
    every deployment using that key.
  • Whether an unverifiable chain should still be refused under some
    configuration. haproxy makes the equivalent choice configurable for peer
    certificates with ca-ignore-err and crt-ignore-err, bitfields of
    X509_V_ERR_* codes accepted by name or number, and stunnel hardcodes a
    short list (UNABLE_TO_GET_ISSUER_CERT_LOCALLY,
    UNABLE_TO_VERIFY_LEAF_SIGNATURE, CERT_UNTRUSTED at depth 0). A
    cert-chain deployment is unverifiable by construction, so the default has
    to stay "warn and serve", and anything stricter is a later opt-in.

Certificate logging is inverted between the two paths

Both loops read SSL_CTX_get0_chain_certs(), which holds different things at
the two points they run.

load_cacerts() runs its loop straight after SSL_CTX_set_cert_store(), when
nothing has been added to the chain yet, so the stack is NULL, sk_X509_num()
returns -1 and the body never executes. The CA certificates the operator listed
are never logged. Measured on a cacerts config with cacert.pem and
intcert.pem: one event=cert_load line, for the leaf, from
load_servcert().

The cert-chain branch runs its loop after
SSL_CTX_use_certificate_chain_file(), where the stack holds the file's
certificates other than the leaf, and it never calls load_servcert().
Measured on chain.pem: two event=cert_load lines, for the intermediate and
the root, and none for the server certificate.

So each path logs the half the other one omits. Whatever replaces the loops
should log the leaf and the chain in one place, after the chain is settled.

Checklist

  • SSL_CTX_set1_chain() on the cert-chain path, sending the file as
    written
  • Compose and install the chain on the cacerts path too, rather than
    leaving it to be rebuilt from the store
  • Our own X509_STORE_CTX validation pass, with a verify callback that
    collects every error at every depth
  • Report each error with its depth, subject and
    X509_verify_cert_error_string(), to the log and to stderr
  • Log the leaf and the chain together, once, on both paths
  • Test: a cert-chain of leaf plus intermediate warns with
    unable to get local issuer certificate and still serves
  • Test: an expired leaf names expiry, not only that it is self-signed
  • Test: the chain on the wire is what the file named, for both paths

#68 is the immediate defect and its fix stands on its own; this supersedes the
WARN that came with it. Whether the self-issued root should be sent at all is
in #44 with the other OpenSSL knobs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions