xcp-ng / xcp

Entry point for issues and wiki. Also contains some scripts and sources.
https://xcp-ng.org
1.27k stars 74 forks source link

HTTPS support for `vm.import` #318

Open julien-f opened 4 years ago

julien-f commented 4 years ago

Xen Orchestra is using this method to deploy a new appliance on a host.

Unfortunately for the moment, HTTPS is not supported which force us to HTTP, which obviously is not great for security:

> xe vm-import url="https://xoa.io/xoa.xva"
The server failed to handle your request, due to an internal error.  The given message may give details useful for debugging the problem.
message: (Failure "Unsupported URI scheme: https")
olivierlambert commented 4 years ago

I don't remember the details, but it's because of a component on HTTP lib in XAPI. There's no obvious solution except using another HTTP library (if I remember correctly)

psafont commented 4 years ago

The error comes from this function: https://github.com/xapi-project/xcp-idl/blob/master/lib/open_uri.ml#L38 which is called from https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_vm.ml#L1280

To handle https URIs a library like cohttp to establish the TLS connection should be used.

This bug is tracked inside Citrix as CP-31952

olivierlambert commented 4 years ago

Thanks for the feedback @psafont

I'm not sure what should we do next exactly?

edwintorok commented 4 years ago

The code currently opens the connection directly, which then of course doesn't support TLS. This should be changed to use Conduit instead which is more flexible and does support TLS (preferably with the OpenSSL backend instead of the native TLS backend so we don't introduce yet another TLS library).

You'd probably need a function with a different signature than with_open_uri to achieve that (it shouldn't pass a file descriptor, but a Conduit instead). Cohttp would then know to use Conduit (see Cohttp sample code).

There might be some deprecated code which uses xen-api-libs-transitional's http code, which doesn't support Conduit or TLS, but just for VM import you may not need that (the right way would be to port this code to use Cohttp too, there are some complications around file descriptor passing, but that is not needed for a client in general, just for Nbd-over-http).

olivierlambert commented 4 years ago

Pinging @johnelse about this

johnelse commented 4 years ago

Thanks for the pointers @psafont and @edwintorok, I'll have a go at this.

johnelse commented 4 years ago

Does anyone know of a Unix IO implementation of Conduit? https://github.com/mirage/ocaml-conduit only seems to support Lwt and Async. If there isn't an existing Unix implementation I can make one, but it seems it might be easier to use https://github.com/savonet/ocaml-ssl directly.

edwintorok commented 4 years ago

There is https://github.com/dinosaure/tuyau which has a Unix implementation, but its TLS implementation is done with the tls library, not OpenSSL, so tuyau would need an ocaml-ssl wrapper in addition to the tls wrapper (Conduit can choose between openssl and tls currently). Upstream conduit is moving to using Tuyau instead (see https://github.com/mirage/ocaml-conduit/issues/310), so adding an OpenSSL implementation to Tuyau might be the easiest.

olivierlambert commented 2 years ago

This topic needs to be addressed at some point (in terms of security, pulling XVAs in HTTP isn't very safe).

What's the state of Tuyau now? Should we still use that @edwintorok ?

edwintorok commented 2 years ago

It would seem that Tuyau got integrated in some form into Conduit (see https://github.com/mirage/ocaml-conduit/pull/311) together with Openssl support through lwt_ssl. I think tuyau is not needed anymore, it hasn't had an update in ~2 years, whereas Conduit got updated just 2 months ago.

I think a first step would be to update the version of Conduit from 2.x to latest 5.x in xs-opam and then we can start using it in xapi: https://github.com/xapi-project/xs-opam

psafont commented 2 years ago

I think a first step would be to update the version of Conduit from 2.x to latest 5.x in xs-opam and then we can start using it in xapi:

Way ahead of you :) xapi-project/xs-opam@5518601 (#610)

olivierlambert commented 2 years ago

So the next step will be actually try to use it :smiley:

psafont commented 2 years ago

Rob is working on a related feature and will revamp the code that supports this. I would holed off working on this for some months, hopefully it can all be done by then

olivierlambert commented 2 years ago

Let us know if we can help :)