EVP_get_cipherbynid/EVP_get_digestbynid when given an invalid ID/EVP_get_digestbyname segfaults with NULL argument

[schwabe]

Previous versions were fine when EVP_get_digestbynid got a random integer that does not map to any object. In the OpenBSD 7.5 version these function now started to segfault in this case. I think the reason is that the underlying EVP_get_digestbyname does not longer accept NULL as argument.

In OpenVPN we had this admittily a bit hacky code to enumerate all ciphers/digest:

    for (int nid = 0; nid < 10000; ++nid)
    {
        const EVP_MD *digest = EVP_get_digestbynid(nid);
        if (digest)
        {
            print_digest(digest, NULL);
        }
    }

which now segfaults to due the behaviour change. I have a workaround for the issue in OpenVPN (https://gerrit.openvpn.net/c/openvpn/+/586) that calls OBJ_nid2sn to figure out if it is safe to call EVP_get_digestbyname but I would like to raise the issue as other software might run into the same issue.

[botovq]

On Mon, May 13, 2024 at 03:51:30AM -0700, Arne Schwabe wrote:

Previous versions were fine when EVP_get_digestbynid got a random integer that does not map to any object. In the OpenBSD 7.5 version these function now started to segfault in this case. I think the reason is that the underlying EVP_get_digestbyname does not longer accept NULL as argument.

In OpenVPN we had this admittily a bit hacky code to enumerate all ciphers/digest:

    for (int nid = 0; nid < 10000; ++nid)
    {
        const EVP_MD *digest = EVP_get_digestbynid(nid);
        if (digest)
        {
            print_digest(digest, NULL);
        }
    }

which now segfaults to due the behaviour change. I have a workaround for the issue in OpenVPN (https://gerrit.openvpn.net/c/openvpn/+/586) that calls OBJ_nid2sn to figure out if it is safe to call EVP_get_digestbyname but I would like to raise the issue as other software might run into the same issue.

Thanks for the report. This issue was fixed shortly after release precisely because of OpenVPN. It didn't feel important enough for a backport or errata, as we haven't found other software relying on this:

https://github.com/openbsd/src/commit/ace1aaedae16f4098783ed4a8c5602142650126c

[cron2]

Hi,

a backport would be extremely nice :-) - we have upgraded our buildbot infrastructure to a new OpenBSD 7.5 bot, which is not passing selftests now. Including extra #ifdef for this particular version of LibreSSL is not something we're very keen on having in our code...

Out of curiousity, how are the packages built for OpenVPN for OpenBSD? The ports tree has no patches (relevant for this), but openvpn --show-ciphers with the ports-provided binary works juist fine.

gert (openvpn upstream maintainer)

[botovq]

On Mon, May 13, 2024 at 07:06:32AM -0700, Gert Doering wrote:

Hi,

a backport would be extremely nice :-) - we have upgraded our buildbot infrastructure to a new OpenBSD 7.5 bot, which is not passing selftests now. Including extra #ifdef for this particular version of LibreSSL is not something we're very keen on having in our code...

I understand that. Patching things in stable involves work from a number of people and it is only done for things that are really important. This use (which is a bit of a hack) in OpenVPN doesn't really qualify, I'm afraid.

If there should be another thing we need to backport in libcrypto, I will see if we can let this fix ride it (unfortunately this report came a bit too late - the last backport happened just last week-end).

Out of curiousity, how are the packages built for OpenVPN for OpenBSD? The ports tree has no patches (relevant for this), but openvpn --show-ciphers with the ports-provided binary works juist fine.

There is a patch in the stable branch which is functionally equivalent to the one linked in your gerrit:

https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/net/openvpn/patches/Attic/patch-src_openvpn_crypto_openssl_c?annotate=1.1.2.1

(the git conversion tool does not take cvs branches into account).

[cron2]

Thanks for the explanation. I didn't think of looking at CVS branches :-)