[Patch] SIGSEGV: crash when certificate contains extension longer than 512 bytes

Bug #1912389 reported by Graham Leggett
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Net-SNMP
Fix Released
Unknown
net-snmp (Ubuntu)
Fix Released
Medium
Sergio Durigan Junior
Hirsute
Fix Released
Medium
Sergio Durigan Junior
Impish
Fix Released
Medium
Sergio Durigan Junior

Bug Description

[ Impact ]

Users can experience a segmentation fault on snmpd (part of net-snmp) when using a certificate that contains an extension longer than 512 bytes and debug output (-D) is enabled. Although this only happens when debugging, it seems to be pretty common to find certificates whose extensions are larger than 512 bytes.

[ Test Case ]

Below you can find a step-by-step procedure to reproduce the bug. Bear in mind that the "sed" command may be mangled due to Launchpad's text renderization.

$ lxc launch images:ubuntu/hirsute net-snmp-bug1912389
$ lxc shell net-snmp-bug1912389
# apt update && apt install snmpd -y
# sed -i "s@^#\s*nsCertType.*@nsCertType = client,email,objsign@; s@^#\s*nsCaRevocationUrl.*@nsCaRevocationUrl = http://www.myverylongurl$(printf '%*s' 512 | tr ' ' 'a').com/ca-crl.pem@; s@^#\s*extendedKeyUsage.*@extendedKeyUsage = critical,timeStamping,serverAuth,clientAuth,codeSigning,emailProtection@; s@^#\s*keyUsage.*@keyUsage = nonRepudiation,digitalSignature,keyEncipherment@" /etc/ssl/openssl.cnf
# openssl req -x509 -out localhost.crt -keyout localhost.key -newkey rsa:2048 -nodes -sha256 -extensions usr_cert -subj '/CN=localhost' -config /etc/ssl/openssl.cnf
# mkdir -p $HOME/.snmp/tls/certs
# cp localhost.crt $HOME/.snmp/tls/certs
# systemctl stop snmpd.service
# snmpd -DALL
...
not enough space or error in allocation for extenstion
Segmentation fault (core dumped)
#

[ Where problems could occur ]

The backported patches are very straightforward and only impact code that is run when debug (-D) is active. There is not much room for regression here, especially considering that this is a very recent version of the package that will very likely not be affected by newer rebuilds.

[ Original Description ]

When net-snmp is given a certificate with an extension that is longer than 512 characters, snmp crashes on startup.

Steps to Reproduce:
1. Configure net-snmp using an EV certificate from a CA (in this case Globalsign).
2. Start snmpd.
3.

Actual results:

[root@localhost tls]# systemctl status snmpd.service
● snmpd.service - Simple Network Management Protocol (SNMP) Daemon.
   Loaded: loaded (/usr/lib/systemd/system/snmpd.service; disabled; vendor preset: disabled)
   Active: failed (Result: core-dump) since Wed 2020-12-16 21:21:59 SAST; 16min ago
  Process: 53269 ExecStart=/usr/sbin/snmpd $OPTIONS -f (code=dumped, signal=SEGV)
 Main PID: 53269 (code=dumped, signal=SEGV)

Dec 16 21:21:57 localhost systemd[1]: Starting Simple Network Management Protocol (SNMP) Daemon....
Dec 16 21:21:58 localhost snmpd[53269]: refusing to read world readable or writable key /etc/snmp/tls/certs/snmpd.crt
Dec 16 21:21:58 localhost snmpd[53269]: not enough space or error in allocation for extenstion
Dec 16 21:21:59 localhost systemd[1]: snmpd.service: Main process exited, code=dumped, status=11/SEGV
Dec 16 21:21:59 localhost systemd[1]: snmpd.service: Failed with result 'core-dump'.
Dec 16 21:21:59 localhost systemd[1]: Failed to start Simple Network Management Protocol (SNMP) Daemon..

Expected results:

Deamon starts without a crash.

Additional info:

Fix available here:

