-
Notifications
You must be signed in to change notification settings - Fork 61
Feat/better logging #1555
Feat/better logging #1555
Changes from all commits
d364271
b64fe4d
b7adc45
c9cf301
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,11 @@ | ||
#include "keymanager.h" | ||
|
||
#include <stdexcept> | ||
#include <utility> | ||
|
||
#include <boost/scoped_array.hpp> | ||
#include <utility> | ||
|
||
#include "crypto/openssl_compat.h" | ||
#include "storage/invstorage.h" | ||
#include "utilities/types.h" | ||
|
||
|
@@ -192,10 +193,74 @@ std::string KeyManager::getCN() const { | |
} | ||
boost::scoped_array<char> buf(new char[len + 1]); | ||
X509_NAME_get_text_by_NID(X509_get_subject_name(x.get()), NID_commonName, buf.get(), len + 1); | ||
std::string cn(buf.get()); | ||
const std::string cn(buf.get()); | ||
return cn; | ||
} | ||
|
||
void KeyManager::getCertInfo(std::string *subject, std::string *issuer, std::string *not_before, | ||
std::string *not_after) const { | ||
std::string not_found_cert_message = "Certificate is not found, can't extract device certificate"; | ||
std::string cert; | ||
if (config_.tls_cert_source == CryptoSource::kFile || config_.tls_cert_source == CryptoSource::kAndroid) { | ||
if (!backend_->loadTlsCert(&cert)) { | ||
throw std::runtime_error(not_found_cert_message); | ||
} | ||
} else { // CryptoSource::kPkcs11 | ||
if (!built_with_p11) { | ||
throw std::runtime_error("Aktualizr was built without PKCS#11 support, can't extract device certificate"); | ||
} | ||
if (!(*p11_)->readTlsCert(&cert)) { | ||
throw std::runtime_error(not_found_cert_message); | ||
} | ||
} | ||
|
||
StructGuard<BIO> bio(BIO_new_mem_buf(const_cast<char *>(cert.c_str()), static_cast<int>(cert.size())), BIO_vfree); | ||
StructGuard<X509> x(PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr), X509_free); | ||
if (x == nullptr) { | ||
throw std::runtime_error("Could not parse certificate"); | ||
} | ||
|
||
StructGuard<BIO> subj_bio(BIO_new(BIO_s_mem()), BIO_vfree); | ||
X509_NAME_print_ex(subj_bio.get(), X509_get_subject_name(x.get()), 1, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docs I've found are not really helpful but are we sure that this call or the next ( Maybe a simple precaution:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was annoyed that the SSL docs don't even mention the return value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit scared of the return value mess so I ended up just taking your advice literally. |
||
char *subj_buf = nullptr; | ||
auto subj_len = BIO_get_mem_data(subj_bio.get(), &subj_buf); // NOLINT(cppcoreguidelines-pro-type-cstyle-cast) | ||
if (subj_buf == nullptr) { | ||
throw std::runtime_error("Could not parse certificate subject"); | ||
} | ||
*subject = std::string(subj_buf, static_cast<size_t>(subj_len)); | ||
|
||
StructGuard<BIO> issuer_bio(BIO_new(BIO_s_mem()), BIO_vfree); | ||
X509_NAME_print_ex(issuer_bio.get(), X509_get_issuer_name(x.get()), 1, 0); | ||
char *issuer_buf = nullptr; | ||
auto issuer_len = BIO_get_mem_data(issuer_bio.get(), &issuer_buf); // NOLINT(cppcoreguidelines-pro-type-cstyle-cast) | ||
if (issuer_buf == nullptr) { | ||
throw std::runtime_error("Could not parse certificate issuer"); | ||
} | ||
*issuer = std::string(issuer_buf, static_cast<size_t>(issuer_len)); | ||
|
||
#if AKTUALIZR_OPENSSL_PRE_11 | ||
const ASN1_TIME *nb_asn1 = X509_get_notBefore(x.get()); | ||
#else | ||
const ASN1_TIME *nb_asn1 = X509_get0_notBefore(x.get()); | ||
#endif | ||
StructGuard<BIO> nb_bio(BIO_new(BIO_s_mem()), BIO_vfree); | ||
ASN1_TIME_print(nb_bio.get(), nb_asn1); | ||
char *nb_buf; | ||
auto nb_len = BIO_get_mem_data(nb_bio.get(), &nb_buf); // NOLINT(cppcoreguidelines-pro-type-cstyle-cast) | ||
*not_before = std::string(nb_buf, static_cast<size_t>(nb_len)); | ||
|
||
#if AKTUALIZR_OPENSSL_PRE_11 | ||
const ASN1_TIME *na_asn1 = X509_get_notAfter(x.get()); | ||
#else | ||
const ASN1_TIME *na_asn1 = X509_get0_notAfter(x.get()); | ||
#endif | ||
StructGuard<BIO> na_bio(BIO_new(BIO_s_mem()), BIO_vfree); | ||
ASN1_TIME_print(na_bio.get(), na_asn1); | ||
char *na_buf; | ||
auto na_len = BIO_get_mem_data(na_bio.get(), &na_buf); // NOLINT(cppcoreguidelines-pro-type-cstyle-cast) | ||
*not_after = std::string(na_buf, static_cast<size_t>(na_len)); | ||
} | ||
|
||
void KeyManager::copyCertsToCurl(HttpInterface &http) { | ||
std::string pkey = getPkey(); | ||
std::string cert = getCert(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mixing metadata/values with message text might not be optimal as you need to search for values between text. The best practice here is to separate a static part of a log message from its variable part. For example,
`
Adding Secondary:
`
but it's not critical here as there are just two variable elements here, it's rather important when there are more of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure I follow. All I did was rephrase it and swap the serial and hardware ID.