Opened 4 years ago

Closed 3 years ago

#800 closed Bug / Defect (fixed)

OpenVPN version 2.3.x and older do not check the CRL signature

Reported by: eriktews Owned by: Steffan Karger
Priority: major Milestone:
Component: Certificates Version: OpenVPN 2.3.9 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: crl, signature
Cc: Steffan Karger

Description

OpenVPN version 2.3.x and older versions do not check the signature of a CRL at all. So when OpenVPN is used in a scenario in which the CRL is regularly updated from an unsecure HTTP server, an attacker might inject his own CRL here. Only the issuer of the CRL needs to match, signatures or expiration dates are not checked.

It looks like this has been fixed with the rewrite of the CRL code in version 2.4.x.

Change History (7)

comment:1 Changed 4 years ago by Gert Döring

Cc: Steffan Karger added

Well, I'm a bit unsure what to do about this one. Yes, CRL handling in 2.3 was much less strict than in 2.4 - and it has been that way since CRL handling code was added to OpenVPN.

We're certainly not going to backport the 2.4 CRL changes to 2.3, as 2.3 will receive maintenance changes only (long-term portability / compatibility and bug fixes) and this is a feature change.

So, anyone concerned about this should just upgrade to 2.4? I assume it affects only a small number of operators anyway (we push our CRLs via ssh from the CA host to the OpenVPN server, for example)...

@syzzer, what do you think? close with "wontfix"?

comment:2 Changed 4 years ago by Steffan Karger

Owner: set to Steffan Karger
Priority: criticalmajor
Status: newassigned

There's two things here; interpretation of the nextUpdate field and accepting CRLs that are not signed by the CA.

The interpretation of the nextUpdate field will not be changed for 2.3. To be clear: the nextUpdate field _is not_ an expiry field, but it is treated as such by many implementations. OpenVPN 2.4 is now one of those.

Not checking the CRL signature is an omission that has been fixed in 2.4, but it is definitely not as critical as it might seem. An attacker that can not modify the CRL can either just replay an old CRL (given that the thisUpdate/nextUpdate fields are not interpreted as validity fields), or even just drop the CRL update on the network.

For now, I'll downgrade the priority from 'critical' to 'major' for two reasons:

  1. 2.4.0 is available if you want to interpret the update fields as validity, and check the CRL signature.
  1. Even if there was 2.4.0 available, CRLs are a dodgy mechanism at best (see above), making this for from critical.

I'll put the CRL signature checking for 2.3 on my todo list. No promises though - it's not on the top of my list and if a fix involves intrusive changes we'll just recommend to upgrade to 2.4 instead.

comment:3 Changed 4 years ago by eriktews

So from my point, updating to 2.4.x is a good solution. I don't know whether there are any OpenVPN users out there who have a major problem with that, but would be very happy with a patch for 2.3.x.

But what is most important from my point of view is, that the 2.4.x release (or a security announcement) should clearly document that 2.3.x doesn't do proper CRL checking (it's not documented in the 2.3.x manpage) and users who need proper CRL checking should upgrade to 2.4.x.

comment:4 Changed 4 years ago by Samuli Seppänen

A clarification to the 2.3 man-page sounds like a good idea, and non-intrusive.

comment:5 Changed 4 years ago by Bjoern Voigt

Unfortunately the stricter CRL checking in version 2.4.0 can result in configurations, which have worked in version 2.3.x and fail in 2.4.x. I could not find much documentation for tracking down problem with CRLs in 2.4.x. I found this changeset

https://github.com/OpenVPN/openvpn/commit/160504a2955c4478cd2c0323452929e07016a336

Better update instructions, documentation and/or error messages are welcome.

I could fix a broken CA/CRL setup, but I needed the source code for this. One example for missing error/debugging messages from openvpn-2.4.0/src/openvpn/ssl_verify_openssl.c:

        X509_OBJECT *obj = sk_X509_OBJECT_value(store->objs, i);
        ASSERT(obj);
        if (obj->type == X509_LU_CRL)
        {
            return false;
        }

It would be nice to have an error message here, e.g. "configured CRL file has the invalid type X509_LU_X509 instead of X509_LU_CRL".

Last edited 4 years ago by Bjoern Voigt (previous) (diff)

comment:6 in reply to:  5 Changed 3 years ago by Steffan Karger

Replying to bjoernv:

Unfortunately the stricter CRL checking in version 2.4.0 can result in configurations, which have worked in version 2.3.x and fail in 2.4.x. I could not find much documentation for tracking down problem with CRLs in 2.4.x. I found this changeset

https://github.com/OpenVPN/openvpn/commit/160504a2955c4478cd2c0323452929e07016a336

Better update instructions, documentation and/or error messages are welcome.

As the release notes explain:

"The [ new CRL checking ] implementations are more strict [ .. ]. This might reject peer certificates that would previously be accepted. If this occurs, OpenVPN will log the crypto library's error description."

If your specific problem was not logged by OpenVPN, please open a new support ticket. In that ticket, explain what your situation was, how you resolved it, and make sure to supply logs.

I could fix a broken CA/CRL setup, but I needed the source code for this. One example for missing error/debugging messages from openvpn-2.4.0/src/openvpn/ssl_verify_openssl.c:

        X509_OBJECT *obj = sk_X509_OBJECT_value(store->objs, i);
        ASSERT(obj);
        if (obj->type == X509_LU_CRL)
        {
            return false;
        }

It would be nice to have an error message here, e.g. "configured CRL file has the invalid type X509_LU_X509 instead of X509_LU_CRL".

This is not an error, but a success condition. This is an excerpt of the tls_verify_crl_missing() function, which returns 'false' if the CRL is *not* missing. There is no reason to print an error message here. (And the caller will print an error if the CRL *is* missing.)

comment:7 Changed 3 years ago by Steffan Karger

Resolution: fixed
Status: assignedclosed

The 2.3 CRL checking behaviour is now documented explicitly in the 2.3 man page (including that the fix is to upgrade to 2.4):
https://github.com/OpenVPN/openvpn/commit/51d936d0

Note: See TracTickets for help on using tickets.