https://github.com/net-snmp/net-snmp/pull/234

Related branches

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the bug report.

This should have been opened against net-snmp, and not nagios-plugins, right? I'm reassigning it to the proper package.

It seems to me that it's a valid bug, but it would be great to have a more detailed reproducer. I tried editing /etc/ssl/openssl.cnf and extend the "usr_cert" extension's "nsComment" field to a string that is really long. Then, I generated a self-signed x509 certificate using the "usr_cert" extension:

# openssl req -x509 -newkey rsa:4096 -keyout key.pem -out cert.pem -days 365 -extensions usr_cert

Then I edited /etc/snmp/snmpd.conf and included a "localCert" parameter there:

[snmp] localCert /usr/local/share/ca-certificates/cert.crt

Finally, restarting the snmpd.service doesn't seem to trigger the bug. I wonder what I'm doing wrong here... Pointers and advices are appreciated.

Thanks.

affects: nagios-plugins (Ubuntu) → net-snmp (Ubuntu)
Revision history for this message
Graham Leggett (minfrin-y) wrote :

Launchpad always seems to get the package wrong, it's odd.

To make net-snmp crash:

- Turn debugging on (the crashing happens when dumping the certificate as part of debug logging).
- Include a cert with an extension that, when printed, is longer than 512 bytes.
- The cert I was using is an EV certificate issued by Globalsign, the certificate transparency section is really large.

I think (need to check) that nsComment isn't technically an extension, and so won't be printed by net-snmp's certificate dump code.

Another way to force the bug is reduce SNMP_MAXBUF_SMALL to something tiny, like 1 byte. It will crash on any extension.

https://github.com/net-snmp/net-snmp/blob/V5-7-patches/snmplib/snmp_openssl.c#L482

This is the crash in an old branch that is unpatched:

https://github.com/net-snmp/net-snmp/blob/V5-7-patches/snmplib/snmp_openssl.c#L502

If the extension is too long, the _cert_get_extension_str_at() function returns NULL. This NULL is fed into strchr(), and boom.

The fix is in two parts - first, use a proper sized buffer that an extension can fit in, and if that's not enough, check str for NULL before trying to strchr() on it.

There were two attempts at a fix, one to stop the crash, and the second to fix the buffer length and stop the crash while also printing the name of the extension (but not value). Could potentially be confusing. Two fixes were developed at the same time.

Revision history for this message
Paride Legovini (paride) wrote :

I found the upstream bug for this issue:

https://github.com/net-snmp/net-snmp/issues/233

The fix landed in the upstream master and V5-9-patches branches [1], but the issue is still open lacking verification. The patch doesn't apply cleanly on version 5.8, the version currently in Focal, Groovy and Hirsute, so this is not a SRU "patch on a plate" scenario, at least not for the moment.

For Hirsute: I had a look at the past upstream release history and I think it's unlikely that we'll get 5.9.1 released in time. However we do have a patch ready for the version in Hirsute, so this should be an easy fix. Before proceeding I'd wait for the upstream issue to be closed as fixed, confirming that the patch works as expected. I poked the issue on GitHub.

[1] https://github.com/net-snmp/net-snmp/commit/bb30f8ee0075750fd3648a6bf3fab543f46152ed

Changed in net-snmp (Ubuntu):
status: New → Triaged
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks, Paride.

I had also found the same bug yesterday, but I decided not to mark the bug as Triaged because I still cannot reproduce it.

As noted previously, I would like to be able to reproduce the issue before moving forward. This will prove useful if we have to SRU the patch.

I still could not successfully generate a TLS cert with an extension bigger than 512 bytes, but I haven't tried again after Graham's last comment.

Revision history for this message
Graham Leggett (minfrin-y) wrote :
Download full text (4.1 KiB)

diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
index e0e6615f0..dd202f440 100644
--- a/snmplib/snmp_openssl.c
+++ b/snmplib/snmp_openssl.c
@@ -499,6 +499,8 @@ netsnmp_openssl_cert_dump_extensions(X509 *ocert)
         extension_name = OBJ_nid2sn(nid);
         buf_len = sizeof(buf);
         str = _cert_get_extension_str_at(ocert, i, &buf_ptr, &buf_len, 0);
+ if (!str)
+ continue;
         lf = strchr(str, '\n'); /* look for multiline strings */
         if (NULL != lf)
             *lf = '\0'; /* only log first line of multiline here */
diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
index 01d72afb2..e0e6615f0 100644
--- a/snmplib/snmp_openssl.c
+++ b/snmplib/snmp_openssl.c
@@ -300,7 +300,7 @@ _cert_get_extension(X509_EXTENSION *oext, char **buf, int *len, int flags)

     if (!buf_ptr) {
         snmp_log(LOG_ERR,
- "not enough space or error in allocation for extenstion\n");
+ "not enough space or error in allocation for extension\n");
         BIO_vfree(bio);
         return NULL;
     }
diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
index dd202f440..7d6db6ae6 100644
--- a/snmplib/snmp_openssl.c
+++ b/snmplib/snmp_openssl.c
@@ -290,8 +290,11 @@ _cert_get_extension(X509_EXTENSION *oext, char **buf, int *len, int flags)

     space = BIO_get_mem_data(bio, &data);
     if (buf && *buf) {
- if (*len < space)
- buf_ptr = NULL;
+ if (*len < space + 1) {
+ snmp_log(LOG_ERR, "not enough buffer space to print extension\n");
+ BIO_vfree(bio);
+ return NULL;
+ }
         else
             buf_ptr = *buf;
     }
@@ -299,8 +302,7 @@ _cert_get_extension(X509_EXTENSION *oext, char **buf, int *len, int flags)
         buf_ptr = calloc(1,space + 1);

     if (!buf_ptr) {
- snmp_log(LOG_ERR,
- "not enough space or error in allocation for extension\n");
+ snmp_log(LOG_ERR, "error in allocation for extension\n");
         BIO_vfree(bio);
         return NULL;
     }
