[SNMP4J] Counting Invalid Traps

[Elise Atkins]

I have used SNMP4J for several years and have been very satisfied. 
Recently I did some testing to see what happens when invalid V1 traps 
and snmp requests were received by the snmp4j code. A CounterListner was 
added to the MessageDispatcherImpl and the number of incoming 
traps/requests were accurately counted but the number of invalid traps 
was not. The large majority of traps were silently discarded. After 
looking at the following code snippet from MessageDispatcherImpl and the 
MPv1.java code I concluded that when the MPv1 encountered an error it 
threw an exception that was caught by the catch block near the end of 
the snippet. The packet is then silently discarded unless the 
Snmp4JSettings.forwardRuntimeExceptions is true and the 
TransportMapping  class catches and counts the exception.

I think that a better solution would be to add the following lines the 
catch block:
        CounterEvent event = new CounterEvent(this,
                                              
SnmpConstants.snmpInvalidMsgs);
        fireIncrementCounter(event);

The packet would be then counted as invalid. I did set the 
forwardRuntimeExceptions to true and was able to get a count of the 
invalid packets and all seemed ok until a little later. I also use 
AgentXSubagent to connect to the snmpd service. When that service was 
not running or misconfigured and the subagent could not connect, a 
socket was orphaned and eventually things stopped working because there 
were too many sockets opened. I fixed that by calling close and 
disconnect on the subagent when the connect method failed. But I am now 
worried that having the forwardRuntimeExceptions set to true will cause 
additional problems down the line.

  public void processMessage(TransportMapping sourceTransport,
                             Address incomingAddress,
                             BERInputStream wholeMessage) {
    fireIncrementCounter(new CounterEvent(this, SnmpConstants.snmpInPkts));
    if (!wholeMessage.markSupported()) {
      String txt = "Message stream must support marks";
      logger.error(txt);
      throw new IllegalArgumentException(txt);
    }
    try {
      wholeMessage.mark(16);
      BER.MutableByte type = new BER.MutableByte();
      // decode header but do not check length here, because we do only 
decode
      // the first 16 bytes.
      BER.decodeHeader(wholeMessage, type, false);
      if (type.getValue() != BER.SEQUENCE) {
        logger.error("ASN.1 parse error (message is not a sequence)");
        CounterEvent event = new CounterEvent(this,
                                              
SnmpConstants.snmpInASNParseErrs);
        fireIncrementCounter(event);
      }
      Integer32 version = new Integer32();
      version.decodeBER(wholeMessage);
      MessageProcessingModel mp = 
getMessageProcessingModel(version.getValue());
      if (mp == null) {
        logger.warn("SNMP version "+version+" is not supported");
        CounterEvent event = new CounterEvent(this,
                                              
SnmpConstants.snmpInBadVersions);
        fireIncrementCounter(event);
      }
      else {
        // reset it
        wholeMessage.reset();
        // dispatch it
        dispatchMessage(sourceTransport, mp, incomingAddress, wholeMessage);
      }
    }
    catch (Exception ex) {
      logger.error(ex);
      if (logger.isDebugEnabled()) {
        ex.printStackTrace();
      }
      if (SNMP4JSettings.isFowardRuntimeExceptions()) {
        throw new RuntimeException(ex);
      }
    }
    catch (OutOfMemoryError oex) {
      logger.error(oex);
      if (SNMP4JSettings.isFowardRuntimeExceptions()) {
        throw oex;
      }
    }
  }

Sincerely,
Elise Atkins

-- 
ZoneRanger "Management Through Firewalls" <http://www.tavve.com/?leadlander>