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

JSRPC-PKI: Logging incorrect error messages #8682

Closed
yhabteab opened this issue Mar 15, 2021 · 2 comments · Fixed by #8965
Closed

JSRPC-PKI: Logging incorrect error messages #8682

yhabteab opened this issue Mar 15, 2021 · 2 comments · Fixed by #8965
Assignees
Labels
area/distributed Distributed monitoring (master, satellites, clients) area/log Logging related bug Something isn't working
Milestone

Comments

@yhabteab
Copy link
Member

yhabteab commented Mar 15, 2021

Describe the bug

When I was messing around with a CRL today, I noticed that icinga logs an incorrect error message namely, even though the certificate was issued by the master, in the logg message states it is not signed by the master.

Master:

[2021-03-15 21:38:43 +0100] information/ApiListener: New client connection for identity 'satellite' from [::ffff:127.0.0.1]:50309 (certificate validation failed: code 23: certificate revoked)
[2021-03-15 21:38:43 +0100] information/JsonRpcConnection: Received certificate request for CN 'satellite' not signed by our CA: certificate revoked (code 23)
[2021-03-15 21:38:43 +0100] information/JsonRpcConnection: Certificate request for CN 'satellite' is pending. Waiting for approval.

I think it is due to the fact that every single exception thrown here is passed off as not being signed by our CA and therefore you might have to verify against the thrown exceptions to log more accurate error messages.

try {
signedByCA = VerifyCertificate(cacert, cert, listener->GetCrlPath());
if (!signedByCA) {
logmsg << " not";
}
logmsg << " signed by our CA.";
} catch (const std::exception &ex) {
logmsg << " not signed by our CA";
if (const unsigned long *openssl_code = boost::get_error_info<errinfo_openssl_error>(ex)) {
logmsg << ": " << X509_verify_cert_error_string(long(*openssl_code)) << " (code " << *openssl_code << ")";
} else {
logmsg << ".";
}
}

@yhabteab yhabteab changed the title Icinga is logging wrong error messages Icinga is logging incorrect error messages Mar 15, 2021
@yhabteab yhabteab changed the title Icinga is logging incorrect error messages JSRPC-PKI: Logging incorrect error messages Mar 19, 2021
@Al2Klimov
Copy link
Member

Do you think s/not signed by our CA/not valid/ would be sufficient?

@yhabteab
Copy link
Member Author

Isn't it possible to log an exact error message based on the raised error code? e.g. with a revoked certificate, you can log as certificate no longer trusted but generally not valid is better than not signed by our CA when the certificate is actually signed by the master's CA.

@Al2Klimov Al2Klimov added area/log Logging related bug Something isn't working labels May 25, 2021
@yhabteab yhabteab assigned yhabteab and unassigned yhabteab May 26, 2021
@Al2Klimov Al2Klimov added the area/distributed Distributed monitoring (master, satellites, clients) label Oct 12, 2021
@Al2Klimov Al2Klimov added this to the 2.14.0 milestone Dec 3, 2021
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/log Logging related bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants