-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Secure SNMPv3 user creation #46
base: master
Are you sure you want to change the base?
Conversation
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 Regarding the failing ⧗ 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 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 Only an initial review, just to get the CI working, hopefully. @alxwr Will you be able to look over this PR? |
@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 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:
|
@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 |
adding simple vim modelines for convenience.
Gentoo is the only one affected, but this is technically a timebomb.
* `init.sls` - This will now enable the service, restarting on config changes normally. * `conf.sls` - Having the watch_in:service on the file.managed state will cause issues if any additional states follow `snmp_conf`.
Added a generic default message to avoid an awkward blank line.
* `map.jinja` - Adding the file path for persistent configs. These files will hold the actual createUser strings before they're consumed. * `conf.sls` - New users' createUser strings will be placed in the `persistentconfig` files, which will consumed on snmp daemon start. The process is stop snmpd, add createUser to the `persistentconfig` file, and then start snmpd back up. This somewhat opinionated choice to stop & start per user avoided a significantly large and needlessly complex alternative. * `snmpd.conf` - Allow defaults/blanks for access entries, where appropriate. Changed `location` to `sysLocation` and `syscontact` to `sysContact` to match snmpd.conf options identically. Changed `logconnects` to `dontLogTCPWrappersConnects` for the same reason and readability. * `snmpd.conf.minimal` - Almost identical changes. * `macros.jinja` - added macro for the createUser string. BREAKING CHANGE: `location` is no longer accepted and has been replaced with `sysLocation` to match the snmpd.conf standard. BREAKING CHANGE: `syscontact` is no longer accepted and has been replaced with `sysContact` to match the snmpd.conf standard. BREAKING CHANGE: `logconnects` is no longer accepted and has been replaced with `dontLogTCPWrappersConnects` to match the snmpd.conf standard, as well as removing less than intuitive boolean.
* `pillar.example` - updated to match secure v3 users functionality * `README.rst` - similarly updated
* `config.rb` - Now aware of the correct createUser state * `default.sls` - Updated to match new snmpd.conf
It should be `onchanges`, not `require` as it triggers if step 1 completes with 'unless condition is true'
* pillar.example - corrected to match yamllint guidelines * snmp/conf.sls - exploded into variables to stay below 160 char * snmp/macros.jinja - corrected style issues * config.rb - fixed to snake_cake
Co-authored-by: Imran Iqbal <[email protected]>
4290518
to
4ca9591
Compare
PR progress checklist (to be filled in by reviewers)
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[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
[docs]
Documentation changes[test]
Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE
?Yes, there are a few breaking changes.
logconnect
has been changed todontLogTCPWrappersConnects
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 tosysContact
to also match the snmpd.conf option.location
changed tosysLocation
to also match the snmpd.conf option.Related issues and/or pull requests
Describe the changes you're proposing
Pillar / config required to test the proposed changes
None, files
test/integration/default/controls/config.rb
andtest/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
CentOS 8 3004.0 py3
Debian 9 3003.3 py3
Debian 9 3004.0 py3
Debian 10 3003.3 py3
Debian 10 3004.0 py3
Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context