diff --git a/ocaml/idl/datamodel.ml b/ocaml/idl/datamodel.ml index de320006eca..8cdfcc73f57 100644 --- a/ocaml/idl/datamodel.ml +++ b/ocaml/idl/datamodel.ml @@ -433,7 +433,10 @@ module Pool_update = struct ~doc:"Attach the pool update VDI" ~in_oss_since:None ~in_product_since:rel_ely - ~params:[ Ref _pool_update, "self", "The update to be attached"] + ~versioned_params:[ + {param_type=Ref _pool_update; param_name="self"; param_doc="The update to be attached"; param_release=ely_release; param_default=None}; + {param_type=Bool; param_name="use_localhost_proxy"; param_doc="Use the localhost proxy"; param_release=naples_release; param_default=Some (VBool false)}; + ] ~result:(String, "The file URL of pool update") ~allowed_roles:_R_POOL_OP () diff --git a/ocaml/idl/datamodel_common.ml b/ocaml/idl/datamodel_common.ml index 5ea9c15ac1b..dcb0792dfa8 100644 --- a/ocaml/idl/datamodel_common.ml +++ b/ocaml/idl/datamodel_common.ml @@ -193,6 +193,12 @@ let get_product_releases in_product_since = | x::xs -> go_through_release_order xs in go_through_release_order release_order +let naples_release = + { internal = get_product_releases rel_naples + ; opensource = get_oss_releases None + ; internal_deprecated_since = None + } + let lima_release = { internal = get_product_releases rel_lima ; opensource = get_oss_releases None diff --git a/ocaml/tests/test_pool_update.ml b/ocaml/tests/test_pool_update.ml index e4d0125581d..e85b28be125 100644 --- a/ocaml/tests/test_pool_update.ml +++ b/ocaml/tests/test_pool_update.ml @@ -25,12 +25,11 @@ let test_pool_update_destroy () = let test_pool_update_refcount () = let assert_equal = Alcotest.(check int) "assertion called by test_pool_update_refcount" in let __context = Mock.make_context_with_new_db "Mock context" in - let uuid = Helpers.get_localhost_uuid () in let vdi = make_vdi ~__context ~virtual_size:4096L () in - Xapi_pool_update.with_inc_refcount ~__context ~uuid ~vdi (fun ~__context ~uuid ~vdi -> ()); - Xapi_pool_update.with_inc_refcount ~__context ~uuid ~vdi (fun ~__context ~uuid ~vdi -> assert_equal 0 1); - Xapi_pool_update.with_dec_refcount ~__context ~uuid ~vdi (fun ~__context ~uuid ~vdi -> assert_equal 0 1); - Xapi_pool_update.with_dec_refcount ~__context ~uuid ~vdi (fun ~__context ~uuid ~vdi -> ()) + Xapi_pool_update.with_inc_refcount ~__context ~uuid:"a" ~vdi (fun ~__context ~uuid ~vdi -> ()); + Xapi_pool_update.with_inc_refcount ~__context ~uuid:"a" ~vdi (fun ~__context ~uuid ~vdi -> assert_equal 0 1); + Xapi_pool_update.with_dec_refcount ~__context ~uuid:"a" ~vdi (fun ~__context ~uuid ~vdi -> assert_equal 0 1); + Xapi_pool_update.with_dec_refcount ~__context ~uuid:"a" ~vdi (fun ~__context ~uuid ~vdi -> ()) let test_assert_space_available () = let free_bytes = 1_000_000L in @@ -41,7 +40,7 @@ let test_assert_space_available () = let test_download_restriction () = Xapi_globs.host_update_dir := "."; - let assert_no_dots s = + let assert_no_dots (_, s) = Alcotest.(check bool) "test_download_restriction: path must not contain ." true @@ -50,10 +49,10 @@ let test_download_restriction () = let test path = path |> Filename.concat Constants.get_pool_update_download_uri - |> Xapi_pool_update.path_from_uri + |> Xapi_pool_update.path_and_host_from_uri |> assert_no_dots in - List.iter test ["myfile"; ".."; "%2e%2e"] + List.iter test ["/myfile"; "/.."; "/%2e%2e"] let test = [ "test_pool_update_destroy", `Quick, test_pool_update_destroy diff --git a/ocaml/xapi/message_forwarding.ml b/ocaml/xapi/message_forwarding.ml index 89f587cbc97..8a6360a449b 100644 --- a/ocaml/xapi/message_forwarding.ml +++ b/ocaml/xapi/message_forwarding.ml @@ -4119,13 +4119,13 @@ module Forward = functor(Local: Custom_actions.CUSTOM_ACTIONS) -> struct info "Pool_update.destroy: pool update = '%s'" (pool_update_uuid ~__context self); Local.Pool_update.destroy ~__context ~self - let attach ~__context ~self = + let attach ~__context ~self ~use_localhost_proxy = info "Pool_update.attach: pool update = '%s'" (pool_update_uuid ~__context self); - let local_fn = Local.Pool_update.attach ~self in + let local_fn = Local.Pool_update.attach ~self ~use_localhost_proxy in let update_vdi = Db.Pool_update.get_vdi ~__context ~self in if Db.is_valid_ref __context update_vdi then VDI.forward_vdi_op ~local_fn ~__context ~self:update_vdi - (fun session_id rpc -> Client.Pool_update.attach rpc session_id self) + (fun session_id rpc -> Client.Pool_update.attach ~rpc ~session_id ~self ~use_localhost_proxy) else raise (Api_errors.Server_error(Api_errors.cannot_find_update, [(pool_update_uuid ~__context self)])) diff --git a/ocaml/xapi/xapi_pool_update.ml b/ocaml/xapi/xapi_pool_update.ml index ccc1d883873..7a94907803e 100644 --- a/ocaml/xapi/xapi_pool_update.ml +++ b/ocaml/xapi/xapi_pool_update.ml @@ -195,9 +195,8 @@ let create_yum_config ~__context ~self ~url = ; "" (* Newline at the end of the file *) ] -let attach_helper ~__context ~uuid ~vdi = +let attach_helper ~__context ~uuid ~vdi ~use_localhost_proxy = let host = Helpers.get_localhost ~__context in - let ip = Db.Host.get_address ~__context ~self:host in with_inc_refcount ~__context ~uuid ~vdi (fun ~__context ~uuid ~vdi -> let mount_point_parent_dir = String.concat "/" [!Xapi_globs.host_update_dir; uuid] in @@ -217,12 +216,13 @@ let attach_helper ~__context ~uuid ~vdi = with_api_errors (mount device) mount_point; debug "pool_update.attach_helper Mounted %s" mount_point ); - "http://" ^ ip ^ Constants.get_pool_update_download_uri ^ uuid ^ "/vdi" + let ip = if use_localhost_proxy then "127.0.0.1" else Db.Host.get_address ~__context ~self:host in + "http://" ^ ip ^ Constants.get_pool_update_download_uri ^ (Db.Host.get_uuid ~__context ~self:host) ^ "/" ^ uuid ^ "/vdi" -let attach ~__context ~self = +let attach ~__context ~self ~use_localhost_proxy = let uuid = Db.Pool_update.get_uuid ~__context ~self in let vdi = Db.Pool_update.get_vdi ~__context ~self in - create_yum_config ~__context ~self ~url:(attach_helper ~__context ~uuid ~vdi) + create_yum_config ~__context ~self ~url:(attach_helper ~__context ~uuid ~vdi ~use_localhost_proxy) exception Bad_update_info exception Invalid_update_uuid of string @@ -294,7 +294,7 @@ let extract_update_info ~__context ~vdi ~verify = let vdi_uuid = Db.VDI.get_uuid ~__context ~self:vdi in finally (fun () -> - let url = attach_helper ~__context ~uuid:vdi_uuid ~vdi in + let url = attach_helper ~__context ~uuid:vdi_uuid ~vdi ~use_localhost_proxy:true in let update_path = Printf.sprintf "%s/%s/vdi" !Xapi_globs.host_update_dir vdi_uuid in debug "pool_update.extract_update_info get url='%s', will parse_file in '%s'" url update_path; let xml = try @@ -532,21 +532,49 @@ let resync_host ~__context ~host = |> List.iter (fun self -> destroy ~__context ~self) end -let path_from_uri uri = +let path_and_host_from_uri uri = (* remove any dodgy use of "." or ".." NB we don't prevent the use of symlinks *) - String.sub_to_end uri (String.length Constants.get_pool_update_download_uri) - |> Filename.concat !Xapi_globs.host_update_dir - |> Uri.pct_decode - |> Stdext.Unixext.resolve_dot_and_dotdot + let host_and_path = String.sub_to_end uri (String.length Constants.get_pool_update_download_uri) in + match String.split ~limit:2 '/' host_and_path with + | [host; untrusted_path] -> + let resolved_path = untrusted_path + |> Filename.concat !Xapi_globs.host_update_dir + |> Uri.pct_decode + |> Stdext.Unixext.resolve_dot_and_dotdot + in + (host,resolved_path) + | _ -> failwith "Expecting both host and path in the uri" + +let proxy_request req s host_uuid = + Server_helpers.exec_with_new_task "pool_update_proxy_request" (fun __context -> + let host = + try Some (Db.Host.get_by_uuid ~__context ~uuid:host_uuid) with _ -> None + in + match host with + |Some host -> + let ip = Db.Host.get_address ~__context ~self:host in + let transport = Xmlrpc_client.(SSL(SSL.make (), ip, 443)) in + Xmlrpc_client.with_transport transport (fun fd -> + Unixext.really_write_string fd (Http.Request.to_wire_string req); + Unixext.proxy s fd) + |None -> + debug "Caught exception while get Host by uuid %s" host_uuid; + Http_svr.response_badrequest ~req s + ) let pool_update_download_handler (req: Request.t) s _ = debug "pool_update.pool_update_download_handler URL %s" req.Request.uri; - let filepath = path_from_uri req.Request.uri in + req.Request.close <- true; + let localhost_uuid = Helpers.get_localhost_uuid () in + let (host_uuid, filepath) = path_and_host_from_uri req.Request.uri in debug "pool_update.pool_update_download_handler %s" filepath; - if not(String.startswith !Xapi_globs.host_update_dir filepath) || not (Sys.file_exists filepath) then begin - debug "Rejecting request for file: %s (outside of or not existed in directory %s)" filepath !Xapi_globs.host_update_dir; - Http_svr.response_forbidden ~req s - end else begin - Fileserver.response_file s filepath; - req.Request.close <- true + if host_uuid <> localhost_uuid then + proxy_request req s host_uuid + else begin + if not(String.startswith !Xapi_globs.host_update_dir filepath) || not (Sys.file_exists filepath) then begin + debug "Rejecting request for file: %s (outside of or not existed in directory %s)" filepath !Xapi_globs.host_update_dir; + Http_svr.response_forbidden ~req s + end else begin + Fileserver.response_file s filepath + end end diff --git a/scripts/extensions/pool_update.apply b/scripts/extensions/pool_update.apply index 56810ef89e8..2266e0827f8 100644 --- a/scripts/extensions/pool_update.apply +++ b/scripts/extensions/pool_update.apply @@ -127,7 +127,7 @@ if __name__ == '__main__': # Apply the update. try: - yum_conf = session.xenapi.pool_update.attach(update) + yum_conf = session.xenapi.pool_update.attach(update, True) try: os.makedirs(os.path.dirname(yum_conf_file)) except OSError as e: diff --git a/scripts/extensions/pool_update.precheck b/scripts/extensions/pool_update.precheck index 81e7e86f311..8ce831dbad8 100755 --- a/scripts/extensions/pool_update.precheck +++ b/scripts/extensions/pool_update.precheck @@ -250,7 +250,7 @@ if __name__ == '__main__': # attach and get the yum configuration # generate the yum configuration file - yum_conf = session.xenapi.pool_update.attach(update) + yum_conf = session.xenapi.pool_update.attach(update, True) yum_conf_file = os.path.join(TMP_DIR, update_uuid, 'yum.conf') try: os.makedirs(os.path.dirname(yum_conf_file))