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 093655c34e47e2a5156e939604f171e44c68e636..a1505d414f0459d56a93e3be1f0592349e61cd66 100644 (file)
--- 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 d452120094b1910150bf7c86e696fd4c894acc92..20bfb2f288b5c2ef1673ab04ee9029b5b37e0c69 100644 (file)
--- 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 a7cbf9bc3c7739566b42215ee72fff4710632419..52105a6cc367cea56b0308bde91de461f6cc4a86 100755 (executable)
--- 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 84ed04d7081288533469d29ba86b49f18000fad9..e3a1f6c8cedb523932f6a0331b8a542d046f470c 100644 (file)
--- 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 a997fe7017eeadf85ece872618f1cc459bfa227c..7fedbc7aeb161fa4fa4efa486ceb38c1382990fd 100644 (file)
--- 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 7fdc43bec02cc9407690261bb51ba7c03d7eb9f5..b5af8b5fd119f945948294373fa63e96dca34f4a 100644 (file)
--- 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);