Revert "[snmp] SNMP v3 fixes and improvements (openhab#16803)"

This reverts commit 51da65b.

Author: psmedley

bundles/org.openhab.binding.snmp/README.md

@@ -77,8 +77,13 @@ The default is `v1`.
 
 ### `target3`
 
-The `target3` thing has an additional mandatory parameter: `user`.
-This value of this parameter is named "securityName" or "userName" in most agents.
+The `target3` thing has additional mandatory parameters: `engineId` and `user`.
+
+The `engineId` must be given in hexadecimal notation (case-insensitive) without separators (e.g. `80000009035c710dbcd9e6`).
+The allowed length is 11 to 32 bytes (22 to 64 hex characters).
+If you encounter problems, please check if your agent prefixes the set engine id (e.g. Mikrotik uses `80003a8c04` and appends the set value to that).
+
+The `user` parameter is named "securityName" or "userName" in most agents.
 
 Optional configuration parameters are: `securityModel`, `authProtocol`, `authPassphrase`, `privProtocol` and `privPassphrase`.
 
@@ -94,7 +99,7 @@ If authentication encryption is required, at least `authPassphrase` needs to be
 Other possible values for `authProtocol` are `SHA`, `HMAC128SHA224`, `HMAC192SHA256`, `HMAC256SHA384` and `HMAC384SHA512`.
 
 If encryption of transmitted data (privacy encryption) is required, at least `privPassphrase` needs to be set, while `privProtocol` defaults to `DES`.
-Other possible values for `privProtocol` are `DES3`, `AES128`, `AES192` and `AES256`.
+Other possible values for `privProtocol` are `AES128`, `AES192` and `AES256`.
 
 ## Channels
 

bundles/org.openhab.binding.snmp/pom.xml

@@ -18,13 +18,7 @@
     <dependency>
       <groupId>org.snmp4j</groupId>
       <artifactId>snmp4j</artifactId>
-      <version>3.8.2</version>
-      <scope>compile</scope>
-    </dependency>
-    <dependency>
-      <groupId>org.snmp4j</groupId>
-      <artifactId>snmp4j-unix-transport</artifactId>
-      <version>1.1.0</version>
+      <version>2.8.6</version>
       <scope>compile</scope>
     </dependency>
   </dependencies>

bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpService.java

@@ -16,13 +16,12 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.snmp.internal.types.SnmpAuthProtocol;
+import org.openhab.binding.snmp.internal.types.SnmpPrivProtocol;
 import org.snmp4j.CommandResponder;
 import org.snmp4j.PDU;
 import org.snmp4j.Target;
 import org.snmp4j.event.ResponseListener;
-import org.snmp4j.security.UsmUser;
-import org.snmp4j.smi.Address;
-import org.snmp4j.smi.OctetString;
 
 /**
  * The {@link SnmpService} is responsible for SNMP communication
@@ -34,53 +33,12 @@
 @NonNullByDefault
 public interface SnmpService {
 
-    /**
-     * Add a listener for received PDUs to the service
-     *
-     * @param listener the listener
-     */
     void addCommandResponder(CommandResponder listener);
 
-    /**
-     * Remove a listener for received PDUs from the service
-     *
-     * @param listener the listener
-     */
     void removeCommandResponder(CommandResponder listener);
 
-    /**
-     * Send a PDU to the given target
-     *
-     * @param pdu the PDU
-     * @param target the target
-     * @param userHandle an optional user-handle to identify the request
-     * @param listener the listener for the response (always called, even in case of timeout)
-     * @throws IOException when an error occurs
-     */
-    void send(PDU pdu, Target<?> target, @Nullable Object userHandle, ResponseListener listener) throws IOException;
+    void send(PDU pdu, Target target, @Nullable Object userHandle, ResponseListener listener) throws IOException;
 
-    /**
-     * Add a user to the service for a given engine id (v3 only)
-     *
-     * @param user the {@link UsmUser} that should be added
-     * @param engineId the engine id
-     */
-    void addUser(UsmUser user, OctetString engineId);
-
-    /**
-     * Remove a user from the service and clear the context engine id for this address (v3 only)
-     *
-     * @param address the remote address
-     * @param user the user
-     * @param engineId the engine id
-     */
-    void removeUser(Address address, UsmUser user, OctetString engineId);
-
-    /**
-     * Get the engine id of a remote system for a given address (v3 only)
-     *
-     * @param address the address of the remote system
-     * @return the engine id or {@code null} when engine id could not be determined
-     */
-    byte @Nullable [] getEngineId(Address address);
+    void addUser(String userName, SnmpAuthProtocol snmpAuthProtocol, @Nullable String authPassphrase,
+            SnmpPrivProtocol snmpPrivProtocol, @Nullable String privPassphrase, byte[] engineId);
 }

bundles/org.openhab.binding.snmp/src/main/java/org/openhab/binding/snmp/internal/SnmpServiceImpl.java

@@ -22,6 +22,8 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.snmp.internal.config.SnmpServiceConfiguration;
+import org.openhab.binding.snmp.internal.types.SnmpAuthProtocol;
+import org.openhab.binding.snmp.internal.types.SnmpPrivProtocol;
 import org.openhab.core.config.core.Configuration;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
@@ -35,22 +37,11 @@
 import org.snmp4j.Target;
 import org.snmp4j.event.ResponseListener;
 import org.snmp4j.mp.MPv3;
-import org.snmp4j.security.AuthHMAC128SHA224;
-import org.snmp4j.security.AuthHMAC192SHA256;
-import org.snmp4j.security.AuthHMAC256SHA384;
-import org.snmp4j.security.AuthHMAC384SHA512;
-import org.snmp4j.security.AuthMD5;
-import org.snmp4j.security.AuthSHA;
 import org.snmp4j.security.Priv3DES;
-import org.snmp4j.security.PrivAES128;
-import org.snmp4j.security.PrivAES192;
-import org.snmp4j.security.PrivAES256;
-import org.snmp4j.security.PrivDES;
 import org.snmp4j.security.SecurityModels;
 import org.snmp4j.security.SecurityProtocols;
 import org.snmp4j.security.USM;
 import org.snmp4j.security.UsmUser;
-import org.snmp4j.smi.Address;
 import org.snmp4j.smi.OctetString;
 import org.snmp4j.smi.UdpAddress;
 import org.snmp4j.transport.DefaultUdpTransportMapping;
@@ -67,6 +58,7 @@
 public class SnmpServiceImpl implements SnmpService {
     private final Logger logger = LoggerFactory.getLogger(SnmpServiceImpl.class);
 
+    private @NonNullByDefault({}) SnmpServiceConfiguration config;
     private @Nullable Snmp snmp;
     private @Nullable DefaultUdpTransportMapping transport;
 
@@ -75,7 +67,9 @@ public class SnmpServiceImpl implements SnmpService {
 
     @Activate
     public SnmpServiceImpl(Map<String, Object> config) {
-        addProtocols();
+        SecurityProtocols.getInstance().addDefaultProtocols();
+        SecurityProtocols.getInstance().addPrivacyProtocol(new Priv3DES());
+
         OctetString localEngineId = new OctetString(MPv3.createLocalEngineID());
         USM usm = new USM(SecurityProtocols.getInstance(), localEngineId, 0);
         SecurityModels.getInstance().addSecurityModel(usm);
@@ -85,33 +79,34 @@ public SnmpServiceImpl(Map<String, Object> config) {
 
     @Modified
     protected void modified(Map<String, Object> config) {
-        SnmpServiceConfiguration snmpCfg = new Configuration(config).as(SnmpServiceConfiguration.class);
+        this.config = new Configuration(config).as(SnmpServiceConfiguration.class);
         try {
             shutdownSnmp();
 
             final DefaultUdpTransportMapping transport;
 
-            if (snmpCfg.port > 0) {
-                transport = new DefaultUdpTransportMapping(new UdpAddress(snmpCfg.port), true);
+            if (this.config.port > 0) {
+                transport = new DefaultUdpTransportMapping(new UdpAddress(this.config.port), true);
             } else {
                 transport = new DefaultUdpTransportMapping();
             }
 
-            addProtocols();
+            SecurityProtocols.getInstance().addDefaultProtocols();
+            SecurityProtocols.getInstance().addPrivacyProtocol(new Priv3DES());
 
             final Snmp snmp = new Snmp(transport);
             listeners.forEach(snmp::addCommandResponder);
             snmp.listen();
 
             // re-add user entries
-            userEntries.forEach(u -> snmp.getUSM().addUser(u.user, u.engineId));
+            userEntries.forEach(u -> addUser(snmp, u));
 
             this.snmp = snmp;
             this.transport = transport;
 
             logger.debug("initialized SNMP at {}", transport.getAddress());
         } catch (IOException e) {
-            logger.warn("could not open SNMP instance on port {}: {}", snmpCfg.port, e.getMessage());
+            logger.warn("could not open SNMP instance on port {}: {}", this.config.port, e.getMessage());
         }
     }
 
@@ -125,21 +120,6 @@ public void deactivate() {
         }
     }
 
-    private void addProtocols() {
-        SecurityProtocols secProtocols = SecurityProtocols.getInstance();
-        secProtocols.addAuthenticationProtocol(new AuthMD5());
-        secProtocols.addAuthenticationProtocol(new AuthSHA());
-        secProtocols.addAuthenticationProtocol(new AuthHMAC128SHA224());
-        secProtocols.addAuthenticationProtocol(new AuthHMAC192SHA256());
-        secProtocols.addAuthenticationProtocol(new AuthHMAC256SHA384());
-        secProtocols.addAuthenticationProtocol(new AuthHMAC384SHA512());
-        secProtocols.addPrivacyProtocol(new PrivDES());
-        secProtocols.addPrivacyProtocol(new Priv3DES());
-        secProtocols.addPrivacyProtocol(new PrivAES128());
-        secProtocols.addPrivacyProtocol(new PrivAES192());
-        secProtocols.addPrivacyProtocol(new PrivAES256());
-    }
-
     private void shutdownSnmp() throws IOException {
         DefaultUdpTransportMapping transport = this.transport;
         if (transport != null) {
@@ -172,7 +152,7 @@ public void removeCommandResponder(CommandResponder listener) {
     }
 
     @Override
-    public void send(PDU pdu, Target<?> target, @Nullable Object userHandle, ResponseListener listener)
+    public void send(PDU pdu, Target target, @Nullable Object userHandle, ResponseListener listener)
             throws IOException {
         Snmp snmp = this.snmp;
         if (snmp != null) {
@@ -184,40 +164,35 @@ public void send(PDU pdu, Target<?> target, @Nullable Object userHandle, Respons
     }
 
     @Override
-    public void addUser(UsmUser user, OctetString engineId) {
-        UserEntry userEntry = new UserEntry(user, engineId);
+    public void addUser(String userName, SnmpAuthProtocol snmpAuthProtocol, @Nullable String authPassphrase,
+            SnmpPrivProtocol snmpPrivProtocol, @Nullable String privPassphrase, byte[] engineId) {
+        UsmUser usmUser = new UsmUser(new OctetString(userName),
+                authPassphrase != null ? snmpAuthProtocol.getOid() : null,
+                authPassphrase != null ? new OctetString(authPassphrase) : null,
+                privPassphrase != null ? snmpPrivProtocol.getOid() : null,
+                privPassphrase != null ? new OctetString(privPassphrase) : null);
+        OctetString securityNameOctets = new OctetString(userName);
+
+        UserEntry userEntry = new UserEntry(securityNameOctets, new OctetString(engineId), usmUser);
         userEntries.add(userEntry);
 
         Snmp snmp = this.snmp;
         if (snmp != null) {
-            snmp.getUSM().addUser(user, engineId);
+            addUser(snmp, userEntry);
         }
     }
 
-    @Override
-    public void removeUser(Address address, UsmUser user, OctetString engineId) {
-        Snmp snmp = this.snmp;
-        if (snmp != null) {
-            snmp.getUSM().removeAllUsers(user.getSecurityName(), engineId);
-            snmp.removeCachedContextEngineId(address);
-        }
-        userEntries.removeIf(e -> e.engineId.equals(engineId) && e.user.equals(user));
-    }
-
-    @Override
-    public byte @Nullable [] getEngineId(Address address) {
-        Snmp snmp = this.snmp;
-        if (snmp != null) {
-            return snmp.discoverAuthoritativeEngineID(address, 15000);
-        }
-        return null;
+    private static void addUser(Snmp snmp, UserEntry userEntry) {
+        snmp.getUSM().addUser(userEntry.securityName, userEntry.engineId, userEntry.user);
     }
 
     private static class UserEntry {
+        public OctetString securityName;
         public OctetString engineId;
         public UsmUser user;
 
-        public UserEntry(UsmUser user, OctetString engineId) {
+        public UserEntry(OctetString securityName, OctetString engineId, UsmUser user) {
+            this.securityName = securityName;
             this.engineId = engineId;
             this.user = user;
         }