@@ -479,7 +481,7 @@ netsnmp_openssl_cert_dump_extensions(X509 *ocert)
 {
     X509_EXTENSION *extension;
     const char *extension_name;
- char buf[SNMP_MAXBUF_SMALL], *buf_ptr = buf, *str, *lf;
+ char buf[SNMP_MAXBUF], *buf_ptr = buf, *str, *lf;
     int i, num_extensions, buf_len, nid;

     if (NULL == ocert)
@@ -499,8 +501,11 @@ netsnmp_openssl_cert_dump_extensions(X509 *ocert)
         extension_name = OBJ_nid2sn(nid);
         buf_len = sizeof(buf);
         str = _cert_get_extension_str_at(ocert, i, &buf_ptr, &buf_len, 0);
- if (!str)
+ if (!str) {
+ DEBUGMSGT(("9:cert:dump", " %2d: %s\n", i,
+ extension_name));
             continue;
+ }
         lf = strchr(str, '\n'); /* look for multiline strings */
         if (NULL != lf)
             *lf = '\0'; /* only log first line of multiline here */
diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
index 7d6db6ae6..c092a007a 100644
--- a/snmplib/snmp_openssl.c
+++ b/snmplib/snmp_openssl.c
@@ -284,33 +284,30 @@ _cert_get_extensio...

Read more...

Changed in netsnmp:
status: Unknown → New
Revision history for this message
Paride Legovini (paride) wrote :

Hi Sergio,

I did manage to reproduce the crash by lowering SNMP_MAXBUF_SMALL and rebuilding the package, as Graham suggested. I couldn't generate a certificate crashing snmpd with the default value of 512, but most likely I didn't manage to add a very long extension to the certs I generated.

In my previous comment I mentioned Focal. I think this is an Invalid bug in Focal, as Focal's snmp doesn't support TLS at all, see LP: 1880724. I'm going to change this bug tasks according to this.

Paride

Changed in net-snmp (Ubuntu Focal):
status: New → Invalid
Changed in netsnmp:
status: New → Fix Released
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Hi Paride,

Thanks for further investigating. I assumed that the crash was indeed reproducible by hacking the package, but I think it's important to get a reproducer that doesn't involve rebuilding anything if we're thinking about an SRU (for Groovy, for example).

In any case, I think it's possible to proceed with the upstream fix. I'll see if I have time later to look into this.

Revision history for this message
Graham Leggett (minfrin-y) wrote :

In theory, any Let's Encrypt certificate should cause this crash.

The serialised certificate transparency of the certificate at redwax.eu is 1577 bytes, three times higher than the 512 byte limit that triggers the crash.

            CT Precertificate SCTs:
                Signed Certificate Timestamp:
                    Version : v1(0)
                    Log ID : 5C:DC:43:92:FE:E6:AB:45:44:B1:5E:9A:D4:56:E6:10:
                                37:FB:D5:FA:47:DC:A1:73:94:B2:5E:E6:F6:C7:0E:CA
                    Timestamp : Jan 11 03:13:53.345 2021 GMT
                    Extensions: none
                    Signature : ecdsa-with-SHA256
                                30:46:02:21:00:E3:F3:7C:A3:71:D1:57:DD:76:1E:01:
                                AA:61:7D:E2:39:6B:0F:2B:93:B3:3A:B2:90:8F:EE:D5:
                                04:92:12:BD:8B:02:21:00:D0:8C:15:42:F5:E7:6C:D0:
                                FC:B4:25:E2:57:5E:C1:EB:F8:7A:4A:06:5B:AD:4C:6E:
                                B0:25:96:45:DF:A9:97:41
                Signed Certificate Timestamp:
                    Version : v1(0)
                    Log ID : F6:5C:94:2F:D1:77:30:22:14:54:18:08:30:94:56:8E:
                                E3:4D:13:19:33:BF:DF:0C:2F:20:0B:CC:4E:F1:64:E3
                    Timestamp : Jan 11 03:13:53.322 2021 GMT
                    Extensions: none
                    Signature : ecdsa-with-SHA256
                                30:44:02:20:3E:CC:F8:39:7E:27:5F:20:D7:77:DC:75:
                                9D:D2:F2:C8:09:F4:86:1C:26:FF:76:DF:21:A0:BD:90:
                                8A:26:FA:59:02:20:75:69:90:93:B2:F4:76:BC:1A:D3:
                                63:B7:1C:D0:72:56:13:97:7D:37:B3:8B:E2:6E:8C:71:
                                1A:72:4D:7E:86:F4

Revision history for this message
Graham Leggett (minfrin-y) wrote :

Quick ping on this one.

Latest net-snmp with this fixed is https://github.com/net-snmp/net-snmp/releases/tag/v5.9.1.rc1.

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Thanks for the heads-up Graham. Our team will be taking a look at it.

Changed in net-snmp (Ubuntu):
assignee: nobody → Sergio Durigan Junior (sergiodj)
importance: Undecided → Medium
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks, Graham.

This issue impacts Hirsute and Impish.

For Impish, the best course of action here would be to wait for Debian to pick up this fix, which would then mean that Ubuntu would automatically pick it up as well. Given that Debian is in freeze right now, I don't know if the net-snmp maintainers there are planning to work on it anytime soon.

For Hirsute, we'd need to go through the SRU process. The first step here would be to come up with a reliable set of steps to reproduce this issue, which I haven't been able to do so far (I confess I haven't had the time to test your Let's Encrypt suggestion, though).

I will see if I can reproduce it locally and then start the SRU process. If you already have a detailed set of steps to reproduce the bug, please let me know. Thanks.

Changed in net-snmp (Ubuntu Hirsute):
assignee: nobody → Sergio Durigan Junior (sergiodj)
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

OK, finally I was able to trigger the bug locally using a self-signed cert. I am going to start writing the SRU template for it.

Thanks.

description: updated
Revision history for this message
Graham Leggett (minfrin-y) wrote :
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Still waiting on the SRU team to address this. I will ping them today.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package net-snmp - 5.9+dfsg-3ubuntu2

---------------
net-snmp (5.9+dfsg-3ubuntu2) impish; urgency=medium

  * Fix segmentation fault when certificate contains extension
    longer than 512 bytes (LP: #1912389)
    - d/p/lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch:
      Skip certificate if loading fails.
    - d/p/lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch:
      Make sure enough space is allocated for extensions longer than
      512 bytes.

 -- Sergio Durigan Junior <email address hidden> Tue, 25 May 2021 19:03:31 -0400

Changed in net-snmp (Ubuntu Impish):
status: Triaged → Fix Released
Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Graham, or anyone else affected,

Accepted net-snmp into hirsute-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/net-snmp/5.9+dfsg-3ubuntu1.21.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-hirsute to verification-done-hirsute. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-hirsute. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in net-snmp (Ubuntu Hirsute):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-hirsute
Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (net-snmp/5.9+dfsg-3ubuntu1.21.04.1)

All autopkgtests for the newly accepted net-snmp (5.9+dfsg-3ubuntu1.21.04.1) for hirsute have finished running.
The following regressions have been reported in tests triggered by the package:

asterisk/1:16.16.1~dfsg-1 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/hirsute/update_excuses.html#net-snmp

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Mathew Hodson (mhodson)
no longer affects: net-snmp (Ubuntu Focal)
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Performing the verification for Hirsute:

First, reproducing the bug with the version currently available:

# apt policy snmpd
snmpd:
  Installed: 5.9+dfsg-3ubuntu1
  Candidate: 5.9+dfsg-3ubuntu1
  Version table:
 *** 5.9+dfsg-3ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu hirsute/main amd64 Packages
        100 /var/lib/dpkg/status
# snmpd -DALL
...
9:cert:dump: 5: authorityKeyIdentifier = keyid:AC:D0:13:2A:98:58:02:02:D2:BA:E9:8A:0B:F3:5A:B8:BD:6C:BB:64
not enough space or error in allocation for extenstion
Segmentation fault (core dumped)

Now, updating the package to the version available in -proposed and making sure that the bug is fixed:

# apt policy snmpd
snmpd:
  Installed: 5.9+dfsg-3ubuntu1.21.04.1
  Candidate: 5.9+dfsg-3ubuntu1.21.04.1
  Version table:
 *** 5.9+dfsg-3ubuntu1.21.04.1 500
        500 http://archive.ubuntu.com/ubuntu hirsute-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     5.9+dfsg-3ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu hirsute/main amd64 Packages
# snmpd -DALL
trace: netsnmp_getaddrinfo(): system.c, 851:
dns:getaddrinfo: looking up "127.0.0.1" with hint ({ ... })
trace: netsnmp_sockaddr_in6_3(): transports/snmpIPv6BaseDomain.c, 314:
netsnmp_sockaddr_in6: failed to parse 127.0.0.1
Error opening specified endpoint "127.0.0.1"
Server Exiting with code 1
#

As can be seen, the segmentation fault doesn't happen anymore. Therefore, the bug has been fixed and the verification is complete.

tags: added: verification-done-hirsute
removed: verification-needed verification-needed-hirsute
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package net-snmp - 5.9+dfsg-3ubuntu1.21.04.1

---------------
net-snmp (5.9+dfsg-3ubuntu1.21.04.1) hirsute; urgency=medium

  * Fix segmentation fault when certificate contains extension
    longer than 512 bytes (LP: #1912389)
    - d/p/lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch:
      Skip certificate if loading fails.
    - d/p/lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch:
      Make sure enough space is allocated for extensions longer than
      512 bytes.

 -- Sergio Durigan Junior <email address hidden> Tue, 25 May 2021 19:09:11 -0400

Changed in net-snmp (Ubuntu Hirsute):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for net-snmp has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.