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

icinga2 feature api fails when pki=icinga2 and ca is the local node #218

Closed
erezzarum opened this issue Jan 24, 2017 · 14 comments
Closed

icinga2 feature api fails when pki=icinga2 and ca is the local node #218

erezzarum opened this issue Jan 24, 2017 · 14 comments
Labels
Milestone

Comments

@erezzarum
Copy link

When creating a new CA using "icinga2::pki::ca" the ca.crt file is not copied to the default pki directory (/etc/icinga2/pki/ca.crt).
It makes it impossible to start a fresh icinga2 master without an existing CA (i.e: creating a fresh master with a CA) using "icinga2::feature::api"
My suggestion is to allow the "ca_host" param under "icinga2::feature::api" to be undef or any other parameter (maybe if the ca_host is the same as fqdn or if the ca_host is 'master') and then simply copying the ca.crt from the ca_dir into the defined ssl_ca_cert_path parameter.
I suggest not copying it directly over to /etc/icinga2/pki/ca.crt by the "icinga2::pki::ca" class as a user may use a different ssl_ca_cert_path for the api.

  if ($ca_host == undef) {
    file { $_ssl_cacert_path:
      ensure => file,
      source => "file://${ca_dir}/ca.crt",
      tag    => 'icinga2::config::file',
    }
  } else {
    validate_string($ca_host)
    exec { 'icinga2 pki get trusted-cert':
      command => "icinga2 pki save-cert --host '${ca_host}' --port ${ca_port} --key '${_ssl_key_path}' --cert '${_ssl_cert_path}' --trustedcert '${trusted_cert}'",
      creates => $trusted_cert,
      notify  => Class['::icinga2::service'],
      require => File[[$_ssl_key_path, $_ssl_cert_path]]
    } ->
    file { $trusted_cert: } ->

    exec { 'icinga2 pki request':
      command => "icinga2 pki request --host '${ca_host}' --port ${ca_port} --ca '${_ssl_cacert_path}' --key '${_ssl_key_path}' --cert '${_ssl_cert_path}' --trustedcert '${trusted_cert}' --ticket '${ticket_id}'",
      creates => $_ssl_cacert_path,
      notify  => Class['::icinga2::service'],
    } ->
    file { $_ssl_cacert_path: }
  }
} # icinga2  
erezzarum pushed a commit to erezzarum/puppet-icinga2 that referenced this issue Jan 24, 2017
@bobapple
Copy link
Contributor

I'm not sure I fully understand the issue. Can you paste the puppet code here that leads to this issue?

@erezzarum
Copy link
Author

If i try to setup a fresh icinga2 master with a self signed CA and enabling the API (icinga2::feature::api with pki=icinga2) it fails, because:

  1. It doesn't have the CA cert file under the ssl_cacert_path (defaults to /etc/icinga2/pki/ca.crt), the icinga2::pki::ca creates it at /var/lib/icinga2/ca/ca.crt which is being used for a different purposes.
  2. It can't fetch the CA cert file because the icinga2 daemon can't be started without the ssl_cacert_path so it can't contact the CA.

To summarize, you can't bootstrap a fresh Icinga2 master with the api feature enabled using pki=icinga2 unless you already have a running icinga2 "master" acting as a CA, you can however do it, but in a few puppet runs, not on one run.

I have forked the repository and committed a fix, it shows here as well.

@erezzarum
Copy link
Author

My fix is not complete, It has to go through a different command set.

  1. Copy <CA_DIR>/ca.crt to <PKI_DIR>/ca.crt
  2. Generate a certificate request (creates a key file and a csr file)
    icinga2 pki new-cert --cn <FQDN> --key <SSL_KEY_PATH> --csr <SSL_CSR_PATH>
  3. Sign the CSR
    icinga2 pki sign-csr --csr <SSL_CSR_PATH> --cert <SSL_CERT_PATH>

I am still thinking where this should be created, when we generate a new CA by calling the icinga2::pki::ca class, or when calling the icinga2::feature::api class when issuing new certificates for the Node and no ca_host is defined (we assume the Node is the CA for itself).

After playing with Icinga2 for a while in the last couple of days it looks like the best place is inside the "icinga2::pki::ca" class, what do you think?

@bobapple
Copy link
Contributor

bobapple commented Jan 30, 2017

I am not sure if this can be solved at all, it's an chicken and egg problem.

When using pki=icinga in the icinga2::feature::api class, it needs access to an existing Icinga 2 API to receive the trusted certificate and send a signing request.

So the module would need to enable an initial API that can be used to sign certificates, before signing the actual certificates for the master.

I don't see how this would be possible in one single step.

Here's some code to reproduce this:

class { '::icinga2':
  constants => {
    'TicketSalt'   => '5a3d695b8aef8f18452fc494593056a4',
  }
}

class { '::icinga2::feature::api':
  pki             => 'icinga2',
  ca_host         => 'icinga2-puppet.example.com',
  ticket_salt     => '5a3d695b8aef8f18452fc494593056a4',
  accept_config   => true,
  accept_commands => true,
  endpoints       => {
    'icinga2-puppet.example.com' => {
      'host' => 'icinga2-puppet.example.com',
    }
  },
  zones           => {
    'master' => {
      'endpoints' => ['icinga2-puppet.example.com']
    }
  }
}


class { '::icinga2::pki::ca':
}

@bobapple
Copy link
Contributor

bobapple commented Jan 30, 2017

@erezzarum I have a branch with a possible fix: https://github.com/Icinga/puppet-icinga2/tree/bug/client-certs-on-ca-master-218

Can you test this and give me feedback, please?

Make sure to set pki=ca for the api feature on your CA master:

class { '::icinga2':
  constants => {
    'TicketSalt'   => '5a3d695b8aef8f18452fc494593056a4',
  }
}

class { '::icinga2::feature::api':
  pki             => 'ca',
  accept_config   => true,
  accept_commands => true,
  endpoints       => {
    'icinga2-puppet.int.netways.de' => {
      'host' => 'icinga2-puppet.int.netways.de',
    }
  },
  zones           => {
    'master' => {
      'endpoints' => ['icinga2-puppet.int.netways.de']
    }
  }
}

@bobapple
Copy link
Contributor

Updated the branch one more time

@bobapple bobapple added the bug label Jan 30, 2017
@bobapple bobapple added this to the v1.0.3 milestone Jan 30, 2017
erezzarum pushed a commit to erezzarum/puppet-icinga2 that referenced this issue Jan 31, 2017
refs voxpupuli#218

- Remove invalid byte sequence in UTF-8
- Set correct permissions for pki dir
- Sign the certificate with icinga2 local CA
@erezzarum
Copy link
Author

erezzarum commented Jan 31, 2017

I liked the idea of having the pki=ca i didn't think about it :)

You can sign the certificate locally on the icinga2 CA instance, the icinga2 pki sign-csr command is a normal certificate signing procedure (like you will do with self signed certificate creaed by openssl for example, which is actually the same case).
If the certificate is not signed we will receive an SSL "error 18" which tells us that we use a self signed certificate, these, the icinga2 instance will behave as a CA but won't be able to have agents/satelites connecting to him, the agents/satelites/etc. will have a correct certificate though (different mechanism when using the ticketsalt).

$ openssl verify -CAfile /etc/icinga2/pki/ca.crt /etc/icinga2/pki/puppet2.local.crt
/etc/icinga2/pki/puppet2.local.crt: CN = puppet2.local
error 18 at 0 depth lookup:self signed certificate
OK
$ openssl verify -CAfile /var/lib/icinga2/ca/ca.crt /etc/icinga2/pki/puppet2.local.crt
/etc/icinga2/pki/puppet2.local.crt: CN = puppet2.local
error 18 at 0 depth lookup:self signed certificate
OK

I have created a fix that should solve that, tested on a clean vagrant box.
I'm not sure if it's needed (security wise) but i also made sure to delete the CSR after.
I also added a protection in case someone delete the certificate by accident so it will generate it again.
Besides that, your code had some kind of invalid byte sequence so i removed it as well.
The expected result:

$ openssl verify -CAfile /etc/icinga2/pki/ca.crt /etc/icinga2/pki/puppet2.local.crt   
/etc/icinga2/pki/puppet2.local.crt: OK
$ openssl verify -CAfile /var/lib/icinga2/ca/ca.crt /etc/icinga2/pki/puppet2.local.crt
/etc/icinga2/pki/puppet2.local.crt: OK

A small suggestion, it could be possible that users did not notice this bug but i think it will be wise to alert that using pki=ca also includes the icinga2::pki::ca class to avoid duplicate resources or another option is to declare the icinga2::pki::ca class in an include like of a resource like.

@bobapple
Copy link
Contributor

You are absolutely right, I missed the signing part completely.
Your changes look good to me, I would merge them if you are ready.

Just one thing: I don't think its a good idea to set permissions for $pki_dir, this could break things on Windows. I don't remember having an issue with the default file permissions, but if there is one, we should adjust permissions on file basis rather than for the directory.

@erezzarum
Copy link
Author

erezzarum commented Jan 31, 2017

Well, i agree with you, i can't test it on windows :( so sorry!
I did have a permission issue when starting with a clean vagrant box.
I have update my previous post with another small "issue" that could happen.

On a side note, we will then have to create the private key, certificate and csr request in a temporary directory where the icinga2 user can write to (icinga2 cli is being run as the defined user), then move it to the pki dir (at this point puppet is doing it, usually running as root) and then change the permissions.

Or, just change the ownership on the pki dir to the icinga user, i don't see a reason why not doing this though which is much more simple and achieve the same.

@bobapple
Copy link
Contributor

I am not able to reproduce the permission issue. If the directory /etc/icinga2/pki is created by the package, the icinga user has already write permissions.

@erezzarum
Copy link
Author

Which distro are you testing it on?

@bobapple
Copy link
Contributor

Alright, apparently the packages on CentOS/RHEL behave differently from those on Ubuntu/Debian, that's why I couldn't reproduce it.

I think the best would be to set the Icinga user for the pki dir as owner. Can you update this in your branch?

erezzarum pushed a commit to erezzarum/puppet-icinga2 that referenced this issue Jan 31, 2017
@erezzarum
Copy link
Author

Done.

@bobapple
Copy link
Contributor

I merged your commits to my branch. I will add documentation and specs and then close the issue. Thanks for the support @erezzarum

@bobapple bobapple modified the milestones: v1.1.0, v1.0.3 Mar 13, 2017
n00by pushed a commit to n00by/puppet-icinga2 that referenced this issue Apr 26, 2018
refs voxpupuli#218

- Remove invalid byte sequence in UTF-8
- Set correct permissions for pki dir
- Sign the certificate with icinga2 local CA
n00by pushed a commit to n00by/puppet-icinga2 that referenced this issue Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants