Fix PrivParams for SNMPv3 USM with AES privacy

* In `snmp_usm:do_decrypt/3`, pass full UsmSecParams to
  `snmp_usm:try_decrypt/5` as expected by AES clause.

* Change `snmpm_usm:aes_encrypt/3` to use EngineBoots and
  EngineTime as cached by `snmpm_config:get_usm_eboots/1`
  and `snmpm_config:get_usm_etime/1` instead of
  `snmpm_config:get_engine_boots/0` and
  `snmpm_config:get_engine_time/0`. This ensures correct
  msgPrivacyParameters are sent when AES is used.

* Add test `snmp.snmp_manager_SUITE.usm_priv_aes/1` to
  avoid regression.

* Fix counting bug in `snmp_usm` macro `?BLOCK_CIPHER_AES`,
  introduced by commit 8d942cd.

lib/snmp/src/manager/snmpm_usm.erl

@@ -268,15 +268,16 @@ decrypt(Data, UsmUser, UsmSecParams, SecLevel) ->
 do_decrypt(Data, #usm_user{sec_name = SecName,
 			   priv     = PrivP,
 			   priv_key = PrivKey}, 
-	   #usmSecurityParameters{msgPrivacyParameters = PrivParms}) ->
+	   UsmSecParams) ->
     EncryptedPDU = snmp_pdus:dec_scoped_pdu_data(Data),
-    try_decrypt(PrivP, PrivKey, PrivParms, EncryptedPDU, SecName).
+    try_decrypt(PrivP, PrivKey, UsmSecParams, EncryptedPDU, SecName).
 
 try_decrypt(usmNoPrivProtocol, _, _, _, SecName) -> % 3.2.5
     error(usmStatsUnsupportedSecLevels, 
 	  ?usmStatsUnsupportedSecLevels_instance, SecName);
 try_decrypt(usmDESPrivProtocol, 
-	    PrivKey, MsgPrivParams, EncryptedPDU, SecName) ->
+	    PrivKey, UsmSecParams, EncryptedPDU, SecName) ->
+    #usmSecurityParameters{msgPrivacyParameters = MsgPrivParams} = UsmSecParams,
     case (catch des_decrypt(PrivKey, MsgPrivParams, EncryptedPDU)) of
 	{ok, DecryptedData} ->
 	    DecryptedData;
@@ -328,11 +329,6 @@ generate_outgoing_msg(Message, SecEngineID, SecName, SecData, SecLevel) ->
 	    _ -> % 3.1.1a
 		SecData
 	end,
-    %% 3.1.4
-    ?vtrace("generate_outgoing_msg -> (3.1.4)",[]),
-    ScopedPduBytes = Message#message.data,
-    {ScopedPduData, MsgPrivParams} =
-	encrypt(ScopedPduBytes, PrivProtocol, PrivKey, SecLevel),
     SnmpEngineID = get_engine_id(),
     ?vtrace("SnmpEngineID: ~p (3.1.6)",[SnmpEngineID]),
     %% 3.1.6
@@ -345,6 +341,11 @@ generate_outgoing_msg(Message, SecEngineID, SecName, SecData, SecLevel) ->
 	    _ ->
 		{get_engine_boots(), get_engine_time()}
 	end,
+    %% 3.1.4
+    ?vtrace("generate_outgoing_msg -> (3.1.4)",[]),
+    ScopedPduBytes = Message#message.data,
+    {ScopedPduData, MsgPrivParams} = encrypt(ScopedPduBytes, PrivProtocol, PrivKey, SecLevel, MsgAuthEngineBoots,
+        MsgAuthEngineTime),
     %% 3.1.5 - 3.1.7
     ?vtrace("generate_outgoing_msg -> (3.1.5 - 3.1.7)",[]),
     UsmSecParams =
@@ -361,12 +362,12 @@ generate_outgoing_msg(Message, SecEngineID, SecName, SecData, SecLevel) ->
 
 
 %% Ret: {ScopedPDU, MsgPrivParams} - both are already encoded as OCTET STRINGs
-encrypt(Data, PrivProtocol, PrivKey, SecLevel) ->
+encrypt(Data, PrivProtocol, PrivKey, SecLevel, EngineBoots, EngineTime) ->
     case snmp_misc:is_priv(SecLevel) of
 	false -> % 3.1.4b
 	    {Data, []};
 	true -> % 3.1.4a
-	    case (catch try_encrypt(PrivProtocol, PrivKey, Data)) of
+	    case (catch try_encrypt(PrivProtocol, PrivKey, Data, EngineBoots, EngineTime)) of
 		{ok, ScopedPduData, MsgPrivParams} ->
 		    {snmp_pdus:enc_oct_str_tag(ScopedPduData), MsgPrivParams};
 		{error, Reason} ->
@@ -376,12 +377,12 @@ encrypt(Data, PrivProtocol, PrivKey, SecLevel) ->
 	    end
     end.
 
-try_encrypt(usmNoPrivProtocol, _PrivKey, _Data) -> % 3.1.2
+try_encrypt(usmNoPrivProtocol, _PrivKey, _Data, _EngineBoots, _EngineTime) -> % 3.1.2
     error(unsupportedSecurityLevel);
-try_encrypt(usmDESPrivProtocol, PrivKey, Data) ->
+try_encrypt(usmDESPrivProtocol, PrivKey, Data, _EngineBoots, _EngineTime) ->
     des_encrypt(PrivKey, Data);
-try_encrypt(usmAesCfb128Protocol, PrivKey, Data) ->
-    aes_encrypt(PrivKey, Data).
+try_encrypt(usmAesCfb128Protocol, PrivKey, Data, EngineBoots, EngineTime) ->
+    aes_encrypt(PrivKey, Data, EngineBoots, EngineTime).
 
 authenticate_outgoing(Message, UsmSecParams, 
 		      AuthKey, AuthProtocol, SecLevel) ->
@@ -419,11 +420,8 @@ get_des_salt() ->
     EngineBoots = get_engine_boots(),
     [?i32(EngineBoots), ?i32(SaltInt)].
 
-aes_encrypt(PrivKey, Data) ->
-    EngineBoots = get_engine_boots(), 
-    EngineTime  = get_engine_time(), 
-    snmp_usm:aes_encrypt(PrivKey, Data, fun get_aes_salt/0, 
-			 EngineBoots, EngineTime).
+aes_encrypt(PrivKey, Data, EngineBoots, EngineTime) ->
+    snmp_usm:aes_encrypt(PrivKey, Data, fun get_aes_salt/0, EngineBoots, EngineTime).
 
 aes_decrypt(PrivKey, UsmSecParams, EncData) ->
     #usmSecurityParameters{msgPrivacyParameters        = MsgPrivParams,

lib/snmp/src/misc/snmp_usm.erl

@@ -45,7 +45,7 @@
 
 -define(i32(Int), (Int bsr 24) band 255, (Int bsr 16) band 255, (Int bsr 8) band 255, Int band 255).
 
--define(BLOCK_CIPHER_AES(Key), case size(iolist_to_binary(Key)) of
+-define(BLOCK_CIPHER_AES(Key), case bit_size(iolist_to_binary(Key)) of
                                    128 -> aes_128_cfb128;
                                    192 -> aes_192_cfb128;
                                    256 -> aes_256_cfb128

lib/snmp/test/snmp_manager_SUITE.erl

@@ -66,6 +66,7 @@
 	 register_agent3/1,
 
 	 info/1,
+	 usm_priv_aes/1,
 
 	 simple_sync_get2/1, 
 	 simple_sync_get3/1, 
@@ -184,7 +185,8 @@ groups() ->
      },
      {misc_tests, [], 
       [
-       info
+       info,
+       usm_priv_aes
       ]
      },
      {user_tests, [], 
@@ -1405,6 +1407,157 @@ verify_info([{Key, SubKeys}|Keys], Info) ->
 
 %%======================================================================
 
+% USM privacy fails with AES in OTP 22.2.3. Test to prevent
+% regression in future releases.
+%
+usm_priv_aes(suite) -> [];
+usm_priv_aes(Config) when is_list(Config) ->
+    ?TC_TRY(info,
+            fun() -> do_usm_priv_aes(Config) end).
+
+do_usm_priv_aes(Config) ->
+    p("starting with Config: ~n~p", [Config]),
+
+    ConfDir = ?config(manager_conf_dir, Config),
+    DbDir   = ?config(manager_db_dir, Config),
+
+    write_manager_conf(ConfDir),
+
+    Opts = [{server, [{verbosity, trace}]},
+	    {net_if, [{verbosity, trace}]},
+	    {note_store, [{verbosity, trace}]},
+	    {config, [{verbosity, trace}, {dir, ConfDir}, {db_dir, DbDir}]}],
+
+    p("try starting manager"),
+    ok = snmpm:start(Opts),
+
+    ?SLEEP(1000),
+
+    p("manager started, now generate AES-encrypted message"),
+
+    EngineID = [128,0,0,0,6],
+    SecName  = "v3_user",
+    AuthPass = "authpass",
+    AuthKey  =
+      snmp:passwd2localized_key(sha, AuthPass, EngineID),
+    PrivPass = "privpass",
+    PrivKey  =
+      snmp:passwd2localized_key(md5, PrivPass, EngineID),
+
+    Credentials =
+      [ {auth,     usmHMACSHAAuthProtocol},
+        {auth_key, AuthKey},
+        {priv,     usmAesCfb128Protocol},
+        {priv_key, PrivKey}
+      ],
+
+    AgentConfig =
+      [ {engine_id, EngineID},
+        {address,   {192,0,2,1}},
+        {version,   v3},
+        {sec_model, usm},
+        {sec_level, authPriv},
+        {sec_name,  SecName}
+      ],
+
+    snmpm:register_user(SecName, snmpm_user_default, nil),
+    snmpm:register_usm_user(EngineID, SecName, Credentials),
+    snmpm:register_agent(SecName, "v3_agent", AgentConfig),
+
+    PduType   = 'get-request',
+    ScopedPDU =
+      { scopedPdu,
+        "",        % CtxEngineID
+        "",        % Context
+        { pdu,
+          PduType,
+          0,       % RequestID
+          noError, % ErrorStatus
+          0,       % ErrorIndex
+          [ {varbind, [1,3,6,1,2,1,1,5,0], 'OCTET STRING', [], 0}
+          ]
+        }
+      },
+
+    MsgSecurityParameters =
+      { usmSecurityParameters,
+        _MsgAuthoritativeEngineID    = EngineID,
+        _MsgAuthoritativeEngineBoots = 1,
+        _MsgAuthoritativeEngineTime  = 0,
+        _MsgUserName                 = SecName,
+        _MsgAuthenticationParameters = AuthKey,
+        _MsgPrivacyParameters        = PrivKey
+      },
+
+    {ok, MsgMaxSize} =
+      snmpm_config:get_engine_max_message_size(),
+
+    Message =
+      { message,
+        _Version = 'version-3',
+        { v3_hdr,
+          _MsgID = 1,
+          MsgMaxSize,
+          _MsgFlags = snmp_misc:mk_msg_flags(PduType, 2),
+          _MsgSecurityModel = 3,  % SEC_USM
+          MsgSecurityParameters,
+          0
+        },
+        Data = snmp_pdus:enc_scoped_pdu(ScopedPDU)
+      },
+
+    {_, CredVals} = lists:unzip(Credentials),
+
+    SecLevel = 2,
+
+    Msg =
+      snmpm_usm:generate_outgoing_msg(
+        Message,
+        EngineID,
+        SecName,
+        list_to_tuple([SecName|CredVals]),
+        SecLevel
+      ),
+
+    p("got AES-encrypted message, now decrypt: ~n~p", [Msg]),
+
+    {message, _Version, Hdr, NextData} =
+      snmp_pdus:dec_message_only(Msg),
+
+    { v3_hdr,
+      _MsgID,
+      _MsgMaxSize,
+      _MsgFlags,
+      _SecModel,
+      SecParams,
+      _Hdr_size
+    } = Hdr,
+
+    { ok,
+      { _MsgAuthEngineID,
+        _SecName,
+        ScopedPDUBytes,
+        _CachedSecData
+      }
+    } =
+      snmpm_usm:process_incoming_msg(
+        Msg,
+        NextData,
+        SecParams,
+        SecLevel
+      ),
+
+    Data = ScopedPDUBytes,
+
+    p("Message decrypted, now try to stop"),
+    ok = snmpm:stop(),
+
+    ?SLEEP(1000),
+
+    ok.
+
+%%======================================================================
+
 register_user1(suite) -> [];
 register_user1(Config) when is_list(Config) ->
     Pre  = fun() ->