Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate SNMPv3 Auth/Priv Protocol for incoming trap message #351

Merged
merged 9 commits into from
Oct 9, 2021
Merged

Validate SNMPv3 Auth/Priv Protocol for incoming trap message #351

merged 9 commits into from
Oct 9, 2021

Conversation

balasankarrajaguru
Copy link
Contributor

Closes : #350

Changes

  • Validate the Incoming message flags with the configured authentication and privacy protocol and return an error stating protocol mismatch
root@choc-esx2-vm7 /home/regress/workspace/src/gosnmp/examples/trapserver $ go run main.go
2021/07/20 14:48:09 Listening
Packet sanity verified, we got all the bytes (314)
parseRawField: version
Parsed version 3
parseRawField: msgID
Parsed message ID 959967879
parseRawField: msgMaxSize
Parsed message max size 65507
parseRawField: msgFlags
parsed msg flags
parseRawField: msgSecurityModel
Parsed security model 3
parseRawField: msgAuthoritativeEngineID
Parsed authoritativeEngineID 80000a4c0180dd84d2
parseRawField: msgAuthoritativeEngineBoots
Parsed authoritativeEngineBoots 8
parseRawField: msgAuthoritativeEngineTime
Parsed authoritativeEngineTime 956502
parseRawField: msgUserName
Parsed userName jtac
parseRawField: msgAuthenticationParameters
Parsed authenticationParameters �k[4�YH���
UnmarshalTrap: error parsing SNMPv3 User Security Model: authentication parameters are not configured to parse incoming authenticated message


root@choc-esx2-vm7 /home/regress/workspace/src/gosnmp/examples/trapserver $ go run main.go
2021/07/20 14:44:43 Listening
Packet sanity verified, we got all the bytes (314)
parseRawField: version
Parsed version 3
parseRawField: msgID
Parsed message ID 959967878
parseRawField: msgMaxSize
Parsed message max size 65507
parseRawField: msgFlags
parsed msg flags
parseRawField: msgSecurityModel
Parsed security model 3
parseRawField: msgAuthoritativeEngineID
Parsed authoritativeEngineID 80000a4c0180dd84d2
parseRawField: msgAuthoritativeEngineBoots
Parsed authoritativeEngineBoots 8
parseRawField: msgAuthoritativeEngineTime
Parsed authoritativeEngineTime 956301
parseRawField: msgUserName
Parsed userName jtac
parseRawField: msgAuthenticationParameters
Parsed authenticationParameters �n@�"�Q\x

parseRawField: msgPrivacyParameters
Parsed privacyParametersV/
UnmarshalTrap: error parsing SNMPv3 User Security Model: privacy parameters are not configured to parse incoming encrypted message

@balasankarrajaguru
fix auth priv check
7a43e7a 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
fix trailing whitespace
8976ebe 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@TimRots TimRots requested a review from SuperQ July 25, 2021 22:03
@TimRots
Copy link
Member

TimRots commented Jul 25, 2021 •
edited
Loading

@SuperQ can you give this one a final review please. This PR fixes the panic observed in #274 as well.

@TimRots TimRots linked an issue Jul 27, 2021 that may be closed by this pull request
Trap Listener panics on a v3 packet if not configured for v3 #274
Closed
SuperQ
SuperQ requested changes Aug 28, 2021
View reviewed changes
v3_usm.go Outdated Show resolved Hide resolved
@balasankarrajaguru
change error message initialization
4711e8b 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
Copy link
Contributor Author

ci retry

1 similar comment
@balasankarrajaguru
Copy link
Contributor Author

ci retry

@balasankarrajaguru
change error message initialization
8cc8b43 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
Merge branch 'v3_auth_priv_dev' of github.com:balasankarrajaguru/gosn…
57627ad 
…mp into v3_auth_priv_dev
@SuperQ
Copy link
Contributor

SuperQ commented Sep 6, 2021

You need to rebase your branch against the latest commit from #355. That should fix the tests. There was a CI container bug due to the release of Debian 11.

@balasankarrajaguru
fix auth priv check
5fe05ec 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
fix trailing whitespace
66e51e3 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
change error message initialization
0d8fe42 
Signed-off-by: Balasankar Rajaguru <balasank@juniper.net>
@balasankarrajaguru
Merge branch 'v3_auth_priv_dev' of github.com:balasankarrajaguru/gosn…
7a14a07 
…mp into v3_auth_priv_dev
@balasankarrajaguru
Copy link
Contributor Author

@SuperQ Thanks, I have updated the changes and addressed the comments. Please review

@SuperQ
Copy link
Contributor

SuperQ commented Sep 9, 2021

Thanks! It would be great to add these two cases to v3_usm_test.go.

@TimRots TimRots mentioned this pull request Oct 7, 2021
Open
SuperQ
SuperQ approved these changes Oct 9, 2021
View reviewed changes
Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SuperQ
Copy link
Contributor

SuperQ commented Oct 9, 2021

We can do cleanup and add tests after merging.

@SuperQ SuperQ merged commit 4087aea into gosnmp:master Oct 9, 2021
TimRots added a commit to TimRots/gosnmp that referenced this pull request Oct 14, 2021
@TimRots
Release v1.33.0
1533d3f 
* [BUGFIX] parseLength: avoid OOB read, prevent panic gosnmp#354
* [FEATURE] Add LocalAddr setting to bind source address of SNMP queries gosnmp#342
* [ENHANCEMENT] Validate SNMPv3 Auth/Priv Protocol for incoming trap message gosnmp#351
* [ENHANCEMENT] helper.go: add error handling to parseLength gosnmp#358
* [ENHANCEMENT] Rename v3_testing_credentials to avoid testing import in prod builds gosnmp#360
* [ENHANCEMENT] helper.go: Improved decodeValue() function gosnmp#340

Signed-off-by: Tim Rots <tim.rots@protonmail.ch>
@TimRots TimRots mentioned this pull request Oct 14, 2021
Merged
TimRots added a commit to TimRots/gosnmp that referenced this pull request Oct 19, 2021
@TimRots
Release v1.33.0
533d69d 
* [BUGFIX] parseLength: avoid OOB read, prevent panic gosnmp#354
* [FEATURE] Add LocalAddr setting to bind source address of SNMP queries gosnmp#342
* [ENHANCEMENT] Validate SNMPv3 Auth/Priv Protocol for incoming trap message gosnmp#351
* [ENHANCEMENT] helper.go: add error handling to parseLength gosnmp#358
* [ENHANCEMENT] Rename v3_testing_credentials to avoid testing import in prod builds gosnmp#360
* [ENHANCEMENT] helper.go: Improved decodeValue() function gosnmp#340

Signed-off-by: Tim Rots <tim.rots@protonmail.ch>
TimRots added a commit that referenced this pull request Oct 19, 2021
@TimRots
Release v1.33.0 (#367)
96366f3 
* [BUGFIX] parseLength: avoid OOB read, prevent panic #354
* [BUGFIX] Detect negative lengths in parseLength, prevent panic #369 
* [FEATURE] Add LocalAddr setting to bind source address of SNMP queries #342
* [ENHANCEMENT] Validate SNMPv3 Auth/Priv Protocol for incoming trap message #351
* [ENHANCEMENT] helper.go: add error handling to parseLength #358
* [ENHANCEMENT] Rename v3_testing_credentials to avoid testing import in prod builds #360
* [ENHANCEMENT] helper.go: Improved decodeValue() function #340

Signed-off-by: Tim Rots <tim.rots@protonmail.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperQ SuperQ SuperQ approved these changes

@TimRots TimRots TimRots approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@balasankarrajaguru @TimRots @SuperQ