Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP: Simpify traditional access control parameters #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: Simpify traditional access control parameters #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 16 additions & 22 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,7 @@ include snmp

### Upgrading

#### Deprecation Warning

##### Past module 3.x series

* The classes `snmp::server` and `snmp::trapd` have been merged into class `snmp`. All of their class parameters available in the `snmp` class.

##### Current module 4.x series

* The parameter `install_client` is renamed to `manage_client`.

* Support for Puppet < 4 is removed.

##### Future module 5.x series

* The parameters `ro_community`, `rw_community`, `ro_network`, and `rw_network` will be removed.

* The snmptrapd parameter name will become `authcommunity`.
Please see the [CHANGELOG](CHANGELOG.md) for details of breaking changes between major releases.

## Usage

Expand All @@ -92,8 +76,8 @@ To change the SNMP community from the default value and limit the netblocks that
```puppet
class { 'snmp':
agentaddress => [ 'udp:161', ],
ro_community => 'myPassword',
ro_network => '192.168.0.0/16',
ro_community => ['myPassword'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point in this as it breaks backward compatibility though in the next release you want to deprecate the option anyhow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this was a step too far. I'll have another think.

ro_network => ['192.168.0.0/16'],
}
```

Expand Down Expand Up @@ -187,11 +171,13 @@ snmp::snmpv3_user { 'myuser':

### Access Control

For access control, it is recommended to configure VACM, (see below), and turn off traditional access control.

With traditional access control, you can give a simple password and (optional) network restriction:
```puppet
class { 'snmp':
ro_community => 'myPassword',
ro_network => '10.0.0.0/8',
ro_community => ['myPassword'],
ro_network => ['10.0.0.0/8'],
}
```
and it becomes this in snmpd.conf:
Expand All @@ -200,6 +186,14 @@ rocommunity myPassword 10.0.0.0/8
```
This says that any host on network 10.0.0.0/8 can read any SNMP value via SNMP versions 1 and 2c as long as they provide the password 'myPassword'.

To disable traditional access control make sure you override the `ro_community` and `ro_community6` parameters.
```puppet
class { 'snmp':
ro_community => [],
ro_community6 => [],
# ...
}

With View-based Access Control Model (VACM), you can do this (more complex) configuration instead:
```puppet
class { 'snmp':
Expand Down Expand Up @@ -233,7 +227,7 @@ Reference: [Manpage of snmpd.conf - Access Control](http://www.net-snmp.org/docs
In traditional access control, you can also pass multiple networks for the community string.
```puppet
class { 'snmp':
ro_community => 'shibboleth',
ro_community => ['shibboleth'],
ro_network => [ '192.168.0.0/16', '1.2.3.4/32', ],
}
```
Expand Down
50 changes: 25 additions & 25 deletions REFERENCE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class { 'snmp':

# Only configure and run the snmptrap daemon:
class { 'snmp':
ro_community => 'SeCrEt',
ro_community => ['SeCrEt'],
service_ensure => 'stopped',
trap_service_ensure => 'running',
trap_handlers => [
Expand Down Expand Up @@ -69,67 +69,67 @@ Default value: [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ]

##### `ro_community`

Data type: `Variant[Undef, String[1], Array[String[1]]]`
Data type: `Array[String[1]]`

Read-only (RO) community string or array for agent and snmptrap daemon.
Read-only (RO) array of community strings for agent and snmptrap daemon.

Default value: 'public'
Default value: ['public']

##### `ro_community6`

Data type: `Variant[Undef, String[1], Array[String[1]]]`
Data type: `Array[String[1]]`

Read-only (RO) community string or array for IPv6 agent.
Read-only (RO) array of community strings for IPv6 agent.

Default value: 'public'
Default value: ['public']

##### `rw_community`

Data type: `Variant[Undef, String[1], Array[String[1]]]`
Data type: `Array[String[1]]`

Read-write (RW) community string or array agent.
Read-write (RW) array of community strings for agent.

Default value: `undef`
Default value: []

##### `rw_community6`

Data type: `Variant[Undef, String[1], Array[String[1]]]`
Data type: `Array[String[1]]`

Read-write (RW) community string or array for IPv6 agent.
Read-write (RW) array of community strings for IPv6 agent.

Default value: `undef`
Default value: []

##### `ro_network`

Data type: `Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR]`
Data type: `Array[Stdlib::IP::Address::V4]`

Network that is allowed to RO query the daemon. Can be string or array.
Networks that are allowed to RO query the daemon.

Default value: '127.0.0.1'
Default value: ['127.0.0.1']

##### `ro_network6`

Data type: `Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR]`
Data type: `Array[Stdlib::IP::Address::V6]`

Network that is allowed to RO query the daemon via IPv6. Can be string or array.
Networks that are allowed to RO query the daemon via IPv6.

Default value: '::1'
Default value: ['::1']

##### `rw_network`

Data type: `Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR]`
Data type: `Array[Stdlib::IP::Address::V4]`

Network that is allowed to RW query the daemon. Can be string or array.
Networks that are allowed to RW query the daemon.

Default value: '127.0.0.1'
Default value: ['127.0.0.1']

##### `rw_network6`

Data type: `Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR]`
Data type: `Array[Stdlib::IP::Address::V6]`

Network that is allowed to RW query the daemon via IPv6. Can be string or array.
Networks that are allowed to RW query the daemon via IPv6.

Default value: '::1'
Default value: ['::1']

##### `contact`

Expand Down
2 changes: 1 addition & 1 deletion examples/snmpd-snmptrapd.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class { 'snmp':
ro_community => 'SeCrEt',
ro_community => ['SeCrEt'],
trap_service_ensure => 'running',
trap_service_enable => true,
trap_handlers => [
Expand Down
2 changes: 1 addition & 1 deletion examples/trapd.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class { 'snmp':
ro_community => 'SeCrEt',
ro_community => ['SeCrEt'],
service_ensure => 'stopped',
trap_service_ensure => 'running',
trap_service_enable => true,
Expand Down
52 changes: 26 additions & 26 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# # Only configure and run the snmptrap daemon:
# class { 'snmp':
# ro_community => 'SeCrEt',
# ro_community => ['SeCrEt'],
# service_ensure => 'stopped',
# trap_service_ensure => 'running',
# trap_handlers => [
Expand All @@ -27,28 +27,28 @@
# SNMP notifications.
#
# @param ro_community
# Read-only (RO) community string or array for agent and snmptrap daemon.
# Read-only (RO) array of community strings for agent and snmptrap daemon.
#
# @param ro_community6
# Read-only (RO) community string or array for IPv6 agent.
# Read-only (RO) array of community strings for IPv6 agent.
#
# @param rw_community
# Read-write (RW) community string or array agent.
# Read-write (RW) array of community strings for agent.
#
# @param rw_community6
# Read-write (RW) community string or array for IPv6 agent.
# Read-write (RW) array of community strings for IPv6 agent.
#
# @param ro_network
# Network that is allowed to RO query the daemon. Can be string or array.
# Networks that are allowed to RO query the daemon.
#
# @param ro_network6
# Network that is allowed to RO query the daemon via IPv6. Can be string or array.
# Networks that are allowed to RO query the daemon via IPv6.
#
# @param rw_network
# Network that is allowed to RW query the daemon. Can be string or array.
# Networks that are allowed to RW query the daemon.
#
# @param rw_network6
# Network that is allowed to RW query the daemon via IPv6. Can be string or array.
# Networks that are allowed to RW query the daemon via IPv6.
#
# @param contact
# Responsible person for the SNMP system.
Expand Down Expand Up @@ -245,23 +245,23 @@
# Group of `var_net_snmp` directory.
#
class snmp (
Enum['present','absent'] $ensure = 'present',
Array[String[1]] $agentaddress = [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ],
Array[String[1]] $snmptrapdaddr = [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ],
Variant[Undef, String[1], Array[String[1]]] $ro_community = 'public',
Variant[Undef, String[1], Array[String[1]]] $ro_community6 = 'public',
Variant[Undef, String[1], Array[String[1]]] $rw_community = undef,
Variant[Undef, String[1], Array[String[1]]] $rw_community6 = undef,
Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR] $ro_network = '127.0.0.1',
Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR] $ro_network6 = '::1',
Variant[Array, Stdlib::IP::Address::V4, Stdlib::IP::Address::V4::CIDR] $rw_network = '127.0.0.1',
Variant[Array, Stdlib::IP::Address::V6, Stdlib::IP::Address::V6::CIDR] $rw_network6 = '::1',
String[1] $contact = 'Unknown',
String[1] $location = 'Unknown',
String[1] $sysname = $facts['networking']['fqdn'],
Integer $services = 72,
Array[String[1]] $com2sec = [ 'notConfigUser default public' ],
Array[String[1]] $com2sec6 = [ 'notConfigUser default public' ],
Enum['present','absent'] $ensure = 'present',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most modules we align the =, but I'm not sure if we should enforce this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's become pretty hard to do this and not have it look ridiculous.

Copy link
Member

@ghoneycutt ghoneycutt Feb 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With regards to styling of class parameters, I'm no longer doing any special alignment between the data type, parameter and value and am only doing a single space between each.

Data types break down the ability to have a style that makes the code readable. With the advent of REFERENCE.md, you can always look there to see all the parameters and their default values.

Copy link
Member

@Dan33l Dan33l Feb 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the = is noisy when using git diff.
Due to this, IMO we should reconsider our default policy about this.

I am in favor of no longer aligning the =

Array[String[1]] $agentaddress = [ 'udp:127.0.0.1:161', 'udp6:[::1]:161' ],
Array[String[1]] $snmptrapdaddr = [ 'udp:127.0.0.1:162', 'udp6:[::1]:162' ],
Array[String[1]] $ro_community = ['public'],
Array[String[1]] $ro_community6 = ['public'],
Array[String[1]] $rw_community = [],
Array[String[1]] $rw_community6 = [],
Array[Stdlib::IP::Address::V4] $ro_network = ['127.0.0.1'],
Array[Stdlib::IP::Address::V6] $ro_network6 = ['::1'],
Array[Stdlib::IP::Address::V4] $rw_network = ['127.0.0.1'],
Array[Stdlib::IP::Address::V6] $rw_network6 = ['::1'],
String[1] $contact = 'Unknown',
String[1] $location = 'Unknown',
String[1] $sysname = $facts['networking']['fqdn'],
Integer $services = 72,
Array[String[1]] $com2sec = [ 'notConfigUser default public' ],
Array[String[1]] $com2sec6 = [ 'notConfigUser default public' ],
Array[String[1]] $groups = [
'notConfigGroup v1 notConfigUser',
'notConfigGroup v2c notConfigUser',
Expand Down
8 changes: 4 additions & 4 deletions spec/classes/snmp_init_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@
end
end

describe 'ro_network => "127.0.0.2"' do
let(:params) { { ro_network: '127.0.0.2' } }
describe 'ro_network => ["127.0.0.2"]' do
let(:params) { { ro_network: ['127.0.0.2'] } }

it 'contains File[snmpd.conf] with contents "127.0.0.2"' do
verify_contents(catalogue, 'snmpd.conf', [
Expand All @@ -460,8 +460,8 @@
end
end

describe 'ro_community => [ "a", "b", ] and ro_network => "127.0.0.2"' do
let(:params) { { ro_community: %w[a b], ro_network: '127.0.0.2' } }
describe 'ro_community => [ "a", "b", ] and ro_network => ["127.0.0.2"]' do
let(:params) { { ro_community: %w[a b], ro_network: ['127.0.0.2'] } }

it 'contains File[snmpd.conf] with contents "a 127.0.0.2" and "b 127.0.0.2"' do
verify_contents(catalogue, 'snmpd.conf', [
Expand Down
16 changes: 8 additions & 8 deletions templates/snmpd.conf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,23 @@ agentaddress <%= @agentaddress.join(',') %>

# ------------------------------------------------------------------------------
# Traditional Access Control
<%- [*@ro_community].compact.each do |c| -%>
<%- [*@ro_network].compact.each do |n| -%>
<%- @ro_community.each do |c| -%>
<%- @ro_network.each do |n| -%>
rocommunity <%= c %> <%= n %>
<%- end -%>
<%- end -%>
<%- [*@ro_community6].compact.each do |c| -%>
<%- [*@ro_network6].compact.each do |n| -%>
<%- @ro_community6.each do |c| -%>
<%- @ro_network6.each do |n| -%>
rocommunity6 <%= c %> <%= n %>
<%- end -%>
<%- end -%>
<%- [*@rw_community].compact.each do |c| -%>
<%- [*@rw_network].compact.each do |n| -%>
<%- @rw_community.each do |c| -%>
<%- @rw_network.each do |n| -%>
rwcommunity <%= c %> <%= n %>
<%- end -%>
<%- end -%>
<%- [*@rw_community6].compact.each do |c| -%>
<%- [*@rw_network6].compact.each do |n| -%>
<%- @rw_community6.each do |c| -%>
<%- @rw_network6.each do |n| -%>
rocommunity6 <%= c %> <%= n %>
<%- end -%>
<%- end -%>
Expand Down
2 changes: 1 addition & 1 deletion templates/snmptrapd.conf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ doNotLogTraps <%= @do_not_log_traps %>
################################################################################
# ACCESS CONTROL

<%- [*@ro_community].compact.each do |c| -%>
<%- @ro_community.each do |c| -%>
authCommunity log,execute,net <%= c %>
<%- end -%>
disableAuthorization <%= @disable_authorization %>
Expand Down