Fix PrivParams for SNMPv3 USM with AES privacy

[jonnystorm]

• 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.

[jonnystorm]

This is a continuation of #1874.

[bmk]

I have added the testing label. But as we have code freeze on
Friday (for 23), I don't know how far i t will come.

[jonnystorm]

Thank you, @bmk. Clearly, I'm in no great rush. :)

[bmk]

Hi,

First night of testrun, and we have a failure.
I have uploaded the log (to a gist) from this
test case:

https://gist.github.com/bmk/0a049cda6fa13cbc0d3daaaa29aff4a6

Another thing was that this test case, just as
many other test cases, start and stop the manager
in the test case, but when (if) they crash out,
they leave the manager running, causing problems
for later test cases.

I have tweaked your test case a bit (to fix this).
Putting the manager start and stop in the pre and
post fun's respectively. See below.
Unfortunately, when I test on my ws it works just
fine. I don't think my changes could have made it
work, since it has nothing to do with the core of
the test case. The difference was that I ran my
tests on maint but the nightly tests was on master.
Different versions of crypto?

Another thing is that although you have indicated
that you want your branch merged into master, its
actually based on maint.
This is not a problem during testing (unless this is
what caused the problems), but could cause issues
when we aventually merge. So, eventually, I would
like you to rebase on master (if that is the ultimate
"destination").

And here are my changes:

usm_priv_aes(suite) -> [];
usm_priv_aes(Config) when is_list(Config) ->
    Pre = fun() ->
                  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) % Give it time to settle
          end,
    Case = fun(ok) -> do_usm_priv_aes(Config) end,
    Post = fun(ok) ->
                   p("try stop manager"),
                   ok = snmpm:stop(), % Give it time to settle
                   ?SLEEP(1000)
           end,
    ?TC_TRY(usm_priv_aes, Pre, Case, Post).

do_usm_priv_aes(Config) ->
    p("starting with Config: "
      "~n      ~p", [Config]),

    p("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"),
    ok.

I will, for now, replace your branch with mine (which is based on yours).
Also, I will add it to maint to see if there is something in master that causes the issues.

Regards,
/BMK

[jonnystorm]

Unfortunately, I'm unable to see the log. Is that because it's secret?

I didn't even realize I could add pre- and post-test functions. Thanks for the clean-up.

Oops... I meant to merge to maint, but if the test fails against master, then the mistake was serendipitous. I'll see if I can find the problem, today.

[bmk]

Stupidly, I never tested that I could access the file. I just assumed it worked.
Its possible to "download", but the result is crap.
Anyway, I added the log as a comment instead...

I previously just used to the pre and post fun:s for starting and
stopping slave nodes. But I should use for the manager start and
stop also.

That it could have something to do with master is just a guess.

I have vacation tomorrow, so you have plenty of time :)

Regards,
/BMK

[jonnystorm]

I've been guilty of stupider. :)

I'll see what I can sort out. Have a great vacation!

[jonnystorm]

Fixed counting bug in snmp_usm macro ?BLOCK_CIPHER_AES, introduced by commit 8d942cd.

[bmk]

Hi,
I know we discussed what this should be based on, maint or master.
You wrote maint (I think), but the changes in snmp_usm.erl is actually
based on master.
This is not really a problem (we could release this in 23), but I thought
I should ask before the 22.3 release window passes...

[jonnystorm]

I had originally intended to merge against maint, but finding the bug in master, I thought I should merge against that, instead. If you think I should bring it back to maint, though, I certainly will.

[bmk]

Master it is then :)

[bmk]

Hi,

Merged "my branch" (with my two commits) with your commit (d98b111)
into master (Feb 25).

Are there any remaining issues? I see that you have merged master into this branch.

[jonnystorm]

Sorry, I was notified of conflicts, so I resolved them through GitHub. In retrospect, that was a mistake, and I should have performed the usual rebase and merge, myself, rather than use the site.

I can see master shows the new commits, so I'll close this now. Thanks so much for all your help!

[bmk]

Not a problem. Thanks for the contributions. Keep them coming :)