Merge ~sergiodj/ubuntu/+source/net-snmp:bug1912389-segv-cert-longer-512-bytes-hirsute into ubuntu/+source/net-snmp:ubuntu/hirsute-devel

[Sergio Durigan Junior]

This is the fix for bug 1912389 on Hirsute.

The bug is about snmpd segfaulting when two conditions are met:

1) Debug output is enabled (-D flag), and

2) The TLS certificate being used has an extension longer than 512 bytes.

The first condition is obviously easy to reproduce, but the second one is much trickier. After a while struggling with openssl and its very easy configuration (*ahem*), I was finally able to generate a self-signed certificate that triggers the issue. This is going to be an SRU for Hirsute, so I wrote detailed instructions on the bug; please take a look and let me know what you think.

The bug has been fixed upstream and is part of the new 5.9.1.rc1 release. Initially I thought about waiting to see if Debian would pick this up, but given that they're in freeze and the net-snmp maintainer didn't touch the package since last year, I decided to backport this to Impish/Hirsute just in case. Worst case scenario, Debian will release a new net-snmp and then we can drop these patches.

You can find the proposed packages on the following PPA:

https://launchpad.net/~sergiodj/+archive/ubuntu/net-snmp-bug1912389

autopkgtest is still happy:

autopkgtest [20:00:20]: @@@@@@@@@@@@@@@@@@@@ summary
command1 PASS

[Lucas Kanashiro (lucaskanashiro)]

LGTM, +1.

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks, Lucas.

Uploaded:

$ dput net-snmp_5.9+dfsg-3ubuntu1.21.04.1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/net-snmp/net-snmp_5.9+dfsg-3ubuntu1.21.04.1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/net-snmp/net-snmp_5.9+dfsg-3ubuntu1.21.04.1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading net-snmp_5.9+dfsg-3ubuntu1.21.04.1.dsc: done.
  Uploading net-snmp_5.9+dfsg-3ubuntu1.21.04.1.debian.tar.xz: done.
  Uploading net-snmp_5.9+dfsg-3ubuntu1.21.04.1_source.buildinfo: done.
  Uploading net-snmp_5.9+dfsg-3ubuntu1.21.04.1_source.changes: done.
Successfully uploaded packages.

As usual, I'm not pushing the tag until the SRU team accepts the package.

Unmerged commits

e0b7db5... by Sergio Durigan Junior on 2021-05-25

changelog for 5.9+dfsg-3ubuntu1.21.04.1

2afb778... by Sergio Durigan Junior on 2021-05-25

  * 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.

diff --git a/debian/changelog b/debian/changelog
index f5ff71b..2642f0f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+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 <[e-mail]>  Tue, 25 May 2021 19:09:11 -0400
+
 net-snmp (5.9+dfsg-3ubuntu1) hirsute; urgency=medium
 
   * Merge with Debian unstable. Remaining changes:
diff --git a/debian/patches/lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch b/debian/patches/lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch
new file mode 100644
index 0000000..52f3a59
--- /dev/null
+++ b/debian/patches/lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch
@@ -0,0 +1,31 @@
+From: Bart Van Assche <[e-mail]>
+Date: Wed, 16 Dec 2020 19:36:55 -0800
+Subject: libsnmp: Handle certificate loading errors gracefully
+
+Instead of crashing (SIGSEGV), skip certificates if loading fails.
+
+Reported-by: https://github.com/minfrin
+Fixes: https://github.com/net-snmp/net-snmp/issues/233
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/4c5d2ceae4d4a8b77db48fc8fff77e14943ff4d3
+Applied-Upstream: 5.9.1.rc1
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1912389
+Bug: https://github.com/net-snmp/net-snmp/issues/233
+Reviewed-By: Sergio Durigan Junior <[e-mail]>
+---
+ snmplib/snmp_openssl.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
+index 01d72af..22be003 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/debian/patches/lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch b/debian/patches/lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch
new file mode 100644
index 0000000..1844823
--- /dev/null
+++ b/debian/patches/lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch
@@ -0,0 +1,70 @@
+From: minfrin <[e-mail]>
+Date: Thu, 17 Dec 2020 12:25:24 +0000
+Subject: libsnmp, SSL: Increase extension buffer size to prevent a crash
+
+Make sure enough space is allocated for extensions longer than 512 bytes. Fix
+the error message when the extension is too long to clearly differentiate the
+error from the out of memory case. Fix a crash when the extension is too long.
+
+[ bvanassche: edited patch description / fixed an off-by-one error / undid a
+  code formatting change ]
+
+Origin: upstream, https://github.com/net-snmp/net-snmp/commit/bb30f8ee0075750fd3648a6bf3fab543f46152ed
+Applied-Upstream: 5.9.1.rc1
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/net-snmp/+bug/1912389
+Bug: https://github.com/net-snmp/net-snmp/issues/233
+Reviewed-By: Sergio Durigan Junior <[e-mail]>
+---
+ snmplib/snmp_openssl.c | 17 +++++++++++------
+ 1 file changed, 11 insertions(+), 6 deletions(-)
+
+diff --git a/snmplib/snmp_openssl.c b/snmplib/snmp_openssl.c
+index 22be003..7d6db6a 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 extenstion\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/debian/patches/series b/debian/patches/series
index 13e1907..970ec14 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -37,3 +37,5 @@ snmpd_conf_5_ro_extend
 Link-libnetsnmptrapd-against-MYSQL_LIBS.patch
 pkgconfig_install_mode
 perl_makefile_man3pods
+lp1912389-libsnmp-Handle-certificate-loading-errors-gracefully.patch
+lp1912389-libsnmp-SSL-Increase-extension-buffer-size-to-preven.patch