From: Ilya Dryomov <idryomov@gmail.com>
Date: Mon, 14 Nov 2022 12:24:00 +0000 (+0100)
Subject: rbd, rbd-nbd: don't strip trailing newline in passphrase files
X-Git-Tag: v18.1.0~754^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d642f7804bb74c2a2a6763e3de3b10433f076cd3;p=ceph.git

rbd, rbd-nbd: don't strip trailing newline in passphrase files

One of the stated goals is compatibility with standard LUKS tools,
in particular being able to load encryption on images formatted with
cryptsetup.  cryptsetup doesn't do this and this really interferes
with randomly generated (binary) passphrases.

While at it, open passphrase files as binary -- it communicates the
intent if nothing else on POSIX.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/PendingReleaseNotes b/PendingReleaseNotes
index 093655c34e47..a1505d414f04 100644
--- a/PendingReleaseNotes
+++ b/PendingReleaseNotes
@@ -85,7 +85,9 @@
   notifications needs to pull them (instead of the notifications be pushed 
   to it), an external message bus (e.g. rabbitmq, Kafka) should be used for 
   that purpose.
-
+* RBD: Trailing newline in passphrase files (`<passphrase-file>` argument in
+  `rbd encryption format` command and `--encryption-passphrase-file` option
+  in other commands) is no longer stripped.
 
 >=17.2.1
 
diff --git a/doc/rbd/rbd-encryption.rst b/doc/rbd/rbd-encryption.rst
index d452120094b1..20bfb2f288b5 100644
--- a/doc/rbd/rbd-encryption.rst
+++ b/doc/rbd/rbd-encryption.rst
@@ -112,8 +112,8 @@ randomly-generated encryption key, and is protected by the passphrase read from
 `passphrase-file`.
 
 .. note::
-   If the content of `passphrase-file` ends with a newline character, it will
-   be stripped off.
+   In older versions, if the content of `passphrase-file` ended with a newline
+   character, it was stripped off.
 
 By default, AES-256 in xts-plain64 mode (which is the current recommended mode,
 and the usual default for other tools) will be used. The format operation
diff --git a/qa/workunits/rbd/luks-encryption.sh b/qa/workunits/rbd/luks-encryption.sh
index a7cbf9bc3c77..52105a6cc367 100755
--- a/qa/workunits/rbd/luks-encryption.sh
+++ b/qa/workunits/rbd/luks-encryption.sh
@@ -184,8 +184,8 @@ dd if=/dev/urandom of=/tmp/testdata1 bs=4M count=4
 dd if=/dev/urandom of=/tmp/testdata2 bs=4M count=4
 
 # create passphrase files
-echo -n "password" > /tmp/passphrase
-echo -n "password2" > /tmp/passphrase2
+printf "pass\0word\n" > /tmp/passphrase
+printf "\t password2   " > /tmp/passphrase2
 
 # create an image
 rbd create testimg --size=32M
diff --git a/src/tools/rbd/Utils.cc b/src/tools/rbd/Utils.cc
index 84ed04d70812..e3a1f6c8cedb 100644
--- a/src/tools/rbd/Utils.cc
+++ b/src/tools/rbd/Utils.cc
@@ -753,7 +753,7 @@ int get_encryption_options(const boost::program_options::variables_map &vm,
   auto& specs = opts->specs;
   specs.resize(spec_count);
   for (size_t i = 0; i < spec_count; ++i) {
-    std::ifstream file(passphrase_files[i].c_str());
+    std::ifstream file(passphrase_files[i], std::ios::in | std::ios::binary);
     auto sg = make_scope_guard([&] { file.close(); });
 
     specs[i].format = formats[i];
@@ -782,11 +782,6 @@ int get_encryption_options(const boost::program_options::variables_map &vm,
                 << std::endl;
       return -errno;
     }
-
-    if (!passphrase->empty() &&
-        (*passphrase)[passphrase->length() - 1] == '\n') {
-      passphrase->erase(passphrase->length() - 1);
-    }
   }
 
   return 0;
diff --git a/src/tools/rbd/action/Encryption.cc b/src/tools/rbd/action/Encryption.cc
index a997fe7017ee..7fedbc7aeb16 100644
--- a/src/tools/rbd/action/Encryption.cc
+++ b/src/tools/rbd/action/Encryption.cc
@@ -58,7 +58,7 @@ int execute(const po::variables_map &vm,
     return -EINVAL;
   }
 
-  std::ifstream file(passphrase_file.c_str());
+  std::ifstream file(passphrase_file, std::ios::in | std::ios::binary);
   if (file.fail()) {
     std::cerr << "rbd: unable to open passphrase file " << passphrase_file
               << ": " << cpp_strerror(errno) << std::endl;
@@ -69,9 +69,6 @@ int execute(const po::variables_map &vm,
   auto sg = make_scope_guard([&] {
       ceph_memzero_s(&passphrase[0], passphrase.size(), passphrase.size()); });
   file.close();
-  if (!passphrase.empty() && passphrase[passphrase.length() - 1] == '\n') {
-    passphrase.erase(passphrase.length() - 1);
-  }
 
   auto alg = RBD_ENCRYPTION_ALGORITHM_AES256;
   if (vm.count("cipher-alg")) {
diff --git a/src/tools/rbd_nbd/rbd-nbd.cc b/src/tools/rbd_nbd/rbd-nbd.cc
index 7fdc43bec02c..b5af8b5fd119 100644
--- a/src/tools/rbd_nbd/rbd-nbd.cc
+++ b/src/tools/rbd_nbd/rbd-nbd.cc
@@ -1693,7 +1693,8 @@ static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
     });
 
     for (size_t i = 0; i < encryption_format_count; ++i) {
-      std::ifstream file(cfg->encryption_passphrase_file[i].c_str());
+      std::ifstream file(cfg->encryption_passphrase_file[i],
+                         std::ios::in | std::ios::binary);
       auto sg2 = make_scope_guard([&] { file.close(); });
 
       specs[i].format = cfg->encryption_format[i];
@@ -1723,11 +1724,6 @@ static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
                   << cpp_strerror(errno) << std::endl;
         goto close_fd;
       }
-
-      if (!passphrase->empty() &&
-          (*passphrase)[passphrase->length() - 1] == '\n') {
-        passphrase->erase(passphrase->length() - 1);
-      }
     }
 
     r = image.encryption_load2(&specs[0], encryption_format_count);