Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2fd09fb
test: pin unauthenticated Reports to authenticated SNMPv3 requests
ilyam8 Oct 11, 2026
cfab1f4
test: pin more Reports the REPORT resync must keep apart
ilyam8 Oct 11, 2026
8f3805c
fix: take the engine discovery exception only while discovering
ilyam8 Oct 11, 2026
58e0d83
feat: act on SNMPv3 Reports sent without authentication
ilyam8 Oct 11, 2026
138a15f
test: pin the review's cases of the REPORT resync
ilyam8 Oct 11, 2026
0bdcdda
fix: review findings on the REPORT resync
ilyam8 Oct 11, 2026
efda359
test: drop the allocation test of sendOneRequest
ilyam8 Oct 11, 2026
b2fd144
test: pin the resend of read and write requests after a Report
ilyam8 Oct 11, 2026
cf509ba
fix: let an unauthenticated Report resynchronize only what it cannot …
ilyam8 Oct 11, 2026
9654cfd
test: pin Reports for another message while the client has no engine ID
ilyam8 Oct 11, 2026
c47d85a
fix: bind a privacy-only Report to its msgID while discovering
ilyam8 Oct 11, 2026
db9e6a7
test: pin a Report another agent with the same credentials signed
ilyam8 Oct 11, 2026
9ef9f50
test: pin replies the msgID and version correlation must keep apart
ilyam8 Oct 11, 2026
3ddaa54
fix: judge an SNMPv3 reply against the request it answers
ilyam8 Oct 11, 2026
b633f87
test: pin replies signed with the empty key
ilyam8 Oct 11, 2026
c9f6ad9
fix: never sign or check an SNMPv3 reply with an empty key
ilyam8 Oct 11, 2026
8d7f947
test: pin a reply naming another engine ID signed with the request's key
ilyam8 Oct 11, 2026
b733a31
test: pin what a Report lends a request without authentication
ilyam8 Oct 11, 2026
164e725
test: pin a Logger that changes the client's version before encoding
ilyam8 Oct 11, 2026
b7f4297
test: pin correlation and engine-less discovery paths the reply check…
ilyam8 Oct 11, 2026
3e229e8
fix: refuse an SNMPv3 request without a key whatever the client's ver…
ilyam8 Oct 11, 2026
6eb526e
docs: correct the reply check comments
ilyam8 Oct 11, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 168 additions & 88 deletions engine.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions engine_harness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ func describeV3Message(b []byte) string {
if err != nil {
return fmt.Sprintf("%d octets, SNMPv3, not decodable (%v)", len(b), err)
}
if req.pdu == nil && m.scopedTag != byte(OctetString) {
return "3 " + describeAgentRequest(req) + " scoped PDU not decodable"
}
if req.pdu == nil {
return "3 " + describeAgentRequest(req) + " encrypted"
}
Expand Down
34 changes: 28 additions & 6 deletions engine_requests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,20 @@ var errEngineCause = errors.New("engine test cause")
// msgData is the Report PDU itself, without the scoped PDU around it, so that
// a v1/v2c client reads it as the PDU after the SNMPv3 header.
func bareV3Report(req []byte, counter string) []byte {
return bareV3Answer(req, 7, Report, SnmpPDU{Name: counter, Type: Counter32, Value: uint32(1)})
}

// bareV3Answer is bareV3Report with msgID for a PDU of type pduType with vbs.
func bareV3Answer(req []byte, msgID uint32, pduType PDUType, vbs ...SnmpPDU) []byte {
in, err := decodeMessage(req)
if err != nil {
panic(fmt.Sprintf("agent cannot decode the request: %v", err))
}
out := &SnmpPacket{
Version: Version3, MsgFlags: NoAuthNoPriv, SecurityModel: UserSecurityModel,
SecurityParameters: &UsmSecurityParameters{UserName: "public"},
PDUType: Report, RequestID: in.RequestID, MsgID: 7,
Variables: []SnmpPDU{{Name: counter, Type: Counter32, Value: uint32(1)}},
PDUType: pduType, RequestID: in.RequestID, MsgID: msgID,
Variables: vbs,
}
b, err := out.marshalMsg()
if err != nil {
Expand Down Expand Up @@ -327,9 +332,26 @@ func engineScenarios() map[string]engineScenario {
p.SecurityParameters = &UsmSecurityParameters{UserName: "public"}
}, sysDescr),
},
"answer/other-version-v3-report": {agent: func(_ int, req []byte) []agentReply {
return []agentReply{{data: bareV3Report(req, usmStatsUnknownUserNames)}}
}},
"answer/other-version-v3-report": {
agent: func(_ int, req []byte) []agentReply {
return []agentReply{{data: bareV3Report(req, usmStatsUnknownUserNames)}}
},
},
"answer/other-version-v3-unknown-engine-report": {
agent: func(_ int, req []byte) []agentReply {
return []agentReply{{data: bareV3Report(req, usmStatsUnknownEngineIDs)}}
},
},
"answer/other-version-v3-msg-id-zero": {
agent: func(_ int, req []byte) []agentReply {
return []agentReply{{data: bareV3Answer(req, 0, Report, SnmpPDU{Name: usmStatsUnknownUserNames, Type: Counter32, Value: uint32(1)})}}
},
},
"answer/other-version-v3-get-response": {
agent: func(_ int, req []byte) []agentReply {
return []agentReply{{data: bareV3Answer(req, 7, GetResponse, sysDescr)}}
},
},
"answer/other-community": {agent: answerWith(func(p *SnmpPacket) { p.Community = "private" }, sysDescr)},
"answer/garbage-then-valid": {
agent: func(_ int, req []byte) []agentReply {
Expand Down Expand Up @@ -719,7 +741,7 @@ func BenchmarkSendOneRequest(b *testing.B) {

b.ReportAllocs()
for b.Loop() {
if _, err := x.sendOneRequest(pkt); err != nil {
if _, _, err := x.sendOneRequest(pkt); err != nil {
b.Fatal(err)
}
}
Expand Down
86 changes: 85 additions & 1 deletion engine_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (x *GoSNMP) negotiateInitialSecurityParameters(packetOut *SnmpPacket) error
// engine ID (at authPriv it fails in the encoder for want of a privacy key).
func (x *GoSNMP) discoverEngine(discoveryPacket, packetOut *SnmpPacket) error {
discoveryPacket.ContextName = x.ContextName
result, err := x.sendOneRequest(discoveryPacket)
result, _, err := x.sendOneRequest(discoveryPacket)
if err != nil && !engineFoundDespiteUnknownUser(result, err) {
return err
}
Expand All @@ -60,6 +60,90 @@ func engineFoundDespiteUnknownUser(result *SnmpPacket, err error) bool {
return usp != nil && usp.AuthoritativeEngineID != ""
}

// settleReply stores the engine parameters an SNMPv3 reply to packetOut
// carries, and returns the reply's Report kind (zero for another reply),
// whether its parameters were stored, and the error the request ends with
// unless kind asks for a resync: err, or the store's for a reply other than
// an error Report. A reply is stored when its digest checked against the
// request (verified), so it names the request's engine; an unverified one only
// in a noAuthNoPriv session, and never a Report (RFC 3414 section 3.2 step 7
// takes the engine boots and time from authenticated messages only; discovery,
// which takes the engine ID from an unauthenticated Report, section 4, is
// discoverEngine's). Known bug: a reply with another security model is not
// discarded (RFC 3412 section 7.2 step 4).
func (x *GoSNMP) settleReply(packetOut, reply *SnmpPacket, verified bool, err error) (kind reportKind, stored bool, _ error) {
if reply == nil || reply.Version != Version3 {
return reportKind{}, false, err
}
kind, isReport := reportKindOf(reply)
if !verified && (isReport || packetOut.MsgFlags&AuthNoPriv != 0) {
return kind, false, err
}
if x.Logger.enabled() {
x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %s", reply.SecurityParameters.SafeString())
}
if storeErr := x.storeSecurityParameters(reply); storeErr != nil {
if isReport && !kind.resync {
return kind, false, err
}
return reportKind{}, false, storeErr
}
return kind, true, err
}

// mayLend reports whether report, a resync Report of kind the client did not
// store, may lend its engine parameters to the resend of packetOut. A request
// without authentication carries no signature to misuse. An authenticated
// request is signed with what the Report names, so it takes only the engine
// boots and time of a Report that names the request's engine ID (RFC 3414
// section 3.2 checks the engine ID before the time window), and only as a
// read request, whose copy an on-path sender could replay only as that read
// later (step 7 a accepts a time within 150 s of the agent's). It never takes
// an engine ID: the request would be signed for another engine with the same
// credentials (section 1.5.2).
func mayLend(packetOut, report *SnmpPacket, kind reportKind) bool {
if packetOut.MsgFlags&AuthNoPriv == 0 {
return true
}
switch packetOut.PDUType {
case GetRequest, GetNextRequest, GetBulkRequest:
default:
return false
}
if kind.namesEngine {
return false
}
return report.SecurityParameters.usm().AuthoritativeEngineID == packetOut.SecurityParameters.usm().AuthoritativeEngineID
}

// lendEngineParameters puts the engine parameters of report, a resync Report
// the client does not store, into packetOut only, for one resend: the engine
// ID, boots and time of a Report that names the engine (the packet's keys are
// localized to that engine ID, and the engine ID becomes the context engine
// ID when the packet has none), the boots and time of another (RFC 3414
// section 3.2 checks the engine ID before the time window). The client takes
// the agent's from the authenticated answer.
func (x *GoSNMP) lendEngineParameters(packetOut, report *SnmpPacket, kind reportKind) error {
if x.Version != Version3 || packetOut.Version != Version3 {
return fmt.Errorf("lendEngineParameters called with non Version3 connection or packet")
}
if x.SecurityModel != packetOut.SecurityModel {
return fmt.Errorf("connection security model does not match security model defined in packet")
}
from := report.SecurityParameters.usm()
if !kind.namesEngine {
packetOut.SecurityParameters.usm().setEngineTime(from)
return nil
}
if err := packetOut.SecurityParameters.usm().setSecurityParameters(from); err != nil {
return err
}
if packetOut.ContextEngineID == "" {
packetOut.ContextEngineID = from.AuthoritativeEngineID
}
return nil
}

// save the connection security parameters after a request/response
func (x *GoSNMP) storeSecurityParameters(result *SnmpPacket) error {
if x.Version != Version3 || result.Version != Version3 {
Expand Down
17 changes: 12 additions & 5 deletions engine_v3_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ type agentAnswer struct {
report string // the Report's counter OID, or "" for a GetResponse
level SnmpV3MsgFlags // the security level of the answer
edit func(*SnmpPacket)
raw func(*v3Message) // edits the encoded answer, after edit
drop bool // no answer
raw func(*v3Message) // edits the encoded answer, after edit
encode func(v3Message) []byte // encodes the answer after raw, instead of v3Message.bytes
drop bool // no answer
why string
}

Expand Down Expand Up @@ -184,13 +185,19 @@ func (a *fakeV3Agent) handle(n int, data []byte) []agentReply {
if err != nil {
panic(fmt.Sprintf("agent cannot encode its answer: %v", err))
}
if ans.raw != nil {
if ans.raw != nil || ans.encode != nil {
m, err := parseV3Message(b)
if err != nil {
panic(fmt.Sprintf("agent cannot read its answer: %v", err))
}
ans.raw(&m)
b = m.bytes()
if ans.raw != nil {
ans.raw(&m)
}
if ans.encode != nil {
b = ans.encode(m)
} else {
b = m.bytes()
}
}
return []agentReply{{data: b}}
}
Expand Down
2 changes: 1 addition & 1 deletion engine_v3_fips_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func getFirstOf2(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) {
// v3FIPSScenarios are the scenarios of TestEngineV3FIPS140Only.
func v3FIPSScenarios() map[string]v3Scenario {
const cfbPanic = "AES-CFB panics in FIPS 140-only mode; send recovers it into an error that carries the stack"
const partialState = "a failed key derivation leaves the agent's engine ID adopted without keys, boots or time, so the next request skips the discovery"
const partialState = "a failed key derivation leaves the agent's engine ID adopted without keys, boots or time, so the next request skips the discovery; its error is dropped and the request ends for want of a key"
return map[string]v3Scenario{
"discovery/noauth": {user: "codec-noauth"},
"discovery/md5": {user: "codec-md5", run: getFirstOf2, knownBug: partialState},
Expand Down
Loading
Loading