snmpTargetParamsTable securityName matching community string

holger · January 19, 2022, 6:54am

Hi,

I am trying to implement a proxy forwarder able to forward SNMPv2c get requests.

My snmpCommunityTable has an entry as follows:
snmpCommunityName: public_p1
snmpCommunitySecurityName: proxy1

In the snmpTargetParamsTable (reference by snmpProxyTable) I have to add an entry with:
snmpTargetParamsSecurityName: public_p1

I was expecting that snmpTargetParamsSecurityName would have to match snmpCommunitySecurityName, but then it won’t forward any proxy messages. Instead I have to use the community string for the proxy to work. Is this a bug?

This was tested using the following versions:
agent++ v4.5.2
snmp++ v3.4.8

Thanks in advance,

Holger

AGENTPP · January 20, 2022, 9:23am

Hi Holger,
I have difficulties to understand your question.
Maybe the following helps: the snmpCommunityTable simply maps a community to a security name. The security name in that table “does not map” anywhere.
That security name is used in any lookups in other SNMPv3 based MIB tables after the mapping. That is all.
May be this helps anyway.

Best regards
Frank

holger · January 20, 2022, 2:15pm

Hi,

Thanks for reply. Let me try to explain with some example tables:

SNMP-COMMUNITY-MIB::snmpCommunityTable
index   Name        SecurityName ContextEngineID ContextName TransportTag
cr      public      comRead      <local_eid>     ""          ""
cw      private     comWrite     <local_eid>     ""          ""
p1      puplic_p1   proxy1       <proxy_eid>     ""          ""

SNMP-PROXY-MIB::snmpProxyTable
index    Type ContextEngineID ContextName TargetParamsIn SingleTargetOut MultipleTargetOut
p1v2rd   read <proxy_eid>     ""          v2inP1         p1Access

SNMP-TARGET-MIB::snmpTargetAddrTable
index     TDomain         TAddress         Timeout RetryCount TagList Params
p1Access  snmpUDPDomain   127.0.0.1:10165  1500    3          ""      proxyAccess

SNMP-COMMUNITY-MIB::snmpTargetAddrExtTable
index     TMask                MMS
p1Access  "FF FF FF FF FF FF"  1500

SNMP-TARGET-MIB::snmpTargetParamsTable
index         MPModel SecurityModel SecurityName SecurityLevel
proxyAccess   1       2             private      noAuthNoPriv
v2inP1        1       2             puplic_p1    noAuthNoPriv

With above tables doing a get to the proxy_forwarder:
snmpget -v 2c -c puplic_p1 127.0.0.1 SNMPv2-MIB::sysDescr.0

The community “public_p1” will select the row indexed “p1” in the snmpCommunityTable, which requires proxy forwarding due to the non-local EngineID.
The EngineID matches the row indexed “p1v2rd” in the snmpProxyTable, which references the “v2inP1” row of the snmpTargetParamsTable.

The forwarding of the get message only happens if snmpTargetParamsSecurityName is set to “public_p1” (i.e. the community string). If I set snmpTargetParamsSecurityName to “proxy1” (i.e. the security name) it won’t work, which I think is a bug.

From RFC3413 Section 7.1:
The snmpProxyTargetParamsIn object identifies an entry in the
snmpTargetParamsTable. The messageProcessingModel, security model,
securityName, and securityLevel must match the values of
snmpTargetParamsMPModel, snmpTargetParamsSecurityModel,
snmpTargetParamsSecurityName, and snmpTargetParamsSecurityLevel of
the identified entry in the snmpTargetParamsTable.

Thanks,

Holger

AGENTPP · January 24, 2022, 3:26pm

Hi Holger,
Sorry for the late reply. i somehow missed your answer. I agree that it looks like a bug. Nevertheless, let me check the code first…
Best regards
Frank

AGENTPP · January 24, 2022, 11:14pm

Hi Holger,

This is indeed a bug. Line 196 of the proxy_forwarder.cpp file in method

bool ProxyForwarder::match_target_params(Request* req,
 				    const OctetStr& paramsIn)
{
...
req->get_address()->get_security_name(sname);

must be changed to in order to use the security name mapped from the community by the coexistence info of the community MIB:

req->get_security_name(sname);

Unfortunately this will break existing proxy forwarder configurations. I have to think about how to support migration to the right configuration best.
The fixed release will be available by end of this week (v4.6.0).

Best regards,
Frank

holger · January 25, 2022, 11:51am

Hi Frank,

Thanks for the reply. I have tried your patch but it didn’t seem to make any difference. I still had to use the community string in snmpTargetParamsSecurityName.

Below is a patch which fixes the issue for me, but I am not familiar enough with the code to be sure that this is the correct/best place for a fix.

diff --git a/src/request.cpp b/src/request.cpp
index 54f175c..64211d9 100644
--- a/src/request.cpp
+++ b/src/request.cpp
@@ -1445,10 +1445,11 @@ Request* RequestList::receive(int sec)
 			LOG("RequestList: proxy request detected (contextEngineID)(rid): ");
 			LOG(pdu.get_context_engine_id().get_printable());
 			LOG(pdu.get_request_id());
 			LOG_END;
 
+			target.set_security_name(security_name);
 			Request* req = new Request(pdu, target);
 			return add_request(req);
 		}
 #endif	// _PROXY_FORWARDER	    
 		int		  viewType;

Thank you,

Holger

AGENTPP · January 25, 2022, 8:49pm

Hi Holger,

I anticipated this additional bug yesterday, but had no time to test test/verify it yet. Indeed, the security_name member needs to be assigned with the coexistence security name to get it working as it was designed but apparently never finished correctly in the implementation.
With both changes it should work now. I need more time to test it and check if there are side effects, but from the code analysis alone it should be fine now.

Best regards,
Frank

holger · January 29, 2022, 1:04pm

Hi Frank,

I had another look at this and noticed that my previous patch would also call set_security_name() for SNMPv3, which isn’t necessary as it was only a problem for v1/v2c. So here is what I think is a better patch which will only call set_security_name() for v1/v2c:

diff --git a/src/request.cpp b/src/request.cpp
index 54f175c..5d62bef 100644
--- a/src/request.cpp
+++ b/src/request.cpp
@@ -1375,10 +1375,11 @@ Request* RequestList::receive(int sec)
                         Counter32MibLeaf::incrementScalar(mib, 
                                 oidSnmpInBadCommunityNames);			
 			return 0;
 		    }
 
+		    target.set_security_name(security_name);
 		    pdu.set_context_engine_id(context_engine_id);
 		    pdu.set_context_name(context_name);
 
 		    if (version == version2c)
 		      security_model = SNMP_SECURITY_MODEL_V2;

With regard to your change on line 196 in proxy_forwarder.cpp both solutions return the same value:

req->get_security_name(sname);

Does this:

void Request::get_security_name(OctetStr &s)
{
  target.get_security_name(s);
}

And the following line also returns target.target.get_security_name(s):

req->get_address()->get_security_name(sname);

NS_SNMP UTarget* get_address() { return &target; }

Just mentioning it because that means the change in proxy_forwarder.cpp doesn’t cause any backwards compatibility issues (the change in request.cpp does though).

Best Regards,

Holger

AGENTPP · February 3, 2022, 7:19am

Hi Holger,

I am still working on it. You are right, the scope of the fix could be narrowed as indicated above, but if everything else would have been correctly implemented both changes should be fine.
I hope to be able to release a new version in the next few days.

Best regards,
Frank