From c18ea7899988e70785d25c12653c4750440ad312 Mon Sep 17 00:00:00 2001
From: Yehuda Sadeh <ysadehwe@ibm.com>
Date: Thu, 27 Feb 2025 16:14:06 -0500
Subject: [PATCH] auth/cephx: modify client + server challenges hashing

This applies when using ciphers that are not the original
AES-128 one. Use the hmac-sha256 hash now. With AES256KRB5
the original method of encrypting the combined challenges
doesn't work as the confounder randomizes the result.

Signed-off-by: Yehuda Sadeh <ysadehwe@ibm.com>
(cherry picked from commit 31c07fbbf3b8c911a51b41791d6b6265923acda2)
---
 src/auth/Crypto.cc              |  5 ++++-
 src/auth/Crypto.h               |  2 +-
 src/auth/cephx/CephxProtocol.cc | 17 +++++++++++++++--
 src/auth/cephx/CephxProtocol.h  | 13 +++++++++++++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/auth/Crypto.cc b/src/auth/Crypto.cc
index 1d77ea3fc89..69bd5c6347a 100644
--- a/src/auth/Crypto.cc
+++ b/src/auth/Crypto.cc
@@ -314,6 +314,7 @@ public:
   int encrypt(CephContext *cct, const ceph::bufferlist& in,
 	      ceph::bufferlist& out,
               std::string* /* unused */) const override {
+    ldout(cct, 20) << "CryptoAESKeyHandler::encrypt()" << dendl;
     // we need to take into account the PKCS#7 padding. There *always* will
     // be at least one byte of padding. This stays even to input aligned to
     // AES_BLOCK_LEN. Otherwise we would face ambiguities during decryption.
@@ -356,6 +357,7 @@ public:
   int decrypt(CephContext *cct, const ceph::bufferlist& in,
 	      ceph::bufferlist& out,
 	      std::string* /* unused */) const override {
+    ldout(cct, 20) << "CryptoAESKeyHandler::decrypt()" << dendl;
     // PKCS#7 padding enlarges even empty plain-text to take 16 bytes.
     if (in.length() < AES_BLOCK_LEN || in.length() % AES_BLOCK_LEN) {
       return -1;
@@ -745,6 +747,7 @@ public:
   int encrypt(CephContext *cct, const ceph::bufferlist& in,
 	      ceph::bufferlist& out,
               std::string* /* unused */) const override {
+    ldout(cct, 20) << "CryptoAES256KRB5KeyHandler::encrypt()" << dendl;
     // encrypted (confounder | data) | hash
     ceph::bufferptr out_tmp{static_cast<unsigned>(
       AES256KRB5_BLOCK_LEN + in.length() + AES256KRB5_HASH_LEN)};
@@ -794,6 +797,7 @@ public:
 	      ceph::bufferlist& out,
 	      std::string* /* unused */) const override {
 
+    ldout(cct, 20) << "CryptoAES256KRB5KeyHandler::decrypt()" << dendl;
     if (in.length() < AES256KRB5_BLOCK_LEN + AES256KRB5_HASH_LEN) { /* minimum size: confounder + hmac */
       return -EINVAL;
     }
@@ -861,7 +865,6 @@ public:
 
     return 0;
   }
-
 };
 
 
diff --git a/src/auth/Crypto.h b/src/auth/Crypto.h
index e18b0436826..c7cd917db7b 100644
--- a/src/auth/Crypto.h
+++ b/src/auth/Crypto.h
@@ -187,7 +187,7 @@ public:
     return ckh->encrypt(cct, in, out);
   }
 
-  sha256_digest_t hmac_sha256(CephContext*, const ceph::buffer::list& in) {
+  sha256_digest_t hmac_sha256(CephContext*, const ceph::buffer::list& in) const {
     ceph_assert(ckh);
     return ckh->hmac_sha256(in);
   }
diff --git a/src/auth/cephx/CephxProtocol.cc b/src/auth/cephx/CephxProtocol.cc
index 93504358d32..73d20b2ba93 100644
--- a/src/auth/cephx/CephxProtocol.cc
+++ b/src/auth/cephx/CephxProtocol.cc
@@ -39,8 +39,21 @@ void cephx_calc_client_server_challenge(CephContext *cct, CryptoKey& secret, uin
   b.client_challenge = client_challenge;
 
   bufferlist enc;
-  if (encode_encrypt(cct, b, secret, enc, error))
-    return;
+  switch (secret.get_type()) {
+    case CEPH_CRYPTO_AES:
+      if (encode_encrypt(cct, b, secret, enc, error))
+        return;
+      break;
+    default:
+      /*
+       * AES256KRB5 has a builtin confounder that randomizes the result,
+       * so just encode_encrypt() cannot be used. We should use
+       * a cryptographic has anyway, keeping the old behavior
+       * for AES for backward compatibility.
+       */
+      if (encode_hash(cct, b, secret, enc, error))
+        return;
+  };
 
   uint64_t k = 0;
   const ceph_le64 *p = (const ceph_le64 *)enc.c_str();
diff --git a/src/auth/cephx/CephxProtocol.h b/src/auth/cephx/CephxProtocol.h
index 260cb13ff5a..73c8666e6bb 100644
--- a/src/auth/cephx/CephxProtocol.h
+++ b/src/auth/cephx/CephxProtocol.h
@@ -648,4 +648,17 @@ int encode_encrypt(CephContext *cct, const T& t, const CryptoKey& key,
   return 0;
 }
 
+template <typename T>
+int encode_hash(CephContext *cct, const T& t, const CryptoKey& key,
+                ceph::buffer::list& out, std::string &error)
+{
+  using ceph::encode;
+  ceph::buffer::list bl_enc;
+  /* simple encoding, we don't need to add any magic because this will not be decoded */
+  ::encode(t, bl_enc);
+  sha256_digest_t hash = key.hmac_sha256(cct, bl_enc);
+  out.append((const char *)&hash, sizeof(hash));
+  return 0;
+}
+
 #endif
-- 
2.39.5