[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-0' - loolwsd/ChildProcessSession.cpp loolwsd/LOOLWSD.cpp loolwsd/Util.cpp

[Ashod Nakashian]

 loolwsd/ChildProcessSession.cpp |    2 -
 loolwsd/LOOLWSD.cpp             |    7 +++++
 loolwsd/Util.cpp                |   47 +++++++++++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 12 deletions(-)

New commits:
commit 2db2da3bb2f7f066631fb28eb15a61e2d1d51464
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Mon May 30 20:42:44 2016 -0400

    loolwsd: more secure random directories
    
    Util::createRandomDirectory now uses /dev/urandom
    (and a complex pseudo-random generator where missing)
    to generate 64-byte long, Base64-encoded, names.
    
    This should provide ample security compared to 64-bit
    pseudo-random numbers hex-encoded, as was the case.
    
    Reviewed-on: https://gerrit.libreoffice.org/25696
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit 8f3dcbcfb68393e3768f570c13598c3edf575dc3)
    
    Change-Id: I714810a9fb03b5dcdbad7a15305940bf7457149e
    Reviewed-on: https://gerrit.libreoffice.org/25697
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp
index bf3b11b..0d07b44 100644
--- a/loolwsd/ChildProcessSession.cpp
+++ b/loolwsd/ChildProcessSession.cpp
@@ -786,12 +786,12 @@ bool ChildProcessSession::downloadAs(const char* /*buffer*/, int /*length*/, Str
         }
     }
 
+    // The file is removed upon downloading.
     const auto tmpDir = Util::createRandomDir(JAILED_DOCUMENT_ROOT);
     const auto url = JAILED_DOCUMENT_ROOT + tmpDir + "/" + name;
 
     std::unique_lock<std::recursive_mutex> lock(Mutex);
 
-    //TODO: Cleanup the file after downloading.
     _loKitDocument->pClass->saveAs(_loKitDocument, url.c_str(),
             format.size() == 0 ? nullptr :format.c_str(),
             filterOptions.size() == 0 ? nullptr : filterOptions.c_str());
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 404f576..65d5c3d 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -415,7 +415,8 @@ private:
                     if (!resultURL.getPath().empty())
                     {
                         const std::string mimeType = "application/octet-stream";
-                        response.sendFile(resultURL.getPath(), mimeType);
+                        URI::encode(resultURL.getPath(), "", encodedTo);
+                        response.sendFile(encodedTo, mimeType);
                         sent = true;
                     }
 
@@ -495,6 +496,10 @@ private:
                 Util::removeFile(dirPath, true);
                 return true;
             }
+            else
+            {
+                Log::error("Download file [" + filePath + "] not found.");
+            }
         }
 
         throw BadRequestException("Invalid or unknown request.");
diff --git a/loolwsd/Util.cpp b/loolwsd/Util.cpp
index a814ae2..cad72a1 100644
--- a/loolwsd/Util.cpp
+++ b/loolwsd/Util.cpp
@@ -29,14 +29,16 @@
 
 #include <png.h>
 
+#include <Poco/Base64Encoder.h>
 #include <Poco/ConsoleChannel.h>
 #include <Poco/Exception.h>
 #include <Poco/Format.h>
 #include <Poco/Net/WebSocket.h>
 #include <Poco/Process.h>
+#include <Poco/RandomStream.h>
+#include <Poco/TemporaryFile.h>
 #include <Poco/Thread.h>
 #include <Poco/Timestamp.h>
-#include <Poco/TemporaryFile.h>
 #include <Poco/Util/Application.h>
 
 #include "Common.hpp"
@@ -72,6 +74,7 @@ namespace rng
 {
     static std::random_device _rd;
     static std::mutex _rngMutex;
+    static Poco::RandomBuf _randBuf;
 
     // Create the prng with a random-device for seed.
     // If we don't have a hardware random-device, we will get the same seed.
@@ -93,6 +96,35 @@ namespace rng
         std::unique_lock<std::mutex> lock(_rngMutex);
         return _rng();
     }
+
+    std::vector<char> getBytes(const size_t length)
+    {
+        std::vector<char> v(length);
+        _randBuf.readFromDevice(v.data(), v.size());
+        return v;
+    }
+
+    /// Generates a random string in Base64.
+    /// Note: May contain '/' characters.
+    std::string getB64String(const size_t length)
+    {
+        std::stringstream ss;
+        Poco::Base64Encoder b64(ss);
+        b64 << getBytes(length).data();
+        return ss.str().substr(0, length);
+    }
+
+    /// Generates a random string suitable for
+    /// file/directory names.
+    std::string getFilename(const size_t length)
+    {
+        std::stringstream ss;
+        Poco::Base64Encoder b64(ss);
+        b64 << getBytes(length).data();
+        std::string s = ss.str();
+        std::replace(s.begin(), s.end(), '/', '_');
+        return s.substr(0, length);
+    }
 }
 }
 
@@ -114,18 +146,13 @@ namespace Util
         return id;
     }
 
+    /// Create a secure, random directory path.
     std::string createRandomDir(const std::string& path)
     {
         Poco::File(path).createDirectories();
-        for (;;)
-        {
-            const auto name = Util::encodeId(rng::getNext());
-            Poco::File dir(Poco::Path(path, name));
-            if (dir.createDirectory())
-            {
-                return name;
-            }
-        }
+        const auto name = rng::getFilename(64);
+        Poco::File(Poco::Path(path, name)).createDirectories();
+        return name;
     }
 
     std::string getTempFilePath(const std::string srcDir, const std::string& srcFilename)