From d188d5e21314773ef1167facd0cb1d9681ef20ad Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sat, 1 Nov 2025 11:23:16 +0000 Subject: Improve the error logging when opering up with a TLS fingerprint. --- src/modules/m_sslinfo.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/modules/m_sslinfo.cpp b/src/modules/m_sslinfo.cpp index bebd08155..428083fc1 100644 --- a/src/modules/m_sslinfo.cpp +++ b/src/modules/m_sslinfo.cpp @@ -414,13 +414,26 @@ public: return MOD_RES_DENY; } - const std::string fingerprint = oper->GetConfig()->getString("fingerprint"); - if (!fingerprint.empty() && (!cert || !cert->IsUsable() || !MatchFingerprint(cert, fingerprint))) + const auto fingerprint = oper->GetConfig()->getString("fingerprint"); + if (fingerprint.empty()) + return MOD_RES_PASSTHRU; + + const auto cert_usable = cert && cert->IsUsable(); + const auto correct_fp = cert_usable && MatchFingerprint(cert, fingerprint); + if (!correct_fp) { if (!automatic) { - ServerInstance->SNO.WriteGlobalSno('o', "{} ({}) [{}] failed to log into the \x02{}\x02 oper account because they are not using the correct TLS client certificate.", - user->nick, user->GetRealUserHost(), user->GetAddress(), oper->GetName()); + const char* error; + if (!cert) + error = "not using a TLS client certificate"; + if (!cert_usable) + error = "using an invalid (probably expired) TLS client certificate"; + else + error = "not using the correct TLS client certificate"; + + ServerInstance->SNO.WriteGlobalSno('o', "{} ({}) [{}] failed to log into the \x02{}\x02 oper account because they are {}.", + user->nick, user->GetRealUserHost(), user->GetAddress(), oper->GetName(), error); } return MOD_RES_DENY; } -- cgit v1.3.1-10-gc9f91 From bfe493af8aef1b5c37c328e27af0f57925d864aa Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sat, 1 Nov 2025 12:28:27 +0000 Subject: Expose the ssl_cert flags in SSLINFO. --- src/modules/m_sslinfo.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/modules/m_sslinfo.cpp b/src/modules/m_sslinfo.cpp index 428083fc1..7d00fe261 100644 --- a/src/modules/m_sslinfo.cpp +++ b/src/modules/m_sslinfo.cpp @@ -65,6 +65,17 @@ public: Delete(user, UnsetRaw(user)); } + static std::string GetFlags(const ssl_cert* cert) + { + std::string ret; + ret.push_back(cert->IsInvalid() ? 'v' : 'V'); + ret.push_back(cert->IsTrusted() ? 'T' : 't'); + ret.push_back(cert->IsRevoked() ? 'R' : 'r'); + ret.push_back(cert->IsUnknownSigner() ? 's' : 'S'); + ret.push_back(cert->GetError().empty() ? 'e' : 'E'); + return ret; + } + std::string ToInternal(const Extensible* container, void* item) const noexcept override { return ToNetwork(container, item); @@ -74,13 +85,7 @@ public: { const ssl_cert* cert = static_cast(item); std::stringstream value; - value - << (cert->IsInvalid() ? "v" : "V") - << (cert->IsTrusted() ? "T" : "t") - << (cert->IsRevoked() ? "R" : "r") - << (cert->IsUnknownSigner() ? "s" : "S") - << (cert->GetError().empty() ? "e" : "E") - << " "; + value << GetFlags(cert) << " "; if (cert->GetError().empty()) value << insp::join(cert->GetFingerprints(), ',') << " " << cert->GetDN() << " " << cert->GetIssuer(); @@ -218,6 +223,7 @@ private: } else { + source->WriteNotice("*** Flags: " + SSLCertExt::GetFlags(cert)); source->WriteNotice("*** Distinguished Name: " + cert->GetDN()); source->WriteNotice("*** Issuer: " + cert->GetIssuer()); for (const auto& fingerprint : cert->GetFingerprints()) -- cgit v1.3.1-10-gc9f91 From 95e08bc907eebbe0cc363b110eacf0cfe07eb5c9 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sat, 1 Nov 2025 14:40:49 +0000 Subject: Remove a long-obsolete workaround from ssl_openssl. --- src/modules/extra/m_ssl_openssl.cpp | 9 --------- 1 file changed, 9 deletions(-) (limited to 'src') diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index 36dac31c4..67c334efd 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -501,14 +501,6 @@ namespace OpenSSL return 1; } - static int destroy(BIO* bio) - { - // XXX: Dummy function to avoid a memory leak in OpenSSL. - // The memory leak happens in BIO_free() (bio_lib.c) when the destroy func of the BIO is NULL. - // This is fixed in OpenSSL but some distros still ship the unpatched version hence we provide this workaround. - return 1; - } - static long ctrl(BIO* bio, int cmd, long num, void* ptr) { if (cmd == BIO_CTRL_FLUSH) @@ -528,7 +520,6 @@ namespace OpenSSL BIO_meth_set_read(meth, OpenSSL::BIOMethod::read); BIO_meth_set_ctrl(meth, OpenSSL::BIOMethod::ctrl); BIO_meth_set_create(meth, OpenSSL::BIOMethod::create); - BIO_meth_set_destroy(meth, OpenSSL::BIOMethod::destroy); return meth; } } -- cgit v1.3.1-10-gc9f91 From a3bd16f3f3c5ff5f5669fc247221ae883ae6e213 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sun, 2 Nov 2025 18:31:07 +0000 Subject: Allow not setting the CA file in ssl_openssl. This matches the behaviour of ssl_gnutls. --- src/configreader.cpp | 2 +- src/modules/extra/m_ssl_openssl.cpp | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/configreader.cpp b/src/configreader.cpp index ac6cd7953..b55ebf86c 100644 --- a/src/configreader.cpp +++ b/src/configreader.cpp @@ -68,7 +68,7 @@ ServerConfig::ServerPaths::ServerPaths(const std::shared_ptr& tag) std::string ServerConfig::ServerPaths::ExpandPath(const std::string& base, const std::string& fragment) { // The fragment is an absolute path, don't modify it. - if (std::filesystem::path(fragment).is_absolute()) + if (fragment.empty() || std::filesystem::path(fragment).is_absolute()) return fragment; if (!fragment.compare(0, 2, "~/", 2)) diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index 67c334efd..bbdb426f5 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -467,11 +467,14 @@ namespace OpenSSL } // Load the CAs we trust - filename = ServerInstance->Config->Paths.PrependConfig(tag->getString("cafile", "ca.pem", 1)); - if ((!ctx.SetCA(filename)) || (!clientctx.SetCA(filename))) + filename = ServerInstance->Config->Paths.PrependConfig(tag->getString("cafile", "ca.pem")); + if (!filename.empty()) { - ERR_print_errors_cb(error_callback, this); - ServerInstance->Logs.Normal(MODNAME, "Can't read CA list from {}. This is only a problem if you want to verify client certificates, otherwise it's safe to ignore this message. Error: {}", filename, lasterr); + if (!ctx.SetCA(filename) || !clientctx.SetCA(filename)) + { + ERR_print_errors_cb(error_callback, this); + ServerInstance->Logs.Normal(MODNAME, "Can't read CA list from {}. This is only a problem if you want to verify client certificates, otherwise it's safe to ignore this message. Error: {}", filename, lasterr); + } } // Load the CRLs. -- cgit v1.3.1-10-gc9f91 From 92a2e8bd65ed9368ee374920cf68ff4c99402910 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sun, 2 Nov 2025 18:53:43 +0000 Subject: Fix an if that should be an else if in sslinfo. --- src/modules/m_sslinfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/modules/m_sslinfo.cpp b/src/modules/m_sslinfo.cpp index 7d00fe261..d8d631d8f 100644 --- a/src/modules/m_sslinfo.cpp +++ b/src/modules/m_sslinfo.cpp @@ -433,7 +433,7 @@ public: const char* error; if (!cert) error = "not using a TLS client certificate"; - if (!cert_usable) + else if (!cert_usable) error = "using an invalid (probably expired) TLS client certificate"; else error = "not using the correct TLS client certificate"; -- cgit v1.3.1-10-gc9f91 From b52b65ca0c55bd3f1f2b1452d830d38700146ce9 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Sun, 2 Nov 2025 20:45:34 +0000 Subject: Improve the way client certificates are validated in ssl_openssl. --- src/modules/extra/m_ssl_openssl.cpp | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index bbdb426f5..5a85f3586 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -62,7 +62,6 @@ # define INSPIRCD_OPENSSL_AUTO_DH #endif -static bool SelfSigned = false; static int exdataindex; static Module* thismod; @@ -537,10 +536,6 @@ static int OnVerify(int preverify_ok, X509_STORE_CTX* ctx) * we can just return preverify_ok here, and openssl * will boot off self-signed and invalid peer certs. */ - int ve = X509_STORE_CTX_get_error(ctx); - - SelfSigned = (ve == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT); - return 1; } @@ -624,18 +619,36 @@ private: return; } - certinfo->invalid = (SSL_get_verify_result(sess) != X509_V_OK); + const auto verify = SSL_get_verify_result(sess); - if (!SelfSigned) + auto selfsigned = false; + switch (verify) { - certinfo->unknownsigner = false; - certinfo->trusted = true; + case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN: + case X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE: + case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT: + selfsigned = true; + [[fallthrough]]; + + case X509_V_OK: + certinfo->invalid = false; + break; + + default: + certinfo->invalid = true; + break; } - else + + if (selfsigned) { certinfo->unknownsigner = true; certinfo->trusted = false; } + else + { + certinfo->unknownsigner = false; + certinfo->trusted = true; + } GetDNString(X509_get_subject_name(cert), certinfo->dn); GetDNString(X509_get_issuer_name(cert), certinfo->issuer); -- cgit v1.3.1-10-gc9f91