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.

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

firewalld::custom_service creates files with invalid names #265

Closed
trevor-vaughan opened this issue Mar 3, 2020 · 0 comments · Fixed by #266
Closed

firewalld::custom_service creates files with invalid names #265

trevor-vaughan opened this issue Mar 3, 2020 · 0 comments · Fixed by #266

Comments

@trevor-vaughan
Copy link
Collaborator

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: All
  • Ruby: All
  • Distribution: All
  • Module version: 4.2.2

How to reproduce (e.g Puppet code you use)

firewalld::custom_service { 'foo.bar.baz': }

What are you seeing

Firewalld error on restarting: Error: INVALID_NAME: '.' is not allowed in 'foo.bar.baz'

What behaviour did you expect instead

The module should scrub names for safety

Any additional information you'd like to impart

Working on a patch.

Determined the disallowed set of characters using the following code:

require 'shellwords'

chars = (0..9).to_a + ('A'..'z').to_a + ('!'..'?').to_a

chars.each do |char|
  name = Shellwords.escape(%(test#{char}thing))
  %x{firewall-cmd --permanent --new-service=#{name}}
end

Output:

Error: INVALID_NAME: '[' is not allowed in 'test[thing'
Error: INVALID_NAME: '' is not allowed in 'test\thing'
Error: INVALID_NAME: ']' is not allowed in 'test]thing'
Error: INVALID_NAME: '^' is not allowed in 'test^thing'
Error: INVALID_NAME: '' is not allowed in 'testthing'
Error: INVALID_NAME: '!' is not allowed in 'test!thing'
Error: INVALID_NAME: '"' is not allowed in 'test"thing'
Error: INVALID_NAME: '#' is not allowed in 'test#thing'
Error: INVALID_NAME: '$' is not allowed in 'test$thing'
Error: INVALID_NAME: '%' is not allowed in 'test%thing'
Error: INVALID_NAME: '&' is not allowed in 'test&thing'
Error: INVALID_NAME: ''' is not allowed in 'test'thing'
Error: INVALID_NAME: '(' is not allowed in 'test(thing'
Error: INVALID_NAME: ')' is not allowed in 'test)thing'
Error: INVALID_NAME: '' is not allowed in 'testthing'
Error: INVALID_NAME: '+' is not allowed in 'test+thing'
Error: INVALID_NAME: ',' is not allowed in 'test,thing'
Error: INVALID_NAME: '.' is not allowed in 'test.thing'
Error: INVALID_NAME: '/' is not allowed in 'test/thing'
Error: INVALID_NAME: ':' is not allowed in 'test:thing'
Error: INVALID_NAME: ';' is not allowed in 'test;thing'
Error: INVALID_NAME: '<' is not allowed in 'test<thing'
Error: INVALID_NAME: '=' is not allowed in 'test=thing'
Error: INVALID_NAME: '>' is not allowed in 'test>thing'
Error: INVALID_NAME: '?' is not allowed in 'test?thing'

This basically means that you can have the following characters in the name: ('A'..'z').to_a + ('0'..'9').to_a + ['_','-']

trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 6, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
trevor-vaughan added a commit to trevor-vaughan/pupmod-voxpupuli-firewalld that referenced this issue Mar 7, 2020
This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes voxpupuli#265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant