Add option to disable password authentication when certauth is used #770

New Issue

2026-10-05T04:43:36+02:00

adam commented

2026-10-05 04:43:36 +02:00

Per discussions with @leonthemisfit @linuxdaemon @A-UNDERSCORE-D and @ThatOneRoadie, similar to SSH's PasswordAuthentication no setting (and similar services), the option to disable password-based authentication and allow only certification / public key login withcertauth would increase security of the ZNC instance and provide for greater control over allowed authentication mechanisms.

Per discussions with @leonthemisfit @linuxdaemon @A-UNDERSCORE-D and @ThatOneRoadie, similar to SSH's `PasswordAuthentication no` setting (and similar services), the option to disable password-based authentication and allow only certification / public key login with`certauth` would increase security of the ZNC instance and provide for greater control over allowed authentication mechanisms.

adam closed this issue

2026-10-05 04:43:36 +02:00

adam commented

2026-10-05 04:43:37 +02:00

Untested patch that anyone else is free to pick up, test, improve and get to work (Is this what you want?):

diff --git a/modules/certauth.cpp b/modules/certauth.cpp
index e668540..f1499a2 100644
--- a/modules/certauth.cpp
+++ b/modules/certauth.cpp
@@ -109,26 +109,27 @@ class CSSLClientCertMod : public CModule {
         Csock* pSock = Auth->GetSocket();
         CUser* pUser = CZNC::Get().FindUser(sUser);
 
-        if (pSock == nullptr || pUser == nullptr) return CONTINUE;
+        if (pSock == nullptr || pUser == nullptr)
+            return Refuse(Auth, "No such user"); // FIXME: It is a *bad* idea if attackers can tell the difference between "no such user" and "wrong password"
 
         const CString sPubKey = GetKey(pSock);
         DEBUG("User: " << sUser << " Key: " << sPubKey);
 
         if (sPubKey.empty()) {
             DEBUG("Peer got no public key, ignoring");
-            return CONTINUE;
+            return Refuse(Auth, "No SSL certificate was provided");
         }
 
         MSCString::const_iterator it = m_PubKeys.find(sUser);
         if (it == m_PubKeys.end()) {
             DEBUG("No saved pubkeys for this client");
-            return CONTINUE;
+            return Refuse(Auth, "Really, this error message gives away too much information again");
         }
 
         SCString::const_iterator it2 = it->second.find(sPubKey);
         if (it2 == it->second.end()) {
             DEBUG("Invalid pubkey");
-            return CONTINUE;
+            return Refuse(Auth, "Your SSL certificate was not accepted");
         }
 
         // This client uses a valid pubkey for this user, let them in
@@ -138,6 +139,15 @@ class CSSLClientCertMod : public CModule {
         return HALT;
     }
 
+    EModRet Refuse(std::shared_ptr<CAuthBase> Auth, const CString& sReason) {
+        bool onlyAllowPasswordLogins = true; // TODO: Turn this into a setting
+        if (onlyAllowPasswordLogins) {
+            Auth->RefuseLogin(sReason);
+            return HALT;
+        }
+        return CONTINUE;
+    }
+
     void HandleShowCommand(const CString& sLine) {
         const CString sPubKey = GetKey(GetClient());

This patch should make any failure to authenticate with a certificate become fatal, instead of falling back to checking the password.

Untested patch that anyone else is free to pick up, test, improve and get to work (Is this what you want?): ```diff diff --git a/modules/certauth.cpp b/modules/certauth.cpp index e668540..f1499a2 100644 --- a/modules/certauth.cpp +++ b/modules/certauth.cpp @@ -109,26 +109,27 @@ class CSSLClientCertMod : public CModule { Csock* pSock = Auth->GetSocket(); CUser* pUser = CZNC::Get().FindUser(sUser); - if (pSock == nullptr || pUser == nullptr) return CONTINUE; + if (pSock == nullptr || pUser == nullptr) + return Refuse(Auth, "No such user"); // FIXME: It is a *bad* idea if attackers can tell the difference between "no such user" and "wrong password" const CString sPubKey = GetKey(pSock); DEBUG("User: " << sUser << " Key: " << sPubKey); if (sPubKey.empty()) { DEBUG("Peer got no public key, ignoring"); - return CONTINUE; + return Refuse(Auth, "No SSL certificate was provided"); } MSCString::const_iterator it = m_PubKeys.find(sUser); if (it == m_PubKeys.end()) { DEBUG("No saved pubkeys for this client"); - return CONTINUE; + return Refuse(Auth, "Really, this error message gives away too much information again"); } SCString::const_iterator it2 = it->second.find(sPubKey); if (it2 == it->second.end()) { DEBUG("Invalid pubkey"); - return CONTINUE; + return Refuse(Auth, "Your SSL certificate was not accepted"); } // This client uses a valid pubkey for this user, let them in @@ -138,6 +139,15 @@ class CSSLClientCertMod : public CModule { return HALT; } + EModRet Refuse(std::shared_ptr<CAuthBase> Auth, const CString& sReason) { + bool onlyAllowPasswordLogins = true; // TODO: Turn this into a setting + if (onlyAllowPasswordLogins) { + Auth->RefuseLogin(sReason); + return HALT; + } + return CONTINUE; + } + void HandleShowCommand(const CString& sLine) { const CString sPubKey = GetKey(GetClient()); ``` This patch should make any failure to authenticate with a certificate become fatal, instead of falling back to checking the password.

adam commented

2026-10-05 04:43:38 +02:00

Currently this behavior is supported with a workaround: replace the salted password hash with something which is not a valid hash.
E.g. 1234 is not SHA-256 of any string, because it has wrong length, or zz, because such characters can't exist in hex representation of the hash.

Currently this behavior is supported with a workaround: replace the salted password hash with something which is not a valid hash. E.g. `1234` is not SHA-256 of any string, because it has wrong length, or `zz`, because such characters can't exist in hex representation of the hash.

adam commented

2026-10-05 04:43:38 +02:00

Oh, and this is duplicate of #1173

Oh, and this is duplicate of #1173

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: adam/znc#770