From 2ce5621fc73500729a908914d5be776fe880cefc Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:07:52 +0300 Subject: [PATCH 1/7] test: pin send's resynchronization after a notInTimeWindow Report Mutation probes on send's restructuring found its resynchronization path unpinned in four places: - resync/panic-in-retransmission: a panic during the retransmission is returned as the recover error together with the Report, the result the request had when it panicked (send's named results). - resync/security-model-changed-by-hook: a hook that changes the client's security model before the retransmission makes the packet update fail, and the request returns its error. - resync/retransmission-answered-by-report: a Report answering the retransmission is returned with ErrNotInTimeWindow (known bug). - resync/retransmission-answer-time: the engine time of the answer to the retransmission is not stored (known bug). All four pass on the current code. --- engine_v3_test.go | 58 +++++++++++++++++++ testdata/engine/v3.golden | 116 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) diff --git a/engine_v3_test.go b/engine_v3_test.go index 4108856..b08f331 100644 --- a/engine_v3_test.go +++ b/engine_v3_test.go @@ -245,6 +245,64 @@ func v3Scenarios() map[string]v3Scenario { }, knownBug: "a notInTimeWindow Report answering the resynchronized retransmission is returned with a nil error", }, + // send's resynchronization after a notInTimeWindow Report. + "resync/panic-in-retransmission": { + user: "codec-md5", + script: onRequest(2, agentAnswer{report: usmStatsNotInTimeWindows, level: AuthNoPriv, why: "not in time window"}), + run: func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { + preSend, calls := x.PreSend, 0 + x.PreSend = func(x *GoSNMP) { + preSend(x) + if calls++; calls == 3 { + a.tr.addf("hook PreSend panics in the retransmission") + panic("hook failure") + } + } + return x.Get([]string{engineOID}) + }, + knownBug: "send recovers a panic into an error, with the result the request had when it panicked", + }, + "resync/security-model-changed-by-hook": { + user: "codec-md5", + script: onRequest(2, agentAnswer{report: usmStatsNotInTimeWindows, level: AuthNoPriv, why: "not in time window"}), + run: func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { + onFinish, calls := x.OnFinish, 0 + x.OnFinish = func(x *GoSNMP) { + onFinish(x) + if calls++; calls == 2 { + x.SecurityModel = 2 + a.tr.addf("hook OnFinish sets the client's security model to 2") + } + } + return x.Get([]string{engineOID}) + }, + }, + "resync/retransmission-answered-by-report": { + user: "codec-noauth", script: func(n int, _ agentRequest) (agentAnswer, bool) { + switch n { + case 2: + return agentAnswer{report: usmStatsNotInTimeWindows, why: "not in time window"}, true + case 3: + return agentAnswer{report: usmStatsUnknownUserNames, why: "unknown user"}, true + } + return agentAnswer{}, false + }, + knownBug: "the retransmission's error is replaced by ErrNotInTimeWindow", + }, + "resync/retransmission-answer-time": { + user: "codec-md5", script: func(n int, _ agentRequest) (agentAnswer, bool) { + switch n { + case 2: + return agentAnswer{report: usmStatsNotInTimeWindows, level: AuthNoPriv, why: "not in time window"}, true + case 3: + return agentAnswer{level: AuthNoPriv, why: "a later engine time", edit: func(p *SnmpPacket) { + p.SecurityParameters.usm().AuthoritativeEngineTime += 7 + }}, true + } + return agentAnswer{}, false + }, + knownBug: "the engine boots and time of the answer to the retransmission are not stored", + }, "engine-change": { user: "codec-md5", run: getTwice(func(a *fakeV3Agent) { a.setEngineID(agentOtherEngineID) }), knownBug: "the unauthenticated unknownEngineID Report fails the digest check and is discarded, so the client never adopts the engine ID", diff --git a/testdata/engine/v3.golden b/testdata/engine/v3.golden index 1846b12..6c6c0a2 100644 --- a/testdata/engine/v3.golden +++ b/testdata/engine/v3.golden @@ -2335,6 +2335,122 @@ 0s result: packet 3 msgID=2002 noAuthNoPriv Report id=1002 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-noauth"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.6.3.15.1.1.5.0=Counter32:1], error "wrong digest" *errors.errorString (equals ErrWrongDigest) 0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=none context engine="\x80\x00\x1f\x88\x04codec-agent" +== v3/resync/panic-in-retransmission +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at authNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s hook PreSend panics in the retransmission +0s result: packet 3 msgID=2002 authNoPriv Report id=1002 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1], error "recover: hook failure Stack: ..." *errors.errorString +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: send recovers a panic into an error, with the result the request had when it panicked + +== v3/resync/retransmission-answer-time +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at authNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with GetResponse id=1003 msgID=2003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] at authNoPriv (a later engine time) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1007 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetResponse id=1003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] +0s hook OnRecv +0s hook OnFinish +0s result: packet 3 msgID=2003 authNoPriv GetResponse id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1007 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"], no error +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: the engine boots and time of the answer to the retransmission are not stored + +== v3/resync/retransmission-answered-by-report +0s client: user="codec-noauth" noAuthNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 noAuthNoPriv+reportable user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at noAuthNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 noAuthNoPriv user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 noAuthNoPriv+reportable user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with Report id=1003 msgID=2003 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1] at noAuthNoPriv (unknown user) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 noAuthNoPriv user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1003 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1] +0s hook OnRecv +0s result: packet 3 msgID=2003 noAuthNoPriv Report id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-noauth"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1], error "not in time window" *errors.errorString (equals ErrNotInTimeWindow) +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=none context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: the retransmission's error is replaced by ErrNotInTimeWindow + +== v3/resync/security-model-changed-by-hook +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at authNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s hook OnFinish sets the client's security model to 2 +0s result: packet nil, error "connection security model does not match security model extracted from packet" *errors.errorString +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" + == v3/time-window/always-not-in-time-window 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 0s deadline +1s From 6ebd81250e6d4ed77d4d0aba1b5a68d1fca031fd Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:10:23 +0300 Subject: [PATCH 2/7] refactor: one resync path in send and the SNMPv3 negotiation in its steps send handled the two Reports that resynchronize the client, notInTimeWindow and unknownEngineID, in two copies of the same branch, and recognized them with its own switch beside the table answers used. - reportKinds replaces reportErrors: each Report counter gives the request's error, whether it resynchronizes the client and its name in log lines; reportKindOf serves answers and send. - send keeps the boundary (recover with its named results, the Conn and Retries checks, the negotiation) and hands a resync Report to resync, which updates the packet, sends it again and returns the Report's error if that fails. - negotiateInitialSecurityParameters discovers the engine (discoverEngine) or derives the keys; engineFoundDespiteUnknownUser names the tolerance for devices that answer discovery with usmStatsUnknownUserNames. Its two guards are gone: mkSnmpPacket copies the client's version and security model, send negotiates only for an SNMPv3 packet, and no hook runs in between. Each known bug of these functions keeps one site with a known-bug comment, setSecurityParameters' included; the stale RFC 2574 comment is replaced. No behavior change: goldens and logger output unchanged. --- engine.go | 234 ++++++++++++++++++++++++++++++------------------------ v3.go | 71 ++++++++--------- v3_usm.go | 4 +- 3 files changed, 166 insertions(+), 143 deletions(-) diff --git a/engine.go b/engine.go index 2f960b7..e41d0fa 100644 --- a/engine.go +++ b/engine.go @@ -14,6 +14,91 @@ import ( "time" ) +// send runs a request: for SNMPv3 it first discovers the agent's engine or +// derives the keys, then sends the request, and sends it once more when the +// answer is a Report that resynchronizes the client with the agent's engine. +func (x *GoSNMP) send(packetOut *SnmpPacket) (result *SnmpPacket, err error) { + // Known bug: a panic becomes the request's error, with the stacks of all + // goroutines in its text (8 KB, NUL-padded), and the result the request had + // when it panicked. + defer func() { + if e := recover(); e != nil { + buf := make([]byte, 8192) + runtime.Stack(buf, true) + + err = fmt.Errorf("recover: %v Stack:%v", e, string(buf)) + } + }() + + if x.Conn == nil { + return nil, fmt.Errorf("&GoSNMP.Conn is missing. Provide a connection or use Connect()") + } + if x.Retries < 0 { + x.Retries = 0 + } + if x.Logger.enabled() { + x.Logger.Print("SEND INIT") + } + if packetOut.Version == Version3 { + if x.Logger.enabled() { + x.Logger.Print("SEND INIT NEGOTIATE SECURITY PARAMS") + } + if err = x.negotiateInitialSecurityParameters(packetOut); err != nil { + return &SnmpPacket{}, err + } + if x.Logger.enabled() { + x.Logger.Print("SEND END NEGOTIATE SECURITY PARAMS") + } + } + + result, err = x.sendOneRequest(packetOut) + if err != nil { + // Known bug: the engine boots and time of an authenticated error Report + // are not stored. + x.Logger.Printf("SEND Error on the first Request Error: %s", err) + return result, err + } + if result.Version != Version3 { + return result, nil + } + + if x.Logger.enabled() { + x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %s", result.SecurityParameters.SafeString()) + } + err = x.storeSecurityParameters(result) + if kind, ok := reportKindOf(result); ok && kind.resync { + // Known bug: the store error is dropped, so a Report with another + // security model is acted on instead of being discarded. + return x.resync(packetOut, kind) + } + // Known bug: a reply with another security model is returned together + // with the store error. + return result, err +} + +// resync sends packetOut again after a Report of kind, with the engine ID or +// time the client stored from it. Known bugs: a failed retransmission returns +// the Report's error instead of its own; a Report answering the +// retransmission is returned with a nil error; the engine boots and time of +// the answer to the retransmission are not stored; the client sends the engine +// time of the last message it received, not the current one (RFC 3414 section +// 3.1 step 6 a), so a request after 150 s of silence goes through this resync. +func (x *GoSNMP) resync(packetOut *SnmpPacket, kind reportKind) (*SnmpPacket, error) { + if x.Logger.enabled() { + x.Logger.Print("WARNING detected " + kind.name + " ERROR") + } + if err := x.updatePktSecurityParameters(packetOut); err != nil { + x.Logger.Printf("ERROR updatePktSecurityParameters error: %s", err) + return nil, err + } + result, err := x.sendOneRequest(packetOut) + if err != nil { + x.Logger.Printf("ERROR %s retransmit error: %s", kind.name, err) + return result, kind.err + } + return result, nil +} + // exchange is one request on its way through the attempts sendOneRequest // makes. type exchange struct { @@ -254,123 +339,66 @@ func (e *exchange) decode(resp []byte) (*SnmpPacket, error) { } // answers reports whether reply answers the request, and with which error: a -// reply answers with the current attempt's request ID or one of earlierIDs. -// Known bugs: nothing else of the reply is compared with the request (its -// version, PDU type and msgID, nor the security model, level, user, engine ID -// and context of RFC 3412 section 7.2 step 12 b); an empty reply without an -// error status and a Report answer before their request ID is checked; request -// ID 0 answers any request; a Report counts only with exactly one varbind, so -// one with more is returned as a successful reply. +// reply answers with the current attempt's request ID or one of earlierIDs; +// a Report gives its kind's error, none for a resync Report. Known bugs: +// nothing else of the reply is compared with the request (its version, PDU +// type and msgID, nor the security model, level, user, engine ID and context +// of RFC 3412 section 7.2 step 12 b); an empty reply without an error status +// and a Report answer before their request ID is checked; request ID 0 answers +// any request. func (e *exchange) answers(reply *SnmpPacket, earlierIDs []uint32) (bool, error) { if reply.Error == NoError && len(reply.Variables) < 1 { e.x.Logger.Printf("ERROR on UnmarshalPayload on v3: Empty result") return true, nil } - if reply.Version == Version3 && reply.PDUType == Report && len(reply.Variables) == 1 { - err, ok := reportErrors[reply.Variables[0].Name] - if !ok { - err = ErrUnknownReportPDU + if kind, ok := reportKindOf(reply); ok { + if kind.resync { + return true, nil } - return true, err + return true, kind.err } id := reply.RequestID return id == e.packet.RequestID || slices.Contains(earlierIDs, id) || id == 0, nil } -// reportErrors maps the counter an SNMPv3 Report carries (the USM counters of -// RFC 3414, the message processing counters of RFC 3412) to the request's -// error. The agent puts the counter of the error it detected in the Report's -// varbinds; the Report's request ID is the request's, or 0 when the agent -// could not read it. Reports about the time window and an unknown engine ID -// give no error: the caller takes the engine parameters they carry, and send, -// when it is the caller, sends the request again. -var reportErrors = map[string]error{ //nolint:gochecknoglobals // a read-only table - usmStatsUnsupportedSecLevels: ErrUnknownSecurityLevel, - usmStatsNotInTimeWindows: nil, - usmStatsUnknownUserNames: ErrUnknownUsername, - usmStatsUnknownEngineIDs: nil, - usmStatsWrongDigests: ErrWrongDigest, - usmStatsDecryptionErrors: ErrDecryption, - snmpUnknownSecurityModels: ErrUnknownSecurityModels, - snmpInvalidMsgs: ErrInvalidMsgs, - snmpUnknownPDUHandlers: ErrUnknownPDUHandlers, +// reportKind is what the counter an SNMPv3 Report carries means to a request. +type reportKind struct { + // err is the request's error. + err error + // resync marks the Reports that carry the agent's engine ID or time: the + // request does not end with err, the caller takes the engine parameters + // they carry and send sends the request again, returning err only if that + // fails. + resync bool + // name names a resync Report in log lines. + name string } -// generic "sender" that negotiate any version of snmp request -func (x *GoSNMP) send(packetOut *SnmpPacket) (result *SnmpPacket, err error) { - defer func() { - if e := recover(); e != nil { - buf := make([]byte, 8192) - runtime.Stack(buf, true) - - err = fmt.Errorf("recover: %v Stack:%v", e, string(buf)) - } - }() - - if x.Conn == nil { - return nil, fmt.Errorf("&GoSNMP.Conn is missing. Provide a connection or use Connect()") - } - - if x.Retries < 0 { - x.Retries = 0 - } - if x.Logger.enabled() { - x.Logger.Print("SEND INIT") - } - if packetOut.Version == Version3 { - if x.Logger.enabled() { - x.Logger.Print("SEND INIT NEGOTIATE SECURITY PARAMS") - } - if err = x.negotiateInitialSecurityParameters(packetOut); err != nil { - return &SnmpPacket{}, err - } - if x.Logger.enabled() { - x.Logger.Print("SEND END NEGOTIATE SECURITY PARAMS") - } - } +// reportKinds are the counters of the USM (RFC 3414) and of message processing +// (RFC 3412) a Report can carry. +var reportKinds = map[string]reportKind{ //nolint:gochecknoglobals // a read-only table + usmStatsUnsupportedSecLevels: {err: ErrUnknownSecurityLevel}, + usmStatsNotInTimeWindows: {err: ErrNotInTimeWindow, resync: true, name: "out-of-time-window"}, + usmStatsUnknownUserNames: {err: ErrUnknownUsername}, + usmStatsUnknownEngineIDs: {err: ErrUnknownEngineID, resync: true, name: "unknown engine id"}, + usmStatsWrongDigests: {err: ErrWrongDigest}, + usmStatsDecryptionErrors: {err: ErrDecryption}, + snmpUnknownSecurityModels: {err: ErrUnknownSecurityModels}, + snmpInvalidMsgs: {err: ErrInvalidMsgs}, + snmpUnknownPDUHandlers: {err: ErrUnknownPDUHandlers}, +} - // perform request - result, err = x.sendOneRequest(packetOut) - if err != nil { - x.Logger.Printf("SEND Error on the first Request Error: %s", err) - return result, err +// reportKindOf returns the kind of p as a Report, and false when p is not one. +// The agent puts the counter of the error it detected in a Report's varbinds; +// the Report's request ID is the request's, or 0 when the agent could not read +// it. An unknown counter gives ErrUnknownReportPDU. Known bug: a Report counts +// only with exactly one varbind, so one with more is a successful reply. +func reportKindOf(p *SnmpPacket) (reportKind, bool) { + if p.Version != Version3 || p.PDUType != Report || len(p.Variables) != 1 { + return reportKind{}, false } - - if result.Version == Version3 { - if x.Logger.enabled() { - x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %s", result.SecurityParameters.SafeString()) - } - err = x.storeSecurityParameters(result) - - if result.PDUType == Report && len(result.Variables) == 1 { - switch result.Variables[0].Name { - case usmStatsNotInTimeWindows: - x.Logger.Print("WARNING detected out-of-time-window ERROR") - if err = x.updatePktSecurityParameters(packetOut); err != nil { - x.Logger.Printf("ERROR updatePktSecurityParameters error: %s", err) - return nil, err - } - // retransmit with updated auth engine params - result, err = x.sendOneRequest(packetOut) - if err != nil { - x.Logger.Printf("ERROR out-of-time-window retransmit error: %s", err) - return result, ErrNotInTimeWindow - } - - case usmStatsUnknownEngineIDs: - x.Logger.Print("WARNING detected unknown engine id ERROR") - if err = x.updatePktSecurityParameters(packetOut); err != nil { - x.Logger.Printf("ERROR updatePktSecurityParameters error: %s", err) - return nil, err - } - // retransmit with updated engine id - result, err = x.sendOneRequest(packetOut) - if err != nil { - x.Logger.Printf("ERROR unknown engine id retransmit error: %s", err) - return result, ErrUnknownEngineID - } - } - } + if kind, ok := reportKinds[p.Variables[0].Name]; ok { + return kind, true } - return result, err + return reportKind{err: ErrUnknownReportPDU}, true } diff --git a/v3.go b/v3.go index b1b0d6d..b2af445 100644 --- a/v3.go +++ b/v3.go @@ -143,51 +143,44 @@ func (x *GoSNMP) initPacket(packetOut *SnmpPacket) error { return nil } -// http://tools.ietf.org/html/rfc2574#section-2.2.3 This code does not -// check if the last message received was more than 150 seconds ago The -// snmpds that this code was tested on emit an 'out of time window' -// error with the new time and this code will retransmit when that is -// received. +// negotiateInitialSecurityParameters prepares packetOut's security parameters +// before an SNMPv3 request: it discovers the agent's engine while the client +// does not know its engine ID, and derives the keys otherwise. Known bug: the +// key derivation error is ignored when the engine ID is known. func (x *GoSNMP) negotiateInitialSecurityParameters(packetOut *SnmpPacket) error { - if x.Version != Version3 || packetOut.Version != Version3 { - return fmt.Errorf("negotiateInitialSecurityParameters called with non Version3 connection or packet") + if discoveryPacket := packetOut.SecurityParameters.usm().discoveryRequired(); discoveryPacket != nil { + return x.discoverEngine(discoveryPacket, packetOut) } + _ = packetOut.SecurityParameters.InitSecurityKeys() + return nil +} - if x.SecurityModel != packetOut.SecurityModel { - return fmt.Errorf("connection security model does not match security model defined in packet") +// discoverEngine runs engine discovery (RFC 3414 section 4): it sends +// discoveryPacket and takes the agent's engine parameters from the answer into +// the client and packetOut. Known bug: a discovery Report with another +// security model fails the request instead of being discarded. +func (x *GoSNMP) discoverEngine(discoveryPacket, packetOut *SnmpPacket) error { + discoveryPacket.ContextName = x.ContextName + result, err := x.sendOneRequest(discoveryPacket) + if err != nil && !engineFoundDespiteUnknownUser(result, err) { + return err } - - if discoveryPacket := packetOut.SecurityParameters.usm().discoveryRequired(); discoveryPacket != nil { - discoveryPacket.ContextName = x.ContextName - result, err := x.sendOneRequest(discoveryPacket) - if err != nil { - // Some devices (e.g. Dell EMC switches) respond to discovery probes with - // usmStatsUnknownUserNames instead of usmStatsUnknownEngineIDs, yet still - // include valid engine parameters. Treat it as a valid discovery response. - if !errors.Is(err, ErrUnknownUsername) || result == nil { - return err - } - usp := usmOf(result.SecurityParameters) - if usp == nil || usp.AuthoritativeEngineID == "" { - return err - } - } - - err = x.storeSecurityParameters(result) - if err != nil { - return err - } - - err = x.updatePktSecurityParameters(packetOut) - if err != nil { - return err - } - } else { - // A key derivation error is ignored here. - _ = packetOut.SecurityParameters.InitSecurityKeys() + if err = x.storeSecurityParameters(result); err != nil { + return err } + return x.updatePktSecurityParameters(packetOut) +} - return nil +// engineFoundDespiteUnknownUser reports whether a discovery that failed with +// ErrUnknownUsername still found the engine: some devices (e.g. Dell EMC +// switches) answer discovery with usmStatsUnknownUserNames instead of +// usmStatsUnknownEngineIDs, with valid engine parameters. +func engineFoundDespiteUnknownUser(result *SnmpPacket, err error) bool { + if !errors.Is(err, ErrUnknownUsername) || result == nil { + return false + } + usp := usmOf(result.SecurityParameters) + return usp != nil && usp.AuthoritativeEngineID != "" } // save the connection security parameters after a request/response diff --git a/v3_usm.go b/v3_usm.go index eda893b..4e92c60 100644 --- a/v3_usm.go +++ b/v3_usm.go @@ -268,7 +268,9 @@ func (sp *UsmSecurityParameters) privacyKey() ([]byte, error) { } // setSecurityParameters adopts the engine ID, boots and time of in, deriving -// new keys when the engine ID changes. +// new keys when the engine ID changes. Known bug: the engine ID is adopted +// before the keys are derived, so a failed derivation leaves it adopted +// without keys, boots or time, and the next request skips the discovery. func (sp *UsmSecurityParameters) setSecurityParameters(in *UsmSecurityParameters) error { sp.mu.Lock() defer sp.mu.Unlock() From 8cda37f8115dc66b5bce34d4c1a8744dddbfc7f7 Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:11:04 +0300 Subject: [PATCH 3/7] test: benchmark Get through send for SNMPv2c and SNMPv3 BenchmarkSendOneRequest stops below send, so the negotiation, the store and the resync dispatch had no benchmark. BenchmarkGet runs an SNMPv2c Get on the in-memory transport; BenchmarkGetV3 an authNoPriv Get after discovery against the fake agent. --- engine_requests_test.go | 13 +++++++++++++ engine_v3_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/engine_requests_test.go b/engine_requests_test.go index 282c479..3673f45 100644 --- a/engine_requests_test.go +++ b/engine_requests_test.go @@ -720,3 +720,16 @@ func BenchmarkSendOneRequest(b *testing.B) { } } } + +// BenchmarkGet measures one SNMPv2c Get through send on the in-memory +// transport. +func BenchmarkGet(b *testing.B) { + c := newFakeTransport(nil, answer(sysDescr)) + x := newEngineClient(b, nil, Version2c, c) + b.ReportAllocs() + for b.Loop() { + if _, err := x.Get([]string{engineOID}); err != nil { + b.Fatal(err) + } + } +} diff --git a/engine_v3_test.go b/engine_v3_test.go index b08f331..ae83d3c 100644 --- a/engine_v3_test.go +++ b/engine_v3_test.go @@ -660,3 +660,33 @@ func describeKeys(sp *UsmSecurityParameters) string { } return "unknown" } + +// BenchmarkGetV3 measures one SNMPv3 authNoPriv Get through send, after +// discovery, against the fake agent; the agent's checks and answer are +// included. +func BenchmarkGetV3(b *testing.B) { + agent := newFakeV3Agent(nil, agentCreds, sysDescr) + c := newFakeTransport(nil, agent.handle) + creds := agentCreds["codec-md5"] + x := newEngineClientFrom(b, nil, fakePacketTransport{c}, &GoSNMP{ + Version: Version3, + MsgFlags: AuthNoPriv, + SecurityModel: UserSecurityModel, + SecurityParameters: &UsmSecurityParameters{ + UserName: "codec-md5", + AuthenticationProtocol: creds.auth, + AuthenticationPassphrase: creds.authPass, + }, + Timeout: time.Second, + Retries: 2, + }) + if _, err := x.Get([]string{engineOID}); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + for b.Loop() { + if _, err := x.Get([]string{engineOID}); err != nil { + b.Fatal(err) + } + } +} From 6d27cd709f3103e3ea17337aa9531771f77c0e0c Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:33:31 +0300 Subject: [PATCH 4/7] test: pin a discovery answer without an engine ID and a Logger that changes the client Review of send's restructuring found two paths no scenario observed; both pins pass on the code before it. - discovery/report-without-engine-id/{md5,sha-aes}: a discovery Report without an engine ID is accepted and the request goes out with an empty engine ID; at authPriv it fails in the encoder for want of a privacy key (known bug). - hooks/logger-changes-{security-model,version}: the client's Logger is user code that runs inside send before the negotiation; when it changes the client's security model or version, the negotiation's guards end the request with an empty packet. --- engine_v3_test.go | 54 ++++++++++++++++++++++++++++++++++ testdata/engine/v3.golden | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/engine_v3_test.go b/engine_v3_test.go index ae83d3c..c026a92 100644 --- a/engine_v3_test.go +++ b/engine_v3_test.go @@ -108,6 +108,35 @@ func asV2c(p *SnmpPacket) { p.Version, p.Community = Version2c, "public" } +// withoutEngineID takes the engine ID out of an answer and its context. +func withoutEngineID(p *SnmpPacket) { + p.SecurityParameters.usm().AuthoritativeEngineID = "" + p.ContextEngineID = "" +} + +// changeClientWhenLogging runs a Get with a Logger that applies change to the +// client the first time it is given msg. +func changeClientWhenLogging(msg string, change func(x *GoSNMP)) func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { + return func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { + done := false + x.Logger = NewLogger(funcLogger(func(s string) { + if s == msg && !done { + done = true + change(x) + a.tr.addf("Logger changes the client at %q", msg) + } + })) + return x.Get([]string{engineOID}) + } +} + +// funcLogger is a LoggerInterface that hands each line to a function. +type funcLogger func(string) + +func (l funcLogger) Print(v ...any) { l(fmt.Sprint(v...)) } + +func (l funcLogger) Printf(format string, v ...any) { l(fmt.Sprintf(format, v...)) } + // fromOtherEngine makes an answer come from the agent's other engine ID. func fromOtherEngine(p *SnmpPacket) { p.SecurityParameters.usm().AuthoritativeEngineID = agentOtherEngineID @@ -245,6 +274,31 @@ func v3Scenarios() map[string]v3Scenario { }, knownBug: "a notInTimeWindow Report answering the resynchronized retransmission is returned with a nil error", }, + // Discovery answered by a Report without an engine ID. + "discovery/report-without-engine-id/md5": { + user: "codec-md5", script: onRequest(1, agentAnswer{ + report: usmStatsUnknownEngineIDs, why: "discovery Report without an engine ID", edit: withoutEngineID, + }), + knownBug: "a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID", + }, + "discovery/report-without-engine-id/sha-aes": { + user: "codec-sha-aes", script: onRequest(1, agentAnswer{ + report: usmStatsUnknownEngineIDs, why: "discovery Report without an engine ID", edit: withoutEngineID, + }), + knownBug: "a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID: at authPriv it fails in the encoder for want of a privacy key", + }, + + // The client's Logger is user code that runs inside send, before the + // negotiation. + "hooks/logger-changes-security-model": { + user: "codec-md5", + run: changeClientWhenLogging("SEND INIT", func(x *GoSNMP) { x.SecurityModel = 2 }), + }, + "hooks/logger-changes-version": { + user: "codec-md5", + run: changeClientWhenLogging("SEND INIT", func(x *GoSNMP) { x.Version = Version2c }), + }, + // send's resynchronization after a notInTimeWindow Report. "resync/panic-in-retransmission": { user: "codec-md5", diff --git a/testdata/engine/v3.golden b/testdata/engine/v3.golden index 6c6c0a2..3345801 100644 --- a/testdata/engine/v3.golden +++ b/testdata/engine/v3.golden @@ -867,6 +867,55 @@ 0s result: packet 3 msgID=2002 authNoPriv GetResponse id=1002 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"], no error 0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +== v3/discovery/report-without-engine-id/md5 +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (discovery Report without an engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="" boots=7 time=1000 context=""/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="" boots=7 time=1000 context=""/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:2] at noAuthNoPriv (unknown engine ID) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 noAuthNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:2] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with GetResponse id=1003 msgID=2003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] at authNoPriv (request accepted) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetResponse id=1003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] +0s hook OnRecv +0s hook OnFinish +0s result: packet 3 msgID=2003 authNoPriv GetResponse id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"], no error +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID + +== v3/discovery/report-without-engine-id/sha-aes +0s client: user="codec-sha-aes" authPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (discovery Report without an engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="" boots=7 time=1000 context=""/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s result: packet nil, error "marshal: crypto/aes: invalid key size 0" *fmt.wrapError +0s client after: engine="" boots=7 time=1000 keys=none context engine="" +0s known bug: a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID: at authPriv it fails in the encoder for want of a privacy key + == v3/discovery/request-unanswered 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 0s deadline +1s @@ -1221,6 +1270,18 @@ 20s client after: engine="\x80\x00\x1f\x88\x04codec-other" boots=7 time=1020 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" 20s known bug: a GetResponse from another engine ID than the request's is accepted and its engine ID adopted (RFC 3412 section 7.2 step 12 b) +== v3/hooks/logger-changes-security-model +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s Logger changes the client at "SEND INIT" +0s result: packet empty, error "connection security model does not match security model defined in packet" *errors.errorString +0s client after: engine="" boots=0 time=0 keys=none context engine="" + +== v3/hooks/logger-changes-version +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s Logger changes the client at "SEND INIT" +0s result: packet empty, error "negotiateInitialSecurityParameters called with non Version3 connection or packet" *errors.errorString +0s client after: engine="" boots=0 time=0 keys=none context engine="" + == v3/hooks/privacy-protocol-changed-on-retry 0s client: user="codec-sha-aes" authPriv+reportable timeout=1s retries=1 0s deadline +1s From 0d85b23f66cfba63cd64fcdaa0f46a2545c17693 Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:33:31 +0300 Subject: [PATCH 5/7] refactor: keep the negotiation's guards and name the store path's known bugs Review of send's restructuring: - The two guards of negotiateInitialSecurityParameters come back: the client's Logger runs inside send between mkSnmpPacket and the negotiation, so a Logger that changes the client reaches them. - The retransmission failure's log line passes the same format and argument to the Logger as before (the text was the same, the call was not). - Known-bug comments: setSecurityParameters adopts boots and time from any accepted message and keeps the time without the moment it arrived (the 150 s notInTimeWindow round trip, moved here from resync); storeSecurityParameters takes the context engine ID only while it is empty; discovery accepts an answer without an engine ID; the discovery exception of testAuthentication skips the digest for any message with an empty user name and engine ID; resync returns a nil error for a resync Report only. - reportKind.resync and reportKindOf say what they decide more precisely. No behavior change: goldens and logger output (method, format, arguments) unchanged. --- engine.go | 21 +++++++++++---------- v3.go | 23 +++++++++++++++++++---- v3_usm.go | 10 ++++++++-- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/engine.go b/engine.go index e41d0fa..858d9ce 100644 --- a/engine.go +++ b/engine.go @@ -78,11 +78,9 @@ func (x *GoSNMP) send(packetOut *SnmpPacket) (result *SnmpPacket, err error) { // resync sends packetOut again after a Report of kind, with the engine ID or // time the client stored from it. Known bugs: a failed retransmission returns -// the Report's error instead of its own; a Report answering the +// the first Report's error instead of its own; a resync Report answering the // retransmission is returned with a nil error; the engine boots and time of -// the answer to the retransmission are not stored; the client sends the engine -// time of the last message it received, not the current one (RFC 3414 section -// 3.1 step 6 a), so a request after 150 s of silence goes through this resync. +// the answer to the retransmission are not stored. func (x *GoSNMP) resync(packetOut *SnmpPacket, kind reportKind) (*SnmpPacket, error) { if x.Logger.enabled() { x.Logger.Print("WARNING detected " + kind.name + " ERROR") @@ -93,7 +91,9 @@ func (x *GoSNMP) resync(packetOut *SnmpPacket, kind reportKind) (*SnmpPacket, er } result, err := x.sendOneRequest(packetOut) if err != nil { - x.Logger.Printf("ERROR %s retransmit error: %s", kind.name, err) + if x.Logger.enabled() { + x.Logger.Printf("ERROR "+kind.name+" retransmit error: %s", err) + } return result, kind.err } return result, nil @@ -365,10 +365,10 @@ func (e *exchange) answers(reply *SnmpPacket, earlierIDs []uint32) (bool, error) type reportKind struct { // err is the request's error. err error - // resync marks the Reports that carry the agent's engine ID or time: the - // request does not end with err, the caller takes the engine parameters - // they carry and send sends the request again, returning err only if that - // fails. + // resync marks the Reports that say the request's engine ID or time is + // out of date: they answer without err, the caller takes the engine + // parameters they carry, and send, when it is the caller, sends the request + // again and returns err only if that fails. resync bool // name names a resync Report in log lines. name string @@ -392,7 +392,8 @@ var reportKinds = map[string]reportKind{ //nolint:gochecknoglobals // a read-onl // The agent puts the counter of the error it detected in a Report's varbinds; // the Report's request ID is the request's, or 0 when the agent could not read // it. An unknown counter gives ErrUnknownReportPDU. Known bug: a Report counts -// only with exactly one varbind, so one with more is a successful reply. +// only with exactly one varbind, so one with more is treated as a reply and +// matched by its request ID. func reportKindOf(p *SnmpPacket) (reportKind, bool) { if p.Version != Version3 || p.PDUType != Report || len(p.Variables) != 1 { return reportKind{}, false diff --git a/v3.go b/v3.go index b2af445..0ac809b 100644 --- a/v3.go +++ b/v3.go @@ -109,8 +109,9 @@ func (x *GoSNMP) testAuthentication(packet []byte, result *SnmpPacket, useRespon } // Engine discovery (RFC 3414 section 4): a message with an empty user name - // and engine ID is accepted without authentication, whatever its flags and - // variable bindings (the bindings are not decoded yet). + // and engine ID is accepted without authentication. Known bug: whatever its + // flags and variable bindings (the bindings are not decoded yet), so any + // message with an empty user name and engine ID skips the digest. msgSecParams := result.SecurityParameters.usm() if msgSecParams.UserName == "" && msgSecParams.AuthoritativeEngineID == "" { return nil @@ -148,6 +149,15 @@ func (x *GoSNMP) initPacket(packetOut *SnmpPacket) error { // does not know its engine ID, and derives the keys otherwise. Known bug: the // key derivation error is ignored when the engine ID is known. func (x *GoSNMP) negotiateInitialSecurityParameters(packetOut *SnmpPacket) error { + // mkSnmpPacket copies the client's version and security model into the + // packet; they differ here only when the client's Logger changed the client + // during send. + if x.Version != Version3 || packetOut.Version != Version3 { + return fmt.Errorf("negotiateInitialSecurityParameters 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") + } if discoveryPacket := packetOut.SecurityParameters.usm().discoveryRequired(); discoveryPacket != nil { return x.discoverEngine(discoveryPacket, packetOut) } @@ -157,8 +167,10 @@ func (x *GoSNMP) negotiateInitialSecurityParameters(packetOut *SnmpPacket) error // discoverEngine runs engine discovery (RFC 3414 section 4): it sends // discoveryPacket and takes the agent's engine parameters from the answer into -// the client and packetOut. Known bug: a discovery Report with another -// security model fails the request instead of being discarded. +// the client and packetOut. Known bugs: a discovery Report with another +// security model fails the request instead of being discarded; an answer +// without an engine ID is accepted, so the request goes out with an empty +// 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) @@ -193,6 +205,9 @@ func (x *GoSNMP) storeSecurityParameters(result *SnmpPacket) error { return fmt.Errorf("connection security model does not match security model extracted from packet") } + // Known bug: the context engine ID is taken only while the client has + // none, so it stays the old engine's after the client adopts a new engine + // ID. if x.ContextEngineID == "" { x.ContextEngineID = result.SecurityParameters.usm().AuthoritativeEngineID } diff --git a/v3_usm.go b/v3_usm.go index 4e92c60..d98e38f 100644 --- a/v3_usm.go +++ b/v3_usm.go @@ -268,9 +268,15 @@ func (sp *UsmSecurityParameters) privacyKey() ([]byte, error) { } // setSecurityParameters adopts the engine ID, boots and time of in, deriving -// new keys when the engine ID changes. Known bug: the engine ID is adopted +// new keys when the engine ID changes. Known bugs: the engine ID is adopted // before the keys are derived, so a failed derivation leaves it adopted -// without keys, boots or time, and the next request skips the discovery. +// without derived keys, boots or time, and the next request skips the +// discovery; boots and time are adopted from any message the client accepts, +// older or outside the time window ones included (RFC 3414 section 3.2 step 7 +// b keeps the latest); the time is kept without the moment it was received, so +// a request sends the engine time of the last message, not the current one +// (RFC 3414 section 3.1 step 6 a), and one after 150 s of silence is first +// answered with notInTimeWindow. func (sp *UsmSecurityParameters) setSecurityParameters(in *UsmSecurityParameters) error { sp.mu.Lock() defer sp.mu.Unlock() From e86eabf7887be188cf3746b6845044391202e8f9 Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:59:36 +0300 Subject: [PATCH 6/7] test: pin the resync paths an equivalence review found unobserved All pass on the code before send's restructuring: - hooks/logger-panics-after-failed-retransmission: a Logger that panics on the retransmission's failure line ends the request with the Report that answered the retransmission, not the first one. - hooks/onfinish-changes-version: a hook that changes the client's version during the request makes the store fail, and the request returns its error with the reply. - resync/report-for-other-request-id: a notInTimeWindow Report for another request ID still resynchronizes the request (known bug). - resync/retransmission-answered-by-authenticated-report: the engine time of an authenticated Report answering the retransmission is not stored (known bug). - discovery/report-without-engine-id/sha256-des: with DES, a discovery answer without an engine ID makes the encoder panic (known bug). onLogging replaces the Logger helper of the previous pins and records the line it acts on. --- engine_v3_test.go | 71 +++++++++++++++++-- testdata/engine/v3.golden | 142 +++++++++++++++++++++++++++++++++++++- 2 files changed, 204 insertions(+), 9 deletions(-) diff --git a/engine_v3_test.go b/engine_v3_test.go index c026a92..1382da1 100644 --- a/engine_v3_test.go +++ b/engine_v3_test.go @@ -114,16 +114,16 @@ func withoutEngineID(p *SnmpPacket) { p.ContextEngineID = "" } -// changeClientWhenLogging runs a Get with a Logger that applies change to the -// client the first time it is given msg. -func changeClientWhenLogging(msg string, change func(x *GoSNMP)) func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { +// onLogging runs a Get with a Logger that calls do the first time it is +// given msg: user code that changes the client or panics inside the engine. +func onLogging(msg string, do func(x *GoSNMP)) func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { return func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { done := false x.Logger = NewLogger(funcLogger(func(s string) { if s == msg && !done { done = true - change(x) - a.tr.addf("Logger changes the client at %q", msg) + a.tr.addf("Logger is given %q", msg) + do(x) } })) return x.Get([]string{engineOID}) @@ -281,6 +281,12 @@ func v3Scenarios() map[string]v3Scenario { }), knownBug: "a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID", }, + "discovery/report-without-engine-id/sha256-des": { + user: "codec-sha256-des", script: onRequest(1, agentAnswer{ + report: usmStatsUnknownEngineIDs, why: "discovery Report without an engine ID", edit: withoutEngineID, + }), + knownBug: "a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID: with DES the encoder panics for want of a privacy key and send recovers it", + }, "discovery/report-without-engine-id/sha-aes": { user: "codec-sha-aes", script: onRequest(1, agentAnswer{ report: usmStatsUnknownEngineIDs, why: "discovery Report without an engine ID", edit: withoutEngineID, @@ -292,11 +298,40 @@ func v3Scenarios() map[string]v3Scenario { // negotiation. "hooks/logger-changes-security-model": { user: "codec-md5", - run: changeClientWhenLogging("SEND INIT", func(x *GoSNMP) { x.SecurityModel = 2 }), + run: onLogging("SEND INIT", func(x *GoSNMP) { x.SecurityModel = 2 }), }, "hooks/logger-changes-version": { user: "codec-md5", - run: changeClientWhenLogging("SEND INIT", func(x *GoSNMP) { x.Version = Version2c }), + run: onLogging("SEND INIT", func(x *GoSNMP) { x.Version = Version2c }), + }, + "hooks/logger-panics-after-failed-retransmission": { + user: "codec-noauth", script: func(n int, _ agentRequest) (agentAnswer, bool) { + switch n { + case 2: + return agentAnswer{report: usmStatsNotInTimeWindows, why: "not in time window"}, true + case 3: + return agentAnswer{report: usmStatsUnknownUserNames, why: "unknown user"}, true + } + return agentAnswer{}, false + }, + run: onLogging("ERROR out-of-time-window retransmit error: unknown username", func(*GoSNMP) { + panic("logger failure") + }), + knownBug: "send recovers a panic into an error, with the result the request had when it panicked", + }, + "hooks/onfinish-changes-version": { + user: "codec-md5", + run: func(x *GoSNMP, a *fakeV3Agent) (*SnmpPacket, error) { + onFinish, calls := x.OnFinish, 0 + x.OnFinish = func(x *GoSNMP) { + onFinish(x) + if calls++; calls == 2 { + x.Version = Version2c + a.tr.addf("hook OnFinish sets the client's version to 2c") + } + } + return x.Get([]string{engineOID}) + }, }, // send's resynchronization after a notInTimeWindow Report. @@ -343,6 +378,28 @@ func v3Scenarios() map[string]v3Scenario { }, knownBug: "the retransmission's error is replaced by ErrNotInTimeWindow", }, + "resync/report-for-other-request-id": { + user: "codec-md5", script: onRequest(2, agentAnswer{ + report: usmStatsNotInTimeWindows, level: AuthNoPriv, why: "not in time window, for another request ID", + edit: func(p *SnmpPacket) { p.RequestID += 100 }, + }), + knownBug: "a Report is mapped without checking its msgID or request ID", + }, + "resync/retransmission-answered-by-authenticated-report": { + user: "codec-md5", script: func(n int, _ agentRequest) (agentAnswer, bool) { + switch n { + case 2: + return agentAnswer{report: usmStatsNotInTimeWindows, level: AuthNoPriv, why: "not in time window"}, true + case 3: + return agentAnswer{ + report: snmpUnknownContexts, level: AuthNoPriv, why: "unknown context, a later engine time", + edit: func(p *SnmpPacket) { p.SecurityParameters.usm().AuthoritativeEngineTime += 7 }, + }, true + } + return agentAnswer{}, false + }, + knownBug: "the retransmission's error is replaced by ErrNotInTimeWindow, and the engine boots and time of its answer are not stored", + }, "resync/retransmission-answer-time": { user: "codec-md5", script: func(n int, _ agentRequest) (agentAnswer, bool) { switch n { diff --git a/testdata/engine/v3.golden b/testdata/engine/v3.golden index 3345801..48576a4 100644 --- a/testdata/engine/v3.golden +++ b/testdata/engine/v3.golden @@ -916,6 +916,22 @@ 0s client after: engine="" boots=7 time=1000 keys=none context engine="" 0s known bug: a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID: at authPriv it fails in the encoder for want of a privacy key +== v3/discovery/report-without-engine-id/sha256-des +0s client: user="codec-sha256-des" authPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (discovery Report without an engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="" boots=7 time=1000 context=""/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s result: packet nil, error "recover: runtime error: slice bounds out of range [8:0] Stack: ..." *errors.errorString +0s client after: engine="" boots=7 time=1000 keys=none context engine="" +0s known bug: a discovery answer without an engine ID is accepted, and the request goes out with an empty engine ID: with DES the encoder panics for want of a privacy key and send recovers it + == v3/discovery/request-unanswered 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 0s deadline +1s @@ -1272,16 +1288,73 @@ == v3/hooks/logger-changes-security-model 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 -0s Logger changes the client at "SEND INIT" +0s Logger is given "SEND INIT" 0s result: packet empty, error "connection security model does not match security model defined in packet" *errors.errorString 0s client after: engine="" boots=0 time=0 keys=none context engine="" == v3/hooks/logger-changes-version 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 -0s Logger changes the client at "SEND INIT" +0s Logger is given "SEND INIT" 0s result: packet empty, error "negotiateInitialSecurityParameters called with non Version3 connection or packet" *errors.errorString 0s client after: engine="" boots=0 time=0 keys=none context engine="" +== v3/hooks/logger-panics-after-failed-retransmission +0s client: user="codec-noauth" noAuthNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 noAuthNoPriv+reportable user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at noAuthNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 noAuthNoPriv user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 noAuthNoPriv+reportable user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with Report id=1003 msgID=2003 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1] at noAuthNoPriv (unknown user) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 noAuthNoPriv user="codec-noauth" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1003 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1] +0s hook OnRecv +0s Logger is given "ERROR out-of-time-window retransmit error: unknown username" +0s result: packet 3 msgID=2003 noAuthNoPriv Report id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-noauth"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.6.3.15.1.1.3.0=Counter32:1], error "recover: logger failure Stack: ..." *errors.errorString +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=none context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: send recovers a panic into an error, with the result the request had when it panicked + +== v3/hooks/onfinish-changes-version +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with GetResponse id=1002 msgID=2002 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] at authNoPriv (request accepted) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetResponse id=1002 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] +0s hook OnRecv +0s hook OnFinish +0s hook OnFinish sets the client's version to 2c +0s result: packet 3 msgID=2002 authNoPriv GetResponse id=1002 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"], error "storeParameters called with non Version3 connection or packet" *errors.errorString +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" + == v3/hooks/privacy-protocol-changed-on-retry 0s client: user="codec-sha-aes" authPriv+reportable timeout=1s retries=1 0s deadline +1s @@ -2423,6 +2496,39 @@ 0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" 0s known bug: send recovers a panic into an error, with the result the request had when it panicked +== v3/resync/report-for-other-request-id +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1102 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at authNoPriv (not in time window, for another request ID) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1102 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with GetResponse id=1003 msgID=2003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] at authNoPriv (request accepted) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetResponse id=1003 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"] +0s hook OnRecv +0s hook OnFinish +0s result: packet 3 msgID=2003 authNoPriv GetResponse id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1000 [.1.3.6.1.2.1.1.1.0=OctetString:"codec agent"], no error +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: a Report is mapped without checking its msgID or request ID + == v3/resync/retransmission-answer-time 0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 0s deadline +1s @@ -2456,6 +2562,38 @@ 0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" 0s known bug: the engine boots and time of the answer to the retransmission are not stored +== v3/resync/retransmission-answered-by-authenticated-report +0s client: user="codec-md5" authNoPriv+reportable timeout=1s retries=2 +0s deadline +1s +0s hook PreSend +0s write #1: 3 msgID=2001 noAuthNoPriv+reportable user="" engine="" boots=0 time=0 context=""/"" GetRequest id=1001 [] +0s agent: answers #1 with Report id=1001 msgID=2001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] at noAuthNoPriv (unknown engine ID) +0s agent answers #1 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2001 noAuthNoPriv user="" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1001 [.1.3.6.1.6.3.15.1.1.4.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #2: 3 msgID=2002 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1002 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #2 with Report id=1002 msgID=2002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] at authNoPriv (not in time window) +0s agent answers #2 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2002 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1002 [.1.3.6.1.6.3.15.1.1.2.0=Counter32:1] +0s hook OnRecv +0s hook OnFinish +0s deadline +1s +0s hook PreSend +0s write #3: 3 msgID=2003 authNoPriv+reportable user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 context="\x80\x00\x1f\x88\x04codec-agent"/"" GetRequest id=1003 [.1.3.6.1.2.1.1.1.0] +0s agent: answers #3 with Report id=1003 msgID=2003 [.1.3.6.1.6.3.12.1.5.0=Counter32:1] at authNoPriv (unknown context, a later engine time) +0s agent answers #3 after 0s +0s hook OnSent +0s read from 127.0.0.2:161: 3 msgID=2003 authNoPriv user="codec-md5" engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1007 context="\x80\x00\x1f\x88\x04codec-agent"/"" Report id=1003 [.1.3.6.1.6.3.12.1.5.0=Counter32:1] +0s hook OnRecv +0s result: packet 3 msgID=2003 authNoPriv Report id=1003 context="\x80\x00\x1f\x88\x04codec-agent"/"" usm="codec-md5"/"\x80\x00\x1f\x88\x04codec-agent"/7/1007 [.1.3.6.1.6.3.12.1.5.0=Counter32:1], error "not in time window" *errors.errorString (equals ErrNotInTimeWindow) +0s client after: engine="\x80\x00\x1f\x88\x04codec-agent" boots=7 time=1000 keys=current context engine="\x80\x00\x1f\x88\x04codec-agent" +0s known bug: the retransmission's error is replaced by ErrNotInTimeWindow, and the engine boots and time of its answer are not stored + == v3/resync/retransmission-answered-by-report 0s client: user="codec-noauth" noAuthNoPriv+reportable timeout=1s retries=2 0s deadline +1s From 61b956cf73277b035f8c871028f50b953bf3df1f Mon Sep 17 00:00:00 2001 From: ilyam8 Date: Fri, 9 Oct 2026 23:59:36 +0300 Subject: [PATCH 7/7] refactor: retransmit inside send, where its named results are assigned send's recover returns the result held by its named results when a panic, user code's included, ends the request. With the retransmission in a helper, a Logger panicking on the retransmission's failure line returned the first Report instead of the retransmission's answer. The resynchronization is one paragraph of send again, with its known-bug comments; resync is gone. No behavior change: goldens and logger output unchanged. --- engine.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/engine.go b/engine.go index 858d9ce..9dde8be 100644 --- a/engine.go +++ b/engine.go @@ -17,6 +17,8 @@ import ( // send runs a request: for SNMPv3 it first discovers the agent's engine or // derives the keys, then sends the request, and sends it once more when the // answer is a Report that resynchronizes the client with the agent's engine. +// Every result it returns passes through its named results, which the +// recover below returns when a panic, user code's included, ends the request. func (x *GoSNMP) send(packetOut *SnmpPacket) (result *SnmpPacket, err error) { // Known bug: a panic becomes the request's error, with the stacks of all // goroutines in its text (8 KB, NUL-padded), and the result the request had @@ -66,30 +68,29 @@ func (x *GoSNMP) send(packetOut *SnmpPacket) (result *SnmpPacket, err error) { x.Logger.Printf("SEND STORE SECURITY PARAMS from result: %s", result.SecurityParameters.SafeString()) } err = x.storeSecurityParameters(result) - if kind, ok := reportKindOf(result); ok && kind.resync { - // Known bug: the store error is dropped, so a Report with another - // security model is acted on instead of being discarded. - return x.resync(packetOut, kind) - } - // Known bug: a reply with another security model is returned together - // with the store error. - return result, err -} + kind, ok := reportKindOf(result) + if !ok || !kind.resync { + // Known bug: a reply with another security model is returned together + // with the store error. + return result, err + } -// resync sends packetOut again after a Report of kind, with the engine ID or -// time the client stored from it. Known bugs: a failed retransmission returns -// the first Report's error instead of its own; a resync Report answering the -// retransmission is returned with a nil error; the engine boots and time of -// the answer to the retransmission are not stored. -func (x *GoSNMP) resync(packetOut *SnmpPacket, kind reportKind) (*SnmpPacket, error) { + // Resynchronize: send the request again with the engine ID or time the + // client stored from the Report. The retransmission's result goes into the + // named result, which a recovered panic returns. Known bugs: the store + // error is dropped, so a Report with another security model is acted on + // instead of being discarded; a failed retransmission returns the first + // Report's error instead of its own; a resync Report answering the + // retransmission is returned with a nil error; the engine boots and time of + // the answer to the retransmission are not stored. if x.Logger.enabled() { x.Logger.Print("WARNING detected " + kind.name + " ERROR") } - if err := x.updatePktSecurityParameters(packetOut); err != nil { + if err = x.updatePktSecurityParameters(packetOut); err != nil { x.Logger.Printf("ERROR updatePktSecurityParameters error: %s", err) return nil, err } - result, err := x.sendOneRequest(packetOut) + result, err = x.sendOneRequest(packetOut) if err != nil { if x.Logger.enabled() { x.Logger.Printf("ERROR "+kind.name+" retransmit error: %s", err)