search

matteocorti / check_ssl_cert

A shell script (that can be used as a Nagios/Icinga plugin) to check an SSL/TLS connection.
GNU General Public License v3.0
368 stars 132 forks source link

Give meaningful error message if Subject Alternative Name is empty #321

Closed geotekberlin closed 3 years ago

geotekberlin commented 3 years ago

When a certificate is checked that does not have a Subject Alternative Name entry but is valid otherwise, a confusing and nonsensical error message is shown like:

CRITICAL error: invalid CN ('testserver.geotek.de' does not match 'testserver.geotek.de')

To Reproduce

Check a web server certificate with missing Subject Alternative Name.

Expected behavior

A clear and concise error message should be shown, such as: CRITICAL error: subjectAltName is missing

In addition a new command line switch should be provided to either accept empty subjectAltName extensions or show a warning instead of an error. While the use of subjectAltName is encouraged in RFC 2818 and it is good practice to do so, it is not required yet in a strict sense. Popular tools like Webmin / Virtualmin still create certificates with missing subjectAltName and it should be possible to accept these certificates.

System:

matteocorti commented 3 years ago

Thanks for the report, do you have a public address that I could use for testing?

geotekberlin commented 3 years ago

Hi Matteo,

you may use https://selfsigned.geotek.de with sni. /usr/lib/nagios/plugins/check_ssl_cert --sni selfsigned.geotek.de -H selfsigned.geotek.de -P https -c 10 -n selfsigned.geotek.de -p 443 -s -t 15 -w 20 returns the error message: SSL_CERT CRITICAL selfsigned.geotek.de: invalid CN ('selfsigned.geotek.de' does not match 'selfsigned.geotek.de')|days_chain_elem1=2929;20;10;;

And thanks for your wonderful and well documented plugin, I really appreciate your work!!! Martin

------ Originalnachricht ------ Von: "Matteo Corti" @.> An: "matteocorti/check_ssl_cert" @.> Cc: "geotekberlin" @.>; "Author" @.> Gesendet: 05.10.2021 16:23:02 Betreff: Re: [matteocorti/check_ssl_cert] Give meaningful error message if Subject Alternative Name is empty (#321)

Thanks for the report, do you have a public address that I could use for testing?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matteocorti/check_ssl_cert/issues/321#issuecomment-934460278, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVBNQQKTYA6II4LTHMVBQDUFMC4NANCNFSM5FL3DCVA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

matteocorti commented 3 years ago

I'll commit a fix soon. May I include the site in the unit tests?

geotekberlin commented 3 years ago

Hi Matteo, I have no objection to include it into your unit tests, but the site will automatically be upgraded to the newest webmin releases which means that if one of the upcoming webmin versions is able to generate self signed certificates with subjectAltName set, it might not be useful anymore for the unit test. In any case the URL https://selfsigned.geotek.de will be permanently available for checking self signed certfificates and you may use it as you wish. Best Regards, Martin

------ Originalnachricht ------ Von: "Matteo Corti" @.> An: "matteocorti/check_ssl_cert" @.> Cc: "geotekberlin" @.>; "Author" @.> Gesendet: 06.10.2021 10:44:44 Betreff: Re: [matteocorti/check_ssl_cert] Give meaningful error message if Subject Alternative Name is empty (#321)

I'll commit a fix soon. May I include the site in the unit tests?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matteocorti/check_ssl_cert/issues/321#issuecomment-935765376, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVBNQT73HRAESWBIRHNSRTUFQD7ZANCNFSM5FL3DCVA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

matteocorti commented 3 years ago

Thanks

lukastribus commented 3 years ago

I'm not sure if accepting certificates without SAN is a good idea.

At least certificates from public CA's without SAN are not accepted by browsers, so I think check_ssl_cert should not either ...

https://bugs.chromium.org/p/chromium/issues/detail?id=308330 https://bugzilla.mozilla.org/show_bug.cgi?id=1245280

I agree with @geotekberlin an error message indicating missing SAN would be best.

matteocorti commented 3 years ago

I see your point. But in Nagios the only way to point out something is a different status.

There are two options:

What do you think?

geotekberlin commented 3 years ago

In my opinion the best solution would be to make the test mandatory (and showing a meaningful error message what exactly is wrong) and include an option to ignore an empty subjectAltName field

------ Originalnachricht ------ Von: "Matteo Corti" @.> An: "matteocorti/check_ssl_cert" @.> Cc: "geotekberlin" @.>; "Mention" @.> Gesendet: 06.10.2021 13:58:12 Betreff: Re: [matteocorti/check_ssl_cert] Give meaningful error message if Subject Alternative Name is empty (#321)

I see your point. But in Nagios the only way to point out something is a different status.

There are two options:

make it mandatory and add an option to ignore the missing one add an option to make it mandatory (and include the test with --all) What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/matteocorti/check_ssl_cert/issues/321#issuecomment-936096506, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVBNQWVRSGCM2YCYFA63QDUFQ2VJANCNFSM5FL3DCVA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

matteocorti commented 3 years ago

I just pushed a new version where the SANs are mandatory and --allow-empty-san skips the check

lukastribus commented 3 years ago

I agree, by default we should be as strict as a browser, since the expectation is that this check will fail before users complain. Thanks.

matteocorti commented 3 years ago

I noticed now that there was an option --require-san which is now enabled by default and deprecated (as does nothing)