Puppets security and inheritance

After talking to a follow Puppetlabs employee [Chris][chris] and reading a blog post he has been writing about security issues related to class inheritance.

This got me thinking about ways to protect your code base against this. The following blog post explains the problem as well as a hackish method to try to protect yourself against it.

The problem

Lets say that you have multiple teams managing multiple modules within your puppet infrastructure. Each team is responsible for managing a set of modules that they need to “do their job”. With one SysAdmin team responsible for security of the underlaying OS.

No other team can commit to the SysAdmin repository. As this sets the root password and configuration items needed to pass PCI/DSS. All teams believe this to be a secure system of working.

As any resource can only be declared onces, so no other team could set the root password as this would course an puppet error saying multiple declarations for the root user.

But lets take the follow example to explain why this is wrong:

class security
{
  user{'root':
    ensure   => present,
    password => 'supersecurepassword',
  }
}

node default {
  include security
}

OK so we declare a class that setups all the security related resources on a node, then applies those resource to the nodes. We know no other class can declare the root user but what about the following puppet code:

class evilclass inherits security
{
  User['root']{
    password => 'superevilpassword',
  }
}

node default {
  include security
  include evilclass
}

The example above uses the Puppet inheritance system to override the root users password with the one contained within the evilclass.

In the organization above that includes many teams modules within their infrastructure without doing a code review, this would allow any team to override any resource within the Puppet code base.

As long as they knew the class that actually declared the resource. This could lead to some interesting security issues.

A solution

The easiest solution to the above problem would be to require the Sys Admin team to review all commits to all modules. The issue with this solution is that it only scales so far and requires vast resource from the Sys Admin team.

Another way to fix/try to fix well make it less likely to happen is in Puppet code.

class security
{
  user{'root':
    password => 'supersecurepassword',
  }
}

class evilclass inherits security
{
  User['root']{
    password => 'superevilpassword',
  }
  User <| title == 'root' |>
  {
    password => 'superevilpassword',
  }
}

class secuirtybolt
{
  User <| title == 'root' |>
  {
    password => 'supersecurepassword',
  }
}

node default {
  stage { 'last': }
  Stage['main'] -> Stage['last']
  include security
  include evilclass
  class {'secuirtybolt': stage => 'last'}
}

We are using the collection syntax and overriding the password once more. I have also setup a run stage to try to make our collection run last. Although this isn’t full proof I can’t think of any other way. If you can please comment on the blog post, [chris]:https://puppetlabs.com/blog/secure-puppet-code-collaboration/