Secure SNMPv3 user creation

bbilyeu commented 2 years ago

PR progress checklist (to be filled in by reviewers)

[ ] Changes to documentation are appropriate (or tick if not required)
[ ] Changes to tests are appropriate (or tick if not required)
[ ] Reviews completed

What type of PR is this?

Primary type

[ ] [build] Changes related to the build system
[ ] [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
[ ] [ci] Changes to the continuous integration configuration
[x] [feat] A new feature
[ ] [fix] A bug fix
[ ] [perf] A code change that improves performance
[ ] [refactor] A code change that neither fixes a bug nor adds a feature
[ ] [revert] A change used to revert a previous commit
[ ] [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

[x] [docs] Documentation changes
[x] [test] Adding missing or correcting existing tests

Does this PR introduce a `BREAKING CHANGE`?

Yes, there are a few breaking changes.

logconnect has been changed to dontLogTCPWrappersConnects which identically matches the snmpd.conf option (instead of forcing a formula specific value). This also corrects a slightly less than intuitive boolean usage.
syscontact changed to sysContact to also match the snmpd.conf option.
location changed to sysLocation to also match the snmpd.conf option.

Related issues and/or pull requests

Describe the changes you're proposing

First and foremost, this addresses the SNMP user/pass being dumped into snmpd.conf in plaintext.
- The specific workflow for Deb/RHEL systems is test SNMP access with credentials -> (on fail) stop snmpd -> add createUser string to the correct config file (/var/lib/____/snmpd.conf) -> start snmpd back up. Doing so will cause the credentials to be "consumed", converted into something no longer human readable.
Second, it standardizes a few of the options and moves closer to the Saltstack formula recommendation of "sane default values".

Pillar / config required to test the proposed changes

None, files test/integration/default/controls/config.rb and test/salt/pillar/default.sls were updated to all turnkey testing.

Debug log showing how the proposed changes work

CentOS 7 3003.3 and 3004.0 (both py3) would fail to start up SSH. Skipping those

CentOS 8 3003.3 py3

-----> Verifying <default-centos-8-3003-3-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:61297

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/net-snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package net-snmp is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

CentOS 8 3004.0 py3

-----> Verifying <default-centos-8-3004-0-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:60780

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/net-snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package net-snmp is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

Debian 9 3003.3 py3

-----> Verifying <default-debian-9-3003-3-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:61403

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package snmpd is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

Debian 9 3004.0 py3

-----> Verifying <default-debian-9-3004-0-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:60961

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package snmpd is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

Debian 10 3003.3 py3

-----> Verifying <default-debian-10-3003-3-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:61477

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package snmpd is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

Debian 10 3004.0 py3

-----> Verifying <default-debian-10-3004-0-py3>...
       Loaded default

Profile: snmp formula (default)
Version: (not specified)
Target:  ssh://kitchen@localhost:60885

  ✔  snmp.config.file: Verify the configuration file
     ✔  File /etc/snmp/snmpd.conf is expected to be file
     ✔  File /etc/snmp/snmpd.conf is expected to be owned by "root"
     ✔  File /etc/snmp/snmpd.conf is expected to be grouped into "root"
     ✔  File /etc/snmp/snmpd.conf mode is expected to cmp == "0644"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysLocation Right Here"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "sysContact System Admin"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "dontLogTCPWrappersConnects yes"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "view all included .1 80"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       localhost"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.0.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rocommunity     public       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rwcommunity     private       192.168.1.0/24"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "rouser myv3user auth -V all"
     ✔  File /etc/snmp/snmpd.conf content is expected to include "createUser string will be added to /var/lib/snmp/snmpd.conf"
  ✔  snmp.package.install: The required package should be installed
     ✔  System Package snmpd is expected to be installed
  ✔  snmp.service.running: The service should be installed, enabled and running
     ✔  Service snmpd is expected to be installed
     ✔  Service snmpd is expected to be enabled
     ✔  Service snmpd is expected to be running

Documentation checklist

[x] Updated the README (e.g. Available states).
[x] Updated pillar.example.

Testing checklist

[x] Included in Kitchen (i.e. under state_top).
[x] Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
[x] Updated the relevant test pillar.

Additional context

bbilyeu commented 2 years ago

Apologies for the radio silence!

4903638765 :: This is a foolish mistake on my part, failing only due to improper casing of the commit subject.

4890361620 :: (EDIT) Resolved

5030042061 :: All the rest are failing due to something specific to Saltstack master branch (3005?), which isn't live yet.

myii commented 2 years ago

@bbilyeu Regarding the failing commitlint job:

⧗   input: style(*): Added vim modelines
Adding simple vim modelines for convenience.
✖   subject must not be sentence-case, start-case, pascal-case, upper-case [subject-case]
✖   found 1 problems, 0 warnings

Please amend the commit title accordingly:

-style(*): Added vim modelines
+style(*): added vim modelines

In terms of the Rendering SLS 'base:snmp.conf' failed: could not find expected ':' failures:

       snmpv3 creating myv3user step 2 of 3:
         file.line:
           - name: /var/lib/snmp/snmpd.conf
           - mode: insert
           - location: end
           - content: 

       createUser myv3user SHA myv3password AES v3privpass
           - show_changes: False
           - onchanges:
             - snmpv3 creating myv3user step 1 of 3

This is happening because the whitespace control for createUser ... macro needs to be amended. I've suggested something inline.

Only an initial review, just to get the CI working, hopefully.

@alxwr Will you be able to look over this PR?

myii commented 2 years ago

@bbilyeu Actually, when you amend the commit, would you mind rebasing this PR on the latest commit to this repo? That will use the updated CI matrix.

bbilyeu commented 2 years ago

@bbilyeu Actually, when you amend the commit, would you mind rebasing this PR on the latest commit to this repo? That will use the updated CI matrix.

I apologize, but my rebase knowledge/experience is embarrassingly weak. Would amending the commit and rebasing not bloat the commit history with duplicates?

myii commented 2 years ago

I apologize, but my rebase knowledge/experience is embarrassingly weak. Would amending the commit and rebasing not bloat the commit history with duplicates?

@bbilyeu No, it's an expected (and usually preferred) procedure. Once you've rebased and amended the commit message, you need to force push it back here.

This is useful documentation:

https://docs.gitlab.com/ee/topics/git/git_rebase.html#interactive-rebase
- The force push (using --force-with-lease) is discussed in the section directly below the one I've linked.

myii commented 2 years ago

@bbilyeu I've just noticed that the very last commit message needs to be updated as well:

-Update snmp/macros.jinja
+fix(macros.jinja): fix macro `v3_createUser_string` whitespace control

saltstack-formulas / snmp-formula