[Libreoffice-commits] online.git: loolwsd.xml.in wsd/FileServer.cpp

[Libreoffice Gerrit user]

 loolwsd.xml.in     |    1 
 wsd/FileServer.cpp |   55 ++++++++++++++++++++++-------------------------------
 2 files changed, 24 insertions(+), 32 deletions(-)

New commits:
commit 296aba1beae64a65e4e86631a9c1458073ec8c2e
Author:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
AuthorDate: Fri Nov 23 09:33:13 2018 +0100
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Fri Nov 23 16:33:55 2018 +0100

    Improve allowed frame-ancestors
    
    Beforehand, any host could embed the iframe as the Referer was always allowed.
    
    Now, only the loolwsd and the WOPI host are allowed to do that.
    Additionally, a config option has been added to add more allowed hosts.
    
    X-Frame-Options supports has been removed as it supports only one host
    and CSP is meanwhile supported in ~all major browsers.
    
    Change-Id: I222720e1220116102708c50edaf08e2a4a0aebda
    Reviewed-on: https://gerrit.libreoffice.org/63864
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
    Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>

diff --git a/loolwsd.xml.in b/loolwsd.xml.in
index 6842a782c..48c053adb 100644
--- a/loolwsd.xml.in
+++ b/loolwsd.xml.in
@@ -79,6 +79,7 @@
         <host desc="Ditto, but as IPv4-mapped IPv6 address">::ffff:127\.0\.0\.1</host>
         <host desc="The IPv6 loopback (localhost) address.">::1</host>
       </post_allow>
+      <frame_ancestors desc="Specify who is allowed to embed the LO Online iframe (loolwsd and WOPI host are always allowed). Separate multiple hosts by space."></frame_ancestors>
     </net>
 
     <ssl desc="SSL settings">
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 0129070eb..651654364 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -681,50 +681,41 @@ void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, Poco::
            << "font-src 'self' data:; "
            << "object-src blob:; ";
 
-    std::string frameAncestor;
-    const auto it = request.find("Referer"); // Referer[sic]
-    if (it != request.end())
+    // Frame ancestors: Allow loolwsd host, wopi host and anything configured.
+    std::string configFrameAncestor = config.getString("net.frame_ancestors", "");
+    std::string frameAncestors = configFrameAncestor;
+    Poco::URI uriHost(host);
+    if (uriHost.getHost() != configFrameAncestor)
+        frameAncestors += " " + uriHost.getHost();
+
+    for (const auto& param : params)
     {
-        frameAncestor = it->second;
-        LOG_TRC("Picking frame ancestor from HTTP Referer header: " << frameAncestor);
-    }
-    else // Use WOPISrc value if Referer is absent
-    {
-        for (const auto& param : params)
+        if (param.first == "WOPISrc")
         {
-            if (param.first == "WOPISrc")
+            std::string wopiFrameAncestor;
+            Poco::URI::decode(param.second, wopiFrameAncestor);
+            if (wopiFrameAncestor != uriHost.getHost() && wopiFrameAncestor != configFrameAncestor)
             {
-                Poco::URI::decode(param.second, frameAncestor);
-                LOG_TRC("Picking frame ancestor from WOPISrc: " << frameAncestor);
-                break;
+                frameAncestors += " " + wopiFrameAncestor;
+                LOG_TRC("Picking frame ancestor from WOPISrc: " << wopiFrameAncestor);
             }
+            break;
         }
     }
 
-    // Keep only the origin, reject everything else
-    Poco::URI uriFrameAncestor(frameAncestor);
-    const std::string& frameAncestorScheme = uriFrameAncestor.getScheme();
-    const std::string& frameAncestorHost = uriFrameAncestor.getHost();
-
-    if (!frameAncestor.empty() && Util::isValidURIScheme(frameAncestorScheme) && Util::isValidURIHost(frameAncestorHost))
+    if (!frameAncestors.empty())
     {
-        frameAncestor = frameAncestorScheme + "://" + frameAncestorHost + ":" + std::to_string(uriFrameAncestor.getPort());
-
-        LOG_TRC("Final frame ancestor: " << frameAncestor);
-
-        // Replaced by frame-ancestors in CSP but some oldies don't know about that
-        oss << "X-Frame-Options: allow-from " << frameAncestor << "\r\n";
-        cspOss << "img-src 'self' data: " << frameAncestor << "; "
-               << "frame-ancestors " << frameAncestor;
+        LOG_TRC("Allowed frame ancestors: " << frameAncestors);
+        // X-Frame-Options supports only one ancestor, ignore that
+        //(it's deprecated anyway and CSP works in all major browsers)
+        cspOss << "img-src 'self' data: " << frameAncestors << "; "
+                << "frame-ancestors " << frameAncestors;
     }
     else
     {
-        LOG_TRC("Denied frame ancestor: " << frameAncestor);
-
-        cspOss << "img-src 'self' data: ;";
-        oss << "X-Frame-Options: deny\r\n";
+        LOG_TRC("Denied all frame ancestors");
+        cspOss << "img-src 'self' data: none;";
     }
-
     cspOss << "\r\n";
     // Append CSP to response headers too
     oss << cspOss.str();