Upgrade to gosnmp mainline with our bespoke changes #2

sacredbanana · 2022-08-22T09:33:33Z

This is taking the latest mainline gosnmp then porting our bespoke changes on top. It's a lot of incoming changes. You can see my specific changes by looking at the latest commits.

https://app.clickup.com/t/1t208he

* Add SnmpEncodePacket & SnmpDecodePacket * Cleanup * Return errors to caller vs log * Relocate functions and add comments * Time for more ☕, fix the comment to match the function

* Store MsgMaxSize, support SNMPv3 SnmpDecodePacket * Export MsgMaxSize * Add SetMsgID/SetRequestID, validate Encode/Decode

Error handling is required. Otherwise, a nil trap will be passed into function unmarshalPayload, which triggers a panic.

* Removed bit-lopping * revamped to adhere to BER

* Rerunning of mockgen off of interface.go * Remove self-import

mockgen -source=interface.go -package gosnmp > gosnmp_mock.go

MrSpock's fix - issue 179

Issue 183

Issue 182

RFC 3430 specifies the transport of SNMP over TCP. It can be used to support more efficient bulk transfers. This is supported in net-snmp and the snmpd from OpenBSD for example. This commit adds the field Transport to the GoSNMP struct that allows the selection of UDP or TCP as transport layer. Connect() uses the specified method to query the server. The interface of the library remains the same. As the connection can be closed at any time by the server, we need to handle an EOF from the connection gracefully. This change also adds an example to walk using TCP. It was tested against the snmpd from OpenBSD.

…some Huawei devices)" This reverts commit 8f085b5.

…wei devices)

…e SNMPv3 configs

sacredbanana · 2022-08-23T11:14:02Z

@initialed85 I have now updated the PR so that we don't have to do any resets. I did a clean merge involving reverting some of our commits

go.mod

Dockerfile

examples/example/main.go

examples/example2/main.go

gosnmp_api_test.go

examples/walktcpexample/main.go

examples/walkexample/main.go

examples/trapserver/main.go

examples/trapsend_v1/main.go

examples/trapsend_v3/main.go

examples/trapsend_v2plus/main.go

examples/tcp_trapserver/main.go

examples/logger/main.go

examples/linux_vrf/main.go

examples/example3/main.go

initialed85 · 2022-08-23T12:08:45Z

I've been having a bit of a look in more detail at the commits we've made in our fork- here's a starting point for a catalogue:

Improvements to support for devices behind NAT aka handling an SNMP response from an unexpected IP
- a49e31d
Don't bother retrying during DNS failure
- 6c129b0
- b0c344c
Fix module name for go.mod
- 089b4b5
Support for devices behind NAT aka handling an SNMP response from an unexpected IP
- 4a6d15b
Support for SNMPSET of type IPAddress
- cdbbd56
- 4eb9535
Increase buffer size for large packets
- 8c8e565
Support multiple params for SNMP traps
Hack around SNMPv3 auth issues for Huawei devices
- 8f085b5

From a brief squiz most of those have had an alternative implemented in upstream (and so for better or worse, I guess we should use that)- however for sure the "Support multiple params for SNMP traps" functionality doesn't have a counterpart.

This one is useful and I suspect not well tested in CI, it speaks to having more than one community string (or a mixture of SNMPv2 and SNMPv3) configured for an SNMP trap listener- so if we get rid of that behaviour we risk finding out the hard way that we needed it (in prod).

And as this isn't Python, it's not as easy as just slamming some code into a running environment to fix it (we'd need to copy built libraries deep into a Virtualenv inside the backend container).

So it might give the perception of lost velocity, but I think we need to take an approach here of the following:

double check my cataloguing above, make sure I've identified the theme of the changes
extend our automated tests to be able to prove each new piece of functionality
go through our "sync w/ upstream" process
see if our tests still pass

It's really the only way we can have any confidence I think.

Stream of consciousness:

[Improvements to] Support for devices behind NAT aka handling an SNMP response from an unexpected IP
- We can take inspiration from https://github.com/ftpsolutions/ims-mono/blob/master/e2e_tests/dev_env_3/docker-compose.yml or https://github.com/ftpsolutions/ims-mono/tree/master/spikes/nats_cluster_replication for recreating a NAT'd environment
Don't bother retrying during DNS failure
- Hopefully we can test by settings timeouts and retries and having a test that tries to hit an unresolvable name
Fix module name for go.mod
- Unimportant
Support for SNMPSET of type IPAddress
- Not sure if snmpsimd (the Python one we use) supports SNMPSETs; might have to put together an snmpd Docker container- totally doable though I think
Increase buffer size for large packets
- Just fake up a really big SNMP packet to be returned from an SNMP simulator and make sure it can be received
Support multiple params for SNMP traps
- We just need a couple of SNMP trap "simulators" (such as the gosnmp-traps-python uses, it's just invocations of the SNMP trap shell command) to exercise some different security combos (e.g. different community strings, SNMPv3, etc)
Hack around SNMPv3 auth issues for Huawei devices
- This is the hardest one to test, because I could never make head nor tail of the actual issue, I was just able to track it down to that part of the code and skip the logic
- I have noticed there are changes to that part of the code- maybe it's fixed now?
- At any rate, I think we'll struggle to test this in an automated way- we might have to rely on manual validation and hope that South32 Worsley are still running Huawei AR503s (perhaps Shane Stuart can confirm)

EDIT: I think https://github.com/ftpsolutions/ims-mono/pull/2237 will cover us for test cases (except for the manual AR503 one)

hdm and others added 30 commits July 26, 2018 17:57

Expose SnmpEncodePacket/SnmpDecodePacket (gosnmp#157)

b788382

* Add SnmpEncodePacket & SnmpDecodePacket * Cleanup * Return errors to caller vs log * Relocate functions and add comments * Time for more ☕, fix the comment to match the function

Add mock for Handler interface

6d59348

Merge 'gosnmp#158 Add mock for Handler interface'

2b34397

Support SNMPv3 Encode/Decode (gosnmp#160)

3651b0e

* Store MsgMaxSize, support SNMPv3 SnmpDecodePacket * Export MsgMaxSize * Add SetMsgID/SetRequestID, validate Encode/Decode

go 1.11 - travis

35d70ef

decrypt packet error handling

c0ed509

Error handling is required. Otherwise, a nil trap will be passed into function unmarshalPayload, which triggers a panic.

marshalInt32()

28507a5

Removed bit-lopping (gosnmp#169)

f45ace4

* Removed bit-lopping * revamped to adhere to BER

exponential timeout + doco; goimports

cd9a07a

fix incorrect logPrint & logPrintf usage (gosnmp#174)

0aea62d

Rerunning of mockgen off of interface.go (gosnmp#175)

74bb830

Remove self-import introduced from previous Mockgen PR (gosnmp#177)

922fbec

* Rerunning of mockgen off of interface.go * Remove self-import

Revise the bug for func parseObjectIdentifier

14015a2

revise 'bug for func parseObjectIdentifier'; gofmt

b77a017

update mocks to new gomock version

843be2d

mockgen -source=interface.go -package gosnmp > gosnmp_mock.go

go mod

15d2ec2

changelog

68e8bea

travis config housekeeping

96c5485

Asn1BER - types on constants; stringer

ac5c52d

SNMPError - stringer

ee3a357

handle uninitialized v3 SecurityParameters in SnmpDecodePacket()

b0ab007

changelog

027c4b0

fix IPAddress marshalling in Set()

b91861a

MrSpock's fix - issue 179

use packetSecParams not sp secretKey in v3 isAuthentic()

28249c3

Issue 183

goimports

36de603

SNMPv1 trap IDs should be marshalInt32() not single byte

a89cf94

Issue 182

CHANGELOG

f16c818

issue132 don't check returned OIDs are increasing

ea8c875

PR187 allow input of auth, priv keys

686d663

sacredbanana added 9 commits August 23, 2022 18:45

Revert "Defeat testAuthentication for SNMPv3 (for compatibility with …

6f82f81

…some Huawei devices)" This reverts commit 8f085b5.

Merge remote-tracking branch 'gosnmp/master'

151519b

Change module to sacredbanana

9124327

Defeat testAuthentication for SNMPv3 (for compatibility with some Hua…

1ab883e

…wei devices)

Change Params to a slice of *gosnmp.GoSNMP- permits us to use multipl…

07d5ec7

…e SNMPv3 configs

Break out of params loop if unmarshaling is successful

a6cb467

Add .GetConn() method for testing state of TrapListener externally

0654f24

Increase buffer size for listener

54f71f7

Fix local tests

d97a4cc

sacredbanana force-pushed the mainline branch from 4b23ecb to d97a4cc Compare August 23, 2022 11:12