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

Doc: Distributed Monitoring: add section "External CA/PKI" #9825

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

Conversation

Al2Klimov
Copy link
Member

The following already works:

  • Custom key sizes, e.g. 2048 bits
  • Custom key types, e.g. ECC
  • Multiple trusted root CAs in /var/lib/icinga2/certs/ca.crt
  • Different root CAs per cluster subtree, as long as each node trusts the issuers of the certificates of all nodes it's directly connected to
  • Any number of intermediate CAs

refs #9798
refs #7323

@Al2Klimov Al2Klimov added enhancement New feature or request area/distributed Distributed monitoring (master, satellites, clients) area/documentation End-user or developer help labels Jul 6, 2023
@cla-bot cla-bot bot added the cla/signed label Jul 6, 2023
@icinga-probot icinga-probot bot added needs-sponsoring Not low on priority but also not scheduled soon without any incentive TBD To be defined - We aren't certain about this yet labels Jul 6, 2023
@Al2Klimov
Copy link
Member Author

FYI @bobapple @widhalmt @tbauriedel

@julianbrost julianbrost removed needs-sponsoring Not low on priority but also not scheduled soon without any incentive TBD To be defined - We aren't certain about this yet labels Jul 6, 2023
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Jul 31, 2023
@Al2Klimov Al2Klimov added the consider backporting Should be considered for inclusion in a bugfix release label Sep 28, 2023
@Al2Klimov Al2Klimov mentioned this pull request Nov 24, 2023
3 tasks
doc/06-distributed-monitoring.md Outdated Show resolved Hide resolved
doc/06-distributed-monitoring.md Outdated Show resolved Hide resolved
doc/06-distributed-monitoring.md Outdated Show resolved Hide resolved
doc/06-distributed-monitoring.md Outdated Show resolved Hide resolved
doc/06-distributed-monitoring.md Outdated Show resolved Hide resolved
Comment on lines 3235 to 3241
Neither the above commands, nor their automatic counterparts in the Icinga
cluster do anything special during certificate issuance. I.e. Icinga
isn't the only possible source of the certificates it uses. E.g.
`openssl req/x509 ...` may be used as well as long as the leaf certificates' CN
and SAN match the endpoint names. Pretty much everything else is limited only by
your imagination and the oldest OpenSSL version of two Icinga nodes connected to
each other. E.g. the following works:
Copy link
Member

Choose a reason for hiding this comment

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

I would express this in a slightly different, more end-user friendly way:

Suggested change
Neither the above commands, nor their automatic counterparts in the Icinga
cluster do anything special during certificate issuance. I.e. Icinga
isn't the only possible source of the certificates it uses. E.g.
`openssl req/x509 ...` may be used as well as long as the leaf certificates' CN
and SAN match the endpoint names. Pretty much everything else is limited only by
your imagination and the oldest OpenSSL version of two Icinga nodes connected to
each other. E.g. the following works:
There is nothing special about the above commands or their automatic counterparts in the Icinga cluster when it comes to certificate issuance. It should be noted that Icinga is not the sole source of the certificates it uses. For instance, openssl `req/x509 ...` can be used as well, as long as the **CN** and **SAN** of the leaf certificates match the endpoint names. Otherwise, the only restrictions are your own imagination and the oldest OpenSSL version of two connected Icinga nodes. You can find a working example below:

Even I wouldn't consider the steps listed below to be a working example of this. IMHO they're just too abstract to offer as a working example for the end user who has no knowledge of how openssl req/x509 works. One shouldn't use external CAs if doesn't know what to do though. So 🤷‍♂️!

Copy link
Member Author

Choose a reason for hiding this comment

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

One shouldn't use external CAs if doesn't know what to do though.

Actually a reason not to make this more user friendly. 😅

Copy link
Member

Choose a reason for hiding this comment

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

I was referring to the examples below and not this opening text. This is still a technical documentation, so you have to comply with certain standards (avoiding the use of i.e or e.g).

Copy link
Member Author

Choose a reason for hiding this comment

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

@julianbrost What's your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, it's a good idea to write down what Icinga 2 expects from certificates and what it issues by itself. However, I think I'd rather place this somewhere into the technical details documentation instead of here and telling users so be "limited only by [their] imagination". It should be more of a "only do this if you know what you're doing and don't expect much help" thing not an invitation to generate special certificates and then be surprised that they weren't (properly) renewed by Icinga 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd even agree with the second half of your comment, but isn't Manual Certificate Creation exactly where users would expect such? Users already can't find something in our docs w/o a search engine!

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually a reason not to make this more user friendly. 😅

exactly where users would expect such

