Bug 1862107
Summary: | Image transfer via imageio proxy broken after replacing apache pki | ||
---|---|---|---|
Product: | [oVirt] ovirt-imageio | Reporter: | Nir Soffer <nsoffer> |
Component: | Daemon | Assignee: | Vojtech Juranek <vjuranek> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Petr Kubica <pkubica> |
Severity: | high | Docs Contact: | |
Priority: | unspecified | ||
Version: | 2.0.9 | CC: | bugs, didi, nsoffer, royoung, sfishbai, tnisan, vjuranek |
Target Milestone: | ovirt-4.4.2 | ||
Target Release: | 2.0.10 | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | v2.0.10 | Doc Type: | If docs needed, set a value |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2020-09-18 07:12:44 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | Storage | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 1385617 |
Description
Nir Soffer
2020-07-30 13:32:33 UTC
In ovirt-4.3 the proxy had both appache ca_file an engine ca_file, and the engine cert_file was used when accessing the host: 226 imaged_prepped = imaged_session.prepare_request(imaged_req) 227 imaged_resp = imaged_session.send( 228 imaged_prepped, verify=config.engine_ca_cert_file, 229 timeout=(connection_timeout, read_timeout), stream=stream) Because the certificate was self signed and it included engine ca, it could be used to verify the host certificates, which are vdsm certificates. In 4.4 we have only one ca_file. By default is the apache ca_file generated by engine, so it works for verifying host certificates, but if you replace it with another CA certificate, it will fail to verity the host certificates. I think the change needed is to add ca_file for the http backend, used to communicate with hosts on the proxy side. This must always be the engine CA certificate since we don't support user certificates on the hosts. Suggested changes: imageio: - Add pki files for proxy, currently we have only one set of pki files - Use proxy pki files in the proxy tests - this will reproduce the issue - Add configuration for http backend: [backend_http] ca_file = - Use config.backend_http.ca_file for http backend, instead of config.tls.ca_file - In the proxy tests, configure http backend ca_file to daemon ca_file. - Document new settings in docs/configuration.md engine: - Include backend_http:ca_file in imageio configuration (/etc/ovirt-imageio/conf.d/50-engine.conf) Handling upgrade: - When host is upgraded, engine setup will create new 50-engine.conf enabling the new configuration. This should keep user configured tls settings in 99-user.conf. Didi, what do you think? Actually this is not about the administration portal, image transfer using the API/SDK via the proxy is also broken after replacing builtin certificates with 3rd party certificates. > Handling upgrade:
> - When host is upgraded, engine setup will create new 50-engine.conf
> enabling the new configuration. This should keep user configured
> tls settings in 99-user.conf.
in case user set up own certificates, 99-user.conf needs to be modified with
[backend_http]
ca_file = /path/to/user/cert
otherwise dafault certificates will be used. Probably we should use same value as for tls.ca_file if no value is set. In case user wants to use default certificates, we will use default here as well.
Setting backend_http:ca_file is not needed in oVirt setup since we don't support user certificates on the hosts. (In reply to Nir Soffer from comment #4) > Setting backend_http:ca_file is not needed in oVirt setup since > we don't support user certificates on the hosts. I'd keep the defaulting to tls.ca_file anyway as it make config more easy and backward compatible (adding backend_http:ca_file is not needed) Do you mean that if backend_http.ca_file is not configured, use tls.ca_file? This will break when you replace certificates, so this is not a good idea. Maybe if you long a very clear warning about missing configuration. Regardless, engine *must* add backend_http:ca_file to 50-engine.conf. Vdsm does not need to configure this since we never add proxy tickets on a host. Looks good to me. Not sure the naming is very clear: - backend_http.ca_file is for accessing the engine, right? - tls.ca_file is for accessing hosts, right? And both point at CA Cert files. I'd personally expect the new setting to be under exiting section tls, perhaps named "backend_ca_file" if you want something similar to your proposal. I'd not use 'http' when you actually mean 'https'. 'http', today, sounds as deliberately "not https". To me. Re upgrades and using 3rd-party certs for https: We should (and already do, AFAIU) use, for tls.ca_file, a (by default) symlink to engine's ca cert, called apache-ca.pem. In the docs to use 3rd-party certs, we say to replace this symlink with the cert of the ca we want to use. So this should not require editing conf files, and work across upgrades. (In reply to Yedidyah Bar David from comment #7) > Looks good to me. > > Not sure the naming is very clear: > - backend_http.ca_file is for accessing the engine, right? > - tls.ca_file is for accessing hosts, right? The other way: - tls.ca_file: for the ovirt-imageio service. This is is by default apache ca_file, and the user can replace the file or change the configuration as needed to use whatever file they want. This must match the cafile used by the client talking with the proxy. - backend_http.ca_file: for contacting the ovirt-imageio service on the host. This is by default engine ca_file, and should never change, since we don't support user pki on the hosts. Users should never change or care about backend_http.ca_file, only about tls.* options. > And both point at CA Cert files. > > I'd personally expect the new setting to be under exiting section tls, > perhaps named "backend_ca_file" if you want something similar to your > proposal. tls is the public configuration that users should be aware of. backned_http.cal_file is something they should not care about. We also need other conflagration for backendds, so we are going to add backend_file and backend_nbd sections soon. Keeping all things related to specific backend at the same place is better than adding different kinds of configurations options under tls section. > I'd not use 'http' when you actually mean 'https'. 'http', today, > sounds as deliberately "not https". To me. https is http + tls, the difference is not important. Actually we support only https, but we use a more generic name for the backend. In the same way we have file backend that handles file and block devices and nbd backend handling nbd:// (tcp) and nbd+unix:// URLs. The backend specific configuration is something that users should not be aware of care about. This is more useufl for development and testing, being able to tune the system without changing the code. > Re upgrades and using 3rd-party certs for https: > > We should (and already do, AFAIU) use, for tls.ca_file, a (by default) > symlink to engine's ca cert, called apache-ca.pem. This is good default. > In the docs to use 3rd-party certs, we say to replace this symlink > with the cert of the ca we want to use. So this should not require > editing conf files, and work across upgrades. This should work, but I think a better way is to leave the files engine created and point to your own files elsewhere using a drop-in configuration. This way engine will never overwrite your certs by accident since it does not know abut them. I forgot to add that the http backend supports also http over unix socket. This is by imageio client to optimize local transfer when running on a hypervisor. So for something supporting https, http+unix http is a good name. Verified in ovirt-imageio-daemon-2.0.10-1.el8ev.x86_64 Download and upload of an image is working with 3rd party CA and changed Apache certificates This bugzilla is included in oVirt 4.4.2 release, published on September 17th 2020. Since the problem described in this bug report should be resolved in oVirt 4.4.2 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report. |