Ensure that verify-only encryption modules can never encrypt passwords.

If another module was loaded first and then later unloaded it was
possible for a deprecated module to encrypt passwords.

5 files changed, 67 insertions, 101 deletions

diff --git a/modules/encryption/enc_md5.cpp b/modules/encryption/enc_md5.cpp
index 1582c5224..023e04656 100644
--- a/modules/encryption/enc_md5.cpp
+++ b/modules/encryption/enc_md5.cpp
@@ -61,18 +61,10 @@ public:
 		});
 	}
 
-	EventReturn OnEncrypt(const Anope::string &src, Anope::string &dest) override
-	{
-		auto enc = "md5:" + Anope::Hex(md5provider.Encrypt(src));
-		Log(LOG_DEBUG_2) << "(enc_md5) hashed password from [" << src << "] to [" << enc << "]";
-		dest = enc;
-		return EVENT_ALLOW;
-	}
-
 	void OnCheckAuthentication(User *, IdentifyRequest *req) override
 	{
 		const auto *na = NickAlias::Find(req->GetAccount());
-		if (na == NULL)
+		if (!na)
 			return;
 
 		NickCore *nc = na->nc;
@@ -84,8 +76,7 @@ public:
 		if (!hash_method.equals_cs("md5"))
 			return;
 
-		Anope::string enc;
-		this->OnEncrypt(req->GetPassword(), enc);
+		auto enc = "md5:" + Anope::Hex(md5provider.Encrypt(req->GetPassword()));
 		if (nc->pass.equals_cs(enc))
 		{
 			// If we are NOT the first encryption module we want to re-encrypt
diff --git a/modules/encryption/enc_none.cpp b/modules/encryption/enc_none.cpp
index a47dedb85..253ffd94c 100644
--- a/modules/encryption/enc_none.cpp
+++ b/modules/encryption/enc_none.cpp
@@ -13,44 +13,35 @@ class ENone final
 	: public Module
 {
 public:
-	ENone(const Anope::string &modname, const Anope::string &creator) : Module(modname, creator, ENCRYPTION | VENDOR)
+	ENone(const Anope::string &modname, const Anope::string &creator)
+		: Module(modname, creator, ENCRYPTION | VENDOR)
 	{
 		if (ModuleManager::FindFirstOf(ENCRYPTION) == this)
 			throw ModuleException("enc_none is deprecated and can not be used as a primary encryption method");
 	}
 
-	EventReturn OnEncrypt(const Anope::string &src, Anope::string &dest) override
-	{
-		Anope::string buf = "plain:";
-		Anope::string cpass;
-		Anope::B64Encode(src, cpass);
-		buf += cpass;
-		Log(LOG_DEBUG_2) << "(enc_none) hashed password from [" << src << "] to [" << buf << "]";
-		dest = buf;
-		return EVENT_ALLOW;
-	}
-
 	void OnCheckAuthentication(User *, IdentifyRequest *req) override
 	{
-		const NickAlias *na = NickAlias::Find(req->GetAccount());
-		if (na == NULL)
+		const auto *na = NickAlias::Find(req->GetAccount());
+		if (!na)
 			return;
-		NickCore *nc = na->nc;
 
-		size_t pos = nc->pass.find(':');
+		NickCore *nc = na->nc;
+		auto pos = nc->pass.find(':');
 		if (pos == Anope::string::npos)
 			return;
+
 		Anope::string hash_method(nc->pass.begin(), nc->pass.begin() + pos);
 		if (!hash_method.equals_cs("plain"))
 			return;
 
-		Anope::string buf;
-		this->OnEncrypt(req->GetPassword(), buf);
-		if (nc->pass.equals_cs(buf))
+		Anope::string b64pass;
+		Anope::B64Encode(req->GetPassword(), b64pass);
+		auto enc = "plain:" + b64pass;
+		if (nc->pass.equals_cs(enc))
 		{
-			/* if we are NOT the first module in the list,
-			 * we want to re-encrypt the pass with the new encryption
-			 */
+			// If we are NOT the first encryption module we want to re-encrypt
+			// the password with the primary encryption method.
 			if (ModuleManager::FindFirstOf(ENCRYPTION) != this)
 				Anope::Encrypt(req->GetPassword(), nc->pass);
 			req->Success(this);
diff --git a/modules/encryption/enc_old.cpp b/modules/encryption/enc_old.cpp
index a3b4c98c5..4b6dc438a 100644
--- a/modules/encryption/enc_old.cpp
+++ b/modules/encryption/enc_old.cpp
@@ -18,6 +18,26 @@ class EOld final
 private:
 	ServiceReference<Encryption::Provider> md5;
 
+	Anope::string EncryptInternal(const Anope::string &src)
+	{
+		if (!md5)
+			return {};
+
+		char digest[32];
+		memset(digest, 0, sizeof(digest));
+
+		auto hash = md5->Encrypt(src);
+		if (hash.length() != sizeof(digest))
+			return {}; // Probably a bug?
+		memcpy(digest, hash.data(), hash.length());
+
+		char digest2[16];
+		for (size_t i = 0; i < sizeof(digest); i += 2)
+			digest2[i / 2] = XTOI(digest[i]) << 4 | XTOI(digest[i + 1]);
+
+		return Anope::Hex(digest2, sizeof(digest2));
+	}
+
 	inline static char XTOI(char c)
 	{
 		return c > 9 ? c - 'A' + 10 : c - '0';
@@ -36,32 +56,9 @@ public:
 			throw ModuleException("Unable to find md5 reference");
 	}
 
-	EventReturn OnEncrypt(const Anope::string &src, Anope::string &dest) override
-	{
-		if (!md5)
-			return EVENT_CONTINUE;
-
-		char digest[32];
-		memset(digest, 0, sizeof(digest));
-
-		auto hash = md5->Encrypt(src);
-		if (hash.length() != sizeof(digest))
-			return EVENT_CONTINUE; // Probably a bug?
-		memcpy(digest, hash.data(), hash.length());
-
-		char digest2[16];
-		for (size_t i = 0; i < sizeof(digest); i += 2)
-			digest2[i / 2] = XTOI(digest[i]) << 4 | XTOI(digest[i + 1]);
-
-		auto enc = "oldmd5:" + Anope::Hex(digest2, sizeof(digest2));
-		Log(LOG_DEBUG_2) << "(enc_old) hashed password from [" << src << "] to [" << enc << "]";
-		dest = enc;
-		return EVENT_ALLOW;
-	}
-
 	void OnCheckAuthentication(User *, IdentifyRequest *req) override
 	{
-		const NickAlias *na = NickAlias::Find(req->GetAccount());
+		const auto *na = NickAlias::Find(req->GetAccount());
 		if (!na)
 			return;
 
@@ -74,9 +71,8 @@ public:
 		if (!hash_method.equals_cs("oldmd5"))
 			return;
 
-		Anope::string buf;
-		this->OnEncrypt(req->GetPassword(), buf);
-		if (nc->pass.equals_cs(buf))
+		auto enc = EncryptInternal(req->GetPassword());
+		if (!enc.empty() && nc->pass.equals_cs(enc))
 		{
 			// If we are NOT the first encryption module we want to re-encrypt
 			// the password with the primary encryption method.
diff --git a/modules/encryption/enc_sha1.cpp b/modules/encryption/enc_sha1.cpp
index c603d4d0e..135a1121d 100644
--- a/modules/encryption/enc_sha1.cpp
+++ b/modules/encryption/enc_sha1.cpp
@@ -185,33 +185,26 @@ public:
 		});
 	}
 
-	EventReturn OnEncrypt(const Anope::string &src, Anope::string &dest) override
-	{
-		Anope::string buf = "sha1:" + Anope::Hex(sha1provider.Encrypt(src));
-
-		Log(LOG_DEBUG_2) << "(enc_sha1) hashed password from [" << src << "] to [" << buf << "]";
-		dest = buf;
-		return EVENT_ALLOW;
-	}
-
 	void OnCheckAuthentication(User *, IdentifyRequest *req) override
 	{
-		const NickAlias *na = NickAlias::Find(req->GetAccount());
-		if (na == NULL)
+		const auto *na = NickAlias::Find(req->GetAccount());
+		if (!na)
 			return;
-		NickCore *nc = na->nc;
 
-		size_t pos = nc->pass.find(':');
+		NickCore *nc = na->nc;
+		auto pos = nc->pass.find(':');
 		if (pos == Anope::string::npos)
 			return;
+
 		Anope::string hash_method(nc->pass.begin(), nc->pass.begin() + pos);
 		if (!hash_method.equals_cs("sha1"))
 			return;
 
-		Anope::string buf;
-		this->OnEncrypt(req->GetPassword(), buf);
-		if (nc->pass.equals_cs(buf))
+		auto enc = "sha1:" + Anope::Hex(sha1provider.Encrypt(req->GetPassword()));
+		if (nc->pass.equals_cs(enc))
 		{
+			// If we are NOT the first encryption module we want to re-encrypt
+			// the password with the primary encryption method.
 			if (ModuleManager::FindFirstOf(ENCRYPTION) != this)
 				Anope::Encrypt(req->GetPassword(), nc->pass);
 			req->Success(this);
diff --git a/modules/encryption/enc_sha256.cpp b/modules/encryption/enc_sha256.cpp
index 1377cfcb6..a106c1bc4 100644
--- a/modules/encryption/enc_sha256.cpp
+++ b/modules/encryption/enc_sha256.cpp
@@ -48,16 +48,7 @@ private:
 			PACK32(reinterpret_cast<unsigned char *>(&buf2[i << 2]), &iv[i]);
 	}
 
-public:
-	ESHA256(const Anope::string &modname, const Anope::string &creator)
-		: Module(modname, creator, ENCRYPTION | VENDOR)
-	{
-		use_iv = false;
-		if (ModuleManager::FindFirstOf(ENCRYPTION) == this)
-			throw ModuleException("enc_sha256 is deprecated and can not be used as a primary encryption method");
-	}
-
-	EventReturn OnEncrypt(const Anope::string &src, Anope::string &dest) override
+	Anope::string EncryptInternal(const Anope::string &src)
 	{
 		if (!use_iv)
 			NewRandomIV();
@@ -73,36 +64,40 @@ public:
 		sha256_final(&ctx, digest);
 		Anope::string hash(reinterpret_cast<const char *>(&digest), sizeof(digest));
 
-		std::stringstream buf;
-		buf << "sha256:" << Anope::Hex(hash) << ":" << GetIVString();
-		Log(LOG_DEBUG_2) << "(enc_sha256) hashed password from [" << src << "] to [" << buf.str() << " ]";
-		dest = buf.str();
-		return EVENT_ALLOW;
+		return Anope::Hex(hash) + ":" + GetIVString();
+	}
+
+public:
+	ESHA256(const Anope::string &modname, const Anope::string &creator)
+		: Module(modname, creator, ENCRYPTION | VENDOR)
+	{
+		use_iv = false;
+		if (ModuleManager::FindFirstOf(ENCRYPTION) == this)
+			throw ModuleException("enc_sha256 is deprecated and can not be used as a primary encryption method");
 	}
 
 	void OnCheckAuthentication(User *, IdentifyRequest *req) override
 	{
-		const NickAlias *na = NickAlias::Find(req->GetAccount());
-		if (na == NULL)
+		const auto *na = NickAlias::Find(req->GetAccount());
+		if (!na)
 			return;
-		NickCore *nc = na->nc;
 
-		size_t pos = nc->pass.find(':');
+		NickCore *nc = na->nc;
+		auto pos = nc->pass.find(':');
 		if (pos == Anope::string::npos)
 			return;
+
 		Anope::string hash_method(nc->pass.begin(), nc->pass.begin() + pos);
 		if (!hash_method.equals_cs("sha256"))
 			return;
 
 		GetIVFromPass(nc->pass);
 		use_iv = true;
-		Anope::string buf;
-		this->OnEncrypt(req->GetPassword(), buf);
-		if (nc->pass.equals_cs(buf))
+		auto enc = EncryptInternal(req->GetPassword());
+		if (nc->pass.equals_cs(enc))
 		{
-			/* if we are NOT the first module in the list or we are using a default IV
-			 * we want to re-encrypt the pass with the new encryption
-			 */
+			// If we are NOT the first encryption module we want to re-encrypt
+			// the password with the primary encryption method.
 			if (ModuleManager::FindFirstOf(ENCRYPTION) != this)
 				Anope::Encrypt(req->GetPassword(), nc->pass);
 			req->Success(this);