Merge ~sergiodj/ubuntu/+source/net-snmp:double-free-bug1877027-groovy into ubuntu/+source/net-snmp:ubuntu/devel
Status: | Rejected |
---|---|
Rejected by: | Andreas Hasenack on 2020-07-03 |
Proposed branch: | ~sergiodj/ubuntu/+source/net-snmp:double-free-bug1877027-groovy |
Merge into: | ubuntu/+source/net-snmp:ubuntu/devel |
Diff against target: |
872 lines (+820/-0) 8 files modified
debian/changelog (+21/-0) debian/patches/fix-usmStateReference-free.patch (+31/-0) debian/patches/introduce-refcount-usmStateReference.patch (+169/-0) debian/patches/move-free-securityStateRef-into-snmp-free-pdu.patch (+92/-0) debian/patches/move-securityStateRef-into-free_securityStateRef.patch (+78/-0) debian/patches/prevent-snmpv3-bulkget-errors-double-free.patch (+349/-0) debian/patches/series (+6/-0) debian/patches/unexport-struct-usmStateReference.patch (+74/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andreas Hasenack | 2020-06-23 | Needs Information on 2020-06-24 | |
Canonical Server Team | 2020-06-23 | Pending | |
Review via email:
|
Description of the change
This series of patches fix a segmentation fault that occurs with snmpd on groovy when the user requests an erroneous bulkget operation using the snmpv3 protocol. It took me a while to figure out the right set of patches that needed to be backported in order to address this issue, mostly because this is my first rodeo with snmp but also because upstream addressed this using different bug numbers.
To reproduce the issue, one can do:
$ lxc launch ubuntu-daily:groovy net-snmp-bug1877027-groovy
$ lxc shell net-snmp-bug1877027-groovy
# apt update
# apt install -y snmpd snmp
# systemctl stop snmpd.service
# cat >> /var/lib/
createUser testuser SHA "testpass" AES "testpass"
__EOF__
# cat >> /etc/snmp/
rwuser testuser
__EOF__
# systemct start snmpd.service
# snmpbulkget -v3 -Cn1 -Cr1472 -l authPriv -u testuser -a SHA -A testpass -x AES -X testpass 127.0.0.1 1.3.6.1.2.1.1.5 1.3.6.1.2.1.1.7
You can check that snmpd crashed by doing:
# systemctl status snmpd.service
There is a PPA with the proposed fix here:
https:/
After installing it, one can check that the fix worked by doing:
# snmpbulkget -v3 -Cn1 -Cr1472 -l authPriv -u testuser -a SHA -A testpass -x AES -X testpass 127.0.0.1 1.3.6.1.2.1.1.5 1.3.6.1.2.1.1.7
Error in packet.
Reason: (genError) A general failure occured
Initially, I wasn't sure if this error should have happened or not. However, after compiling the upstream project and running it here, I noticed that it also displays this error message.
One of the persons interested in the bug was able to try the PPA and confirmed that it solves the segmentation fault for him.
autopkgtest is still happy:
autopkgtest [16:07:24]: test command1: perl -MSNMP -e1
autopkgtest [16:07:24]: test command1: [------
autopkgtest [16:07:24]: test command1: -------
autopkgtest [16:07:25]: test command1: - - - - - - - - - - results - - - - - - - - - -
command1 PASS
autopkgtest [16:07:26]: @@@@@@@
command1 PASS
Andreas Hasenack (ahasenack) wrote : | # |
Andreas Hasenack (ahasenack) wrote : | # |
First pass:
- please use full urls for the Origin tag in the DEP3 header, it helps a lot to verify what upstream committed and what we are shipping
- it's a bit troublesome that the bulk get command still fails. I'm by far not an snmp expert, but "error in packet" doesn't look like a normal error, but could indicate that something is corrupted, i.e., another bug, maybe a new one, or introduced by these changes. Could you clarify with upstream, file a bug there, something like this?
I also pinged #security, as I think this is a remote DoS that can be triggered by authenticated users.
Sergio Durigan Junior (sergiodj) wrote : | # |
On Wednesday, June 24 2020, Andreas Hasenack wrote:
> First pass:
> - please use full urls for the Origin tag in the DEP3 header, it helps a lot to verify what upstream committed and what we are shipping
Ah, sorry about that. I have now updated the Origin tag with the
complete URL.
> - it's a bit troublesome that the bulk get command still fails. I'm by
> far not an snmp expert, but "error in packet" doesn't look like a
> normal error, but could indicate that something is corrupted, i.e.,
> another bug, maybe a new one, or introduced by these changes. Could
> you clarify with upstream, file a bug there, something like this?
Yep, I also found it strange, and went to great lengths to verify that
this was indeed "normal". As far as I have checked, the error is
expected in this case. The upstream binary (without any of our patches,
compiled directly from the V5-8-patches branch) also displays the error,
and the Fedora net-snmp package too.
I am trying to confirm with upstream that this is OK, and if they say
it's not then I will certainly file a bug.
> I also pinged #security, as I think this is a remote DoS that can be triggered by authenticated users.
Thanks.
--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14
Andreas Hasenack (ahasenack) wrote : | # |
The origin links are missing a /commit/ url component, the way they are now return a 404
Sergio Durigan Junior (sergiodj) wrote : | # |
On Wednesday, June 24 2020, Andreas Hasenack wrote:
> The origin links are missing a /commit/ url component, the way they are now return a 404
You beat me to it: I have just noticed and updated these links a few
minutes ago, before you commented.
--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14
Sergio Durigan Junior (sergiodj) wrote : | # |
On Wednesday, June 24 2020, Andreas Hasenack wrote:
> Diff comments:
>
>> diff --git a/debian/
>> new file mode 100644
>> index 0000000..f5d0df8
>> --- /dev/null
>> +++ b/debian/
>> @@ -0,0 +1,349 @@
>> +From: Sam Tannous <email address hidden>
>> +Date: Wed, 10 Apr 2019 06:57:21 -0700
>> +Subject: snmpd: Avoid that snmpv3 bulkget errors result in a double free
>> +
>> +See also https:/
>> +See also https:/
>> +
>> +Author: Ming Chen <email address hidden>
>> +Origin: upstream, https:/
>
> This origin URL does not match the patch content below. Did this come from several commits?
Ah, it seems I pasted the wrong commit hash. The right one should be
https:/
--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14
Christian Ehrhardt (paelzer) wrote : | # |
Let us set this back to WIP (to clear the overview) until it is resolved either way
Sergio Durigan Junior (sergiodj) wrote : | # |
On Friday, July 03 2020, Christian Ehrhardt wrote:
> Let us set this back to WIP (to clear the overview) until it is resolved either way
Thanks, Christian.
Yesterday the security team uploaded my fix to Groovy:
https:/
So I believe we should close this MP.
Thanks,
--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14
Unmerged commits
- 2e5ec2f... by Sergio Durigan Junior on 2020-06-23
- afe5dd9... by Sergio Durigan Junior on 2020-06-19
Will look at this tomorrow.