snmp-formula icon indicating copy to clipboard operation
snmp-formula copied to clipboard

Secure SNMPv3 user creation

Open bbilyeu opened this issue 3 years ago • 6 comments

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.

  1. 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.
  2. syscontact changed to sysContact to also match the snmpd.conf option.
  3. 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 avatar Jan 21 '22 00:01 bbilyeu

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.

bbilyeu avatar Feb 01 '22 22:02 bbilyeu

@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 avatar Feb 03 '22 13:02 myii

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

myii avatar Feb 03 '22 13:02 myii

@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?

bbilyeu avatar Feb 04 '22 16:02 bbilyeu

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 avatar Feb 04 '22 17:02 myii

@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

myii avatar Feb 04 '22 17:02 myii