Fix bug: arbitrary file read in log manager & remove block io ops

src/log-manager/build/log-manager-nginx.k8s.dockerfile

@@ -18,7 +18,7 @@
 FROM openresty/openresty:1.15.8.3-2-alpine-fat
 
 RUN luarocks install lua-cjson && luarocks install lua-resty-jwt && \
-  luarocks install luafilesystem
+  luarocks install luafilesystem && luarocks install lua-path
 
 COPY src/nginx/nginx.conf.default /etc/nginx/conf.d/default.conf
 COPY src/nginx/nginx.conf /usr/local/openresty/nginx/conf/nginx.conf

src/log-manager/src/nginx/get_log_content.lua

@@ -14,6 +14,9 @@
 
 
 local lfs = require "lfs"
+local path = require "path"
+
+local util = require "util"
 
 local function get_rotated_log(log_path)
   for file in lfs.dir(log_path) do
@@ -24,6 +27,15 @@ local function get_rotated_log(log_path)
   end
 end
 
+local function is_path_under_log_folder(log_path)
+  local real_path = path.abspath(log_path)
+
+  if not string.match(real_path, "^/usr/local/pai/logs/.*") then
+    return false
+  end
+  return true
+end
+
 
 local args = ngx.req.get_uri_args()
 local username = args["username"]
@@ -62,7 +74,7 @@ end
 
 ngx.log(ngx.INFO, "get log from path"..log_path)
 
-if lfs.attributes(log_path, "mode") ~= "file" then
+if not util.is_path_under_log_folder(log_path) or not path.isfile(log_path) then
   ngx.log(ngx.ERR, log_path.." not exists")
   ngx.status = ngx.HTTP_NOT_FOUND
   return ngx.exit(ngx.HTTP_OK)
@@ -72,7 +84,8 @@ local logs
 if (tail_mode == "true") then
   logs = io.popen("tail -c 16k "..log_path)
 else
-  logs = io.popen("cat "..log_path)
+  ngx.req.set_uri(log_path)
+  ngx.exec("@download_file")
 end
 
 -- buffer size (8K)
@@ -82,3 +95,4 @@ while true do
   if not block then break end
   ngx.say(block)
 end
+logs:close()

src/log-manager/src/nginx/list_logs.lua

@@ -14,6 +14,9 @@
 
 local cjson = require "cjson"
 local lfs = require "lfs"
+local path = require "path"
+
+local util = require "util"
 
 local function has_file_with_pattern(path, pattern)
   for file in lfs.dir(path) do
@@ -24,10 +27,6 @@ local function has_file_with_pattern(path, pattern)
   return false
 end
 
-local function is_dir(path)
-  return lfs.attributes(path, "mode") == "directory"
-end
-
 local args = ngx.req.get_uri_args()
 local username = args["username"]
 local framework_name = args["framework-name"]
@@ -43,19 +42,19 @@ end
 
 local log_query_param = "?username="..username.."&framework-name="..framework_name..
   "&pod-uid="..pod_uid.."&taskrole="..taskrole.."&token="..token
-local path = "/usr/local/pai/logs/"..username.."/".. framework_name.."/".. taskrole.."/"..pod_uid.."/"
+local log_dir = "/usr/local/pai/logs/"..username.."/".. framework_name.."/".. taskrole.."/"..pod_uid.."/"
 local path_prefix = "/api/v1/logs/"
 
 local ret = {}
 
-if not is_dir(path) then
+if not util.is_path_under_log_folder(log_dir) or not path.isdir(log_dir) then
   ngx.log(ngx.ERR, "log folder not exists")
   ngx.status = ngx.HTTP_NOT_FOUND
   return ngx.exit(ngx.HTTP_OK)
 end
 
-for file in lfs.dir(path) do
-  if not is_dir(path..file) then
+for file in lfs.dir(log_dir) do
+  if not path.isdir(log_dir..file) then
     if string.match(file, "^user%.pai%..*$") then
       local sub_str = string.sub(file, string.len("user.pai.") + 1)
       ret[sub_str] = path_prefix..file..log_query_param
@@ -65,7 +64,7 @@ for file in lfs.dir(path) do
   elseif string.match(file, "^user-.*$") then
     local sub_str = string.sub(file, string.len("user-") + 1)
     ret[sub_str] = path_prefix..file..log_query_param
-    if has_file_with_pattern(path..file, "^@.*%.s") then
+    if has_file_with_pattern(log_dir..file, "^@.*%.s") then
       ret[sub_str..".1"] = path_prefix..file..".1"..log_query_param
     end
   end

src/log-manager/src/nginx/nginx.conf

@@ -80,6 +80,9 @@ http {
     errlog.set_filter_level(ngx.INFO)
   }
   include /etc/nginx/conf.d/*.conf;
+
+  # set lua path
+  lua_package_path "/etc/nginx/lua/?.lua;;";
 }
 env ADMIN_NAME;
 env ADMIN_PASSWORD;

src/log-manager/src/nginx/nginx.conf.default

@@ -70,8 +70,17 @@ server {
     limit_except GET {
       deny all;
     }
-    default_type text/plain;
+    default_type application/octet-stream;
+    limit_rate_after 1m;
+    limit_rate 1m;
     access_by_lua_file /etc/nginx/lua/guard.lua;
     content_by_lua_file /etc/nginx/lua/get_log_content.lua;
   }
+
+  location @download_file {
+    internal;
+
+    root /;
+    try_files $uri =404;
+  }
 }

src/log-manager/src/nginx/util.lua

@@ -0,0 +1,28 @@
+-- Copyright (c) Microsoft Corporation
+-- All rights reserved.
+-- MIT License
+-- Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+-- documentation files (the "Software"), to deal in the Software without restriction, including without limitation
+-- the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and
+-- to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+-- The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+-- THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING
+-- BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+-- NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+-- DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+-- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+local path = require "path"
+
+local function is_path_under_log_folder(log_path)
+  local real_path = path.abspath(log_path)
+
+  if not string.match(real_path, "^/usr/local/pai/logs/.*") then
+    return false
+  end
+  return true
+end
+
+return {
+  is_path_under_log_folder = is_path_under_log_folder,
+}