Skip to content

[ITL] Added some ignore options for check_ssl_cert#9512

Merged
julianbrost merged 2 commits intoIcinga:masterfrom
fabieins:feature/check_ssl_cert
Oct 7, 2022
Merged

[ITL] Added some ignore options for check_ssl_cert#9512
julianbrost merged 2 commits intoIcinga:masterfrom
fabieins:feature/check_ssl_cert

Conversation

@fabieins
Copy link
Contributor

@fabieins fabieins commented Sep 5, 2022

  • ssl_cert_ignore_ocsp_errors | Optional. Continue if the OCSP status cannot be checked
  • ssl_cert_ignore_ocsp_timeout | Optional. Ignore OCSP result when timeout occurs while checking
  • ssl_cert_ignore_host_cn | Optional. Do not complain if the CN does not match

@cla-bot
Copy link

cla-bot bot commented Sep 5, 2022

Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA).

Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA.

After that, please reply here with a comment and we'll verify.

Contributors that have not signed yet: @fabieins

Details
  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Please contact us if you think this is the case.

  • If you signed the CLA as a corporation, your GitHub username may not have been submitted to us. Please reach out to the responsible person in your organization.

@fabieins
Copy link
Contributor Author

fabieins commented Sep 5, 2022 via email

@bobapple
Copy link
Member

bobapple commented Sep 6, 2022

@cla-bot check

@cla-bot cla-bot bot added the cla/signed label Sep 6, 2022
@fabieins
Copy link
Contributor Author

fabieins commented Oct 5, 2022

How does it go on here?
Do I have to do anything?

Copy link
Member

@julianbrost julianbrost left a comment

Choose a reason for hiding this comment

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

How does it go on here? Do I have to do anything?

Until just now no, just mail backlog and unfortunate timing, sorry about that.

Comment on lines 5790 to 5796
ssl_cert_ignore_expiration | **Optional.** Ignore expiration date.
ssl_cert_ignore_ocsp | **Optional.** Do not check revocation with OCSP.
ssl_cert_ignore_ocsp_errors | **Optional.** Continue if the OCSP status cannot be checked
ssl_cert_ignore_ocsp_timeout | **Optional.** Ignore OCSP result when timeout occurs while checking
ssl_cert_ignore_sct | **Optional.** Do not check for signed certificate timestamps.
ssl_cert_ignore_tls_renegotiation | **Optional.** Do not check for renegotiation.
ssl_cert_ignore_host_cn | **Optional.** Do not complain if the CN does not match
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why the ordering differs between the changes in both files. So far, at least the ignore options seem to be ordered alphabetically, so I'd keep it this way. Also, most (unfortunately not all) descriptions end with a period, so I'd keep most of them consistent and add them here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the web.conf it was a try to keep the order of the output of check_ssl_cert -h version 2.40.0.
The descriptions are also taken from this help output and there is no period.

Copy link
Member

Choose a reason for hiding this comment

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

In the web.conf it was a try to keep the order of the output of check_ssl_cert -h version 2.40.0.

The order in web.conf is fine. I suggest to use the same in 10-icinga-template-library.md.

The descriptions are also taken from this help output and there is no period.

So is the description for --ignore-ocsp which is also consistent with the majority of options in that file, so I opt for adding the period.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like [0ae3625]?

* ssl_cert_ignore_ocsp_errors   | **Optional.** Continue if the OCSP status cannot be checked
* ssl_cert_ignore_ocsp_timeout  | **Optional.** Ignore OCSP result when timeout occurs while checking
* ssl_cert_ignore_host_cn       | **Optional.** Do not complain if the CN does not match
  * fix order
  * add a period at the end of the description.
@fabieins fabieins force-pushed the feature/check_ssl_cert branch from f789024 to 0ae3625 Compare October 6, 2022 19:34
@julianbrost julianbrost added this to the 2.14.0 milestone Oct 7, 2022
@julianbrost julianbrost enabled auto-merge (squash) October 7, 2022 06:53
@julianbrost julianbrost merged commit e615b29 into Icinga:master Oct 7, 2022
@fabieins fabieins deleted the feature/check_ssl_cert branch October 7, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants