Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improper TLS CRL handling 2/2 in mod_tls of ProFTPD < 1.3.6 #860

Closed
debrouxl opened this issue Nov 4, 2019 · 2 comments

Comments

@debrouxl
Copy link

@debrouxl debrouxl commented Nov 4, 2019

For tracking purposes (this problem is fixed in 1.3.6+).
This is the 3rd of 4 bugs in the tls_verify_crl() function. The wrong iteration variable is passed to sk_X509_REVOKED_value(), probably causing some CRL entries to be ignored. From the 1.3.5 -> 1.3.6 branches diff:

@@ -6001,14 +9084,15 @@ static int tls_verify_crl(int ok, X509_S
       /* Check if the current certificate is revoked by this CRL */
       n = sk_X509_REVOKED_num(X509_CRL_get_REVOKED(crl));
       for (j = 0; j < n; j++) {
         X509_REVOKED *revoked;
         ASN1_INTEGER *sn;
 
-        revoked = sk_X509_REVOKED_value(X509_CRL_get_REVOKED(crl), i);
-#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+        revoked = sk_X509_REVOKED_value(X509_CRL_get_REVOKED(crl), j);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L && \
+    !defined(HAVE_LIBRESSL)
         sn = X509_REVOKED_get0_serialNumber(revoked);
 #else
         sn = revoked->serialNumber;
 #endif /* OpenSSL-1.1.x and later */
 
         if (ASN1_INTEGER_cmp(sn, X509_get_serialNumber(xs)) == 0) {

FWIW as well, 4 years ago, stunnel got rid of custom CRL handling code and started relying on OpenSSL's built-in handling instead. That was between 5.23 and 5.24, compare src/verify.c from https://www.usenix.org.uk/mirrors/stunnel/archive/5.x/stunnel-5.23.tar.gz and https://www.usenix.org.uk/mirrors/stunnel/archive/5.x/stunnel-5.24.tar.gz .

I didn't hit this issue in the summer of 2018 when dealing with TLS CRLs using CentOS 7's ProFTPD 1.3.5e package, because the set of test CRLs only contained CRLs revoking at most one certificate, but I noticed it in the 1.3.5 -> 1.3.6 diff.
I quickly reported the issues privately, but ProFTPD's TLS CRL handling remains broken on all branches more than a year later... I'm aware that TLS CRLs are highly unpopular, and that only system administrators are supposed to define them, but clearly, low-profile responsible disclosure didn't work here :)
Public reports, and CVE ID assignments (for which I'll use this issue as reference), piling onto the recent higher risk issue #846 (CVE-2019-18217) and older vulnerabilities should help the downstream propagation of all fixes, at least if any downstream provides security support for ProFTPD.

@debrouxl

This comment has been minimized.

Copy link
Author

@debrouxl debrouxl commented Nov 26, 2019

CVE-2019-19271 .

@Castaglia

This comment has been minimized.

Copy link
Member

@Castaglia Castaglia commented Nov 27, 2019

The 1.3.5 release for ProFTPD is no longer supported; see the Versioning doc. The fix will be to upgrade.

@Castaglia Castaglia closed this Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.