Which of these two now? Currently it's a "be limited only by your imagination and good luck with that" for end users. I'd be happy with not directly presenting it to most standard users but moving it to the more technical documentation that's more for developers and expert users.

If this is supposed to become standard user documentation, this at the very least needs a warning that custom certificates may not be renewed by Icinga 2 and that one has to take care of this externally. By the way, did you test what happens when Icinga 2 would attempt to renew an externally issued certificate that does not closely resemble the ones issued by Icinga 2?

Copy link
Member

Choose a reason for hiding this comment

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

As I was asked to put my two euro cents in, I am generally in favor of this PR, as it both (loosely) describes Icinga 2's own requirements for certificates as well as how to realize a "SSL added and removed here! :-)"-setup required by some corporations.

However, I would advise adding more details and moving it to "12-icinga2-api" or another later section, to not confuse or overload first time users. Furthermore, a hint how to monitor the certificate expiration - link to ITL, check_ssl_cert, for example - would be good, as otherwise people tend to forget those things, even if noted that Icinga 2 itself does not do a thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, I would advise adding more details and moving it to "12-icinga2-api" or another later section, to not confuse or overload first time users.

I'm afraid this naturally belongs to Manual Certificate Creation. That section can be seen as a bit advanced by itself and its parent section is even called Advanced Hints, so... 🤷‍♂️ (Have I already mentioned there's a big warning now in here?)

@Al2Klimov
Copy link
Member Author

FWIW, we've already written this under https://icinga.com/solutions/monitoring-and-security/

Secure Communication

(...)

  • Create a custom Certificate Authority (CA) or use your existing one

@Al2Klimov Al2Klimov mentioned this pull request Nov 14, 2024
2 tasks
@Al2Klimov Al2Klimov assigned oxzi and unassigned Al2Klimov Nov 15, 2024
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

In general, I am happy with this addition to the docs. There are only two points left (next to my inline comment):

  1. Please expand the contracted forms of the words, like "don't" to "do not".
  2. This may be optional, but might help. What about actual commands showing how to use an external CA? At the moment, this section is on a very theoretical level. If you say this theoretical approach is enough, I am fine with this as well.

Comment on lines 3243 to 3249
The only serious reasons to generate own certificates are company policies.
You are responsible for making Icinga working with your certificates,
as well as for [expiry monitoring](10-icinga-template-library.md#plugin-check-command-ssl_cert)
and renewal. Especially the CLI doesn't expect such certificates.
Apropos renewal, don't provide the CA private key in `/var/lib/icinga2/ca`!
Icinga uses it to automatically renew certificates with standard properties,
not any custom ones.
Copy link
Member

Choose a reason for hiding this comment

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

I would split this paragraph as it contains lots of important information about different topics.

Suggested change
The only serious reasons to generate own certificates are company policies.
You are responsible for making Icinga working with your certificates,
as well as for [expiry monitoring](10-icinga-template-library.md#plugin-check-command-ssl_cert)
and renewal. Especially the CLI doesn't expect such certificates.
Apropos renewal, don't provide the CA private key in `/var/lib/icinga2/ca`!
Icinga uses it to automatically renew certificates with standard properties,
not any custom ones.
The only serious reasons to generate own certificates are company policies.
You are responsible for making Icinga working with your certificates,
as well as for [expiry monitoring](10-icinga-template-library.md#plugin-check-command-ssl_cert)
and renewal.
Especially the CLI doesn't expect such certificates.
Apropos renewal, don't provide the CA private key in `/var/lib/icinga2/ca`!
Icinga uses it to automatically renew certificates with standard properties,
not any custom ones.

Then, please clarify what you are meaning when talking about CLI tools. And maybe state that the CA should be within /var/lib/icinga2/certs/ca.crt (as mentioned later on) and not only where it shouldn't be.

@Al2Klimov
Copy link
Member Author

  1. Please expand the contracted forms of the words, like "don't" to "do not".

I could do it, but what for? Also, see e.g: grep -rnFwe "don't" doc

  1. This may be optional, but might help. What about actual commands showing how to use an external CA? At the moment, this section is on a very theoretical level.

And that's perfect. We're not responsible for what we don't provide. :-)

If you say this theoretical approach is enough, I am fine with this as well.

The following already works:

* Custom key sizes, e.g. 2048 bits
* Custom key types, e.g. ECC
* Multiple trusted root CAs in `/var/lib/icinga2/certs/ca.crt`
* Different root CAs per cluster subtree, as long as each node trusts the
  issuers of the certificates of all nodes it's directly connected to
* Any number of intermediate CAs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/distributed Distributed monitoring (master, satellites, clients) area/documentation End-user or developer help cla/signed consider backporting Should be considered for inclusion in a bugfix release enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants