[nova][tripleo][rpm-packaging][kolla][puppet][debian][osa] Nova enforces that no DB credentials are allowed for the nova-compute service
Dear packagers and deployment engine developers, Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'. Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup. The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged. There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3]. Cheers, gibi [1] https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... -- http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
Thank you for the heads up. I write to confirm that Kolla has, for some time, not provided database credentials to nova-compute. -yoctozepto On Wed, Nov 11, 2020 at 5:36 PM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
[1] https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... -- http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
On 11/11/20 5:35 PM, Balázs Gibizer wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this. How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)? If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata Is this list correct? Or is there some services that also don't need it? Cheers, Thomas Goirand (zigo)
On Wed, 2020-11-11 at 21:08 +0100, Thomas Goirand wrote:
On 11/11/20 5:35 PM, Balázs Gibizer wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. well not to be too blunt but im not sure we resonably can chose not to implement this.
Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)? we might want two db files so nova-cell-db and nova-api-db really
If I understand well, these services would need access to db: - conductor
- scheduler
i has been raised by customer that have don a security audit in the past as a hardening issue that the db cred which are unused are present on the compute nodes. all agreeing to do it the same way woudl be a good thing but at the end of the day we should do this this cycle. we also shoudl come up with some upstream recomendation in the install docs on the best partice for this hopefully we can get agreement but if a specific config managment tool chosses to go a different way i dont think they should be force to chagne. for example kolla already dose the right thing and has for several releases. i dont think they shoudl need to change there approch. the have a singel nova.conf per container but a different one for each container https://github.com/openstack/kolla-ansible/blob/master/ansible/roles/nova-ce... the nova.conf for the compute contaienr dose not contian the db info which is the correct approch. openstack ansible also do the same thing https://github.com/openstack/openstack-ansible-os_nova/blob/master/templates... ooo will do the same thing once https://review.opendev.org/#/c/718552/ is done more or less. so form a config managment point of view the pattern is to generate a configile with only the info tha tis needed. form a packager point of view hte only way to mimic that is to use multiple files with descreeet chunks or to not install any config by default nad require operators to copy thet samples. the install guides dont expclitly call this out https://docs.openstack.org/nova/latest/install/controller-install-ubuntu.htm... https://docs.openstack.org/nova/latest/install/compute-install-ubuntu.html#i... but the compute node one does not tell you to add the db config either. i do thikn we shoudl update the comptue node install docs to use a different file however instad of nova.conf. the super conductor need the cell db and the api db form talking to dan the instance affinity/antiaffintiy late check would reuire the cell db to unfortually need api db access too. the scudler need db acess although im not sure if it needs both.
- novncproxy - serialproxy - spicehtml5proxy
- api - api-metadata
so im not sure if these 3 do. they might for some kind fo token storage although i did not think they really should need db acess, maybe to get the instance host, it would be nice if these could be restrcted to just use the api db but they might need both. there is a propsal too support passwords with consoles. https://review.opendev.org/#/c/759828/ i have not check but i assume that will require tehm to have db access if they do not already. although i think the password enforcement might be via the hypervisor e.g. libvirt so they may not need db acess for that. we should determin that. the api and metadata api both need db acess yes. i think we need both the api db and cell db acess in both cases.
Is this list correct? Or is there some services that also don't need it?
the only other nova service is the compute agent and it does not need db acess. im not sure if there is utility in
Cheers,
Thomas Goirand (zigo)
On Thu, Nov 12, 2020 at 11:18, Dmitriy Rabotyagov <noonedeadpunk@ya.ru> wrote:
Hi,
Well, while OSA really place DB credentials into nova.conf only when needed, this patch will still break our all-in-one setup. I have nothing against approach of the separating config files, but I'm sure this needs to be really well documented. Also would be great to have list of options that are required for nova-compute to work. As then it makes sense to do nova-compute.conf as minimal as possible and exclude all scheduler and placement bits from it as well (and libvirt from scheduler/api part of the conf).
I've filed a doc bug in nova[1] to improve the documentation about the mandatory minimal config for each service. Some background discussion also happened on the nova meeting[2] yesterday. Cheers, gibi [1] https://bugs.launchpad.net/nova/+bug/1904179 [2] http://eavesdrop.openstack.org/meetings/nova/2020/nova.2020-11-12-16.00.log....
On 11/18/20 9:55 AM, Dmitriy Rabotyagov wrote:
Can you kindly share the way of doing that? As passing --config-file with pyargv for uwsgi seems not the right way to do that. 15.11.2020, 15:30, "Thomas Goirand" <zigo@debian.org>:
I'll manage to get --config-file as parameters when starting daemons (it's possible to do so, even when using uwsgi).
-- Kind Regards, Dmitriy Rabotyagov
As much as I know, the --pyargv *does* the right thing. Currently, in Debian, we have neutron-api started like this: /usr/bin/uwsgi_python37 \ --https-socket :9696,/etc/neutron/ssl/public/HOSTNAME.crt,/etc/neutron/ssl/private/HOSTNAME.pem \ --ini /etc/neutron/neutron-api-uwsgi.ini \ --pyargv "--config-dir=/etc/neutron/server.conf.d \ --config-file=/etc/neutron/neutron.conf \ --config-file=/etc/neutron/plugins/ml2/ml2_conf.ini" Though it's a little bit tricky. Notice the quotes so that there's only a single argument after --pyargv... All of this has been set in openstack-pkg-tools for a few years already, though it's only in use in Debian, because Ubuntu people don't use uwsgi (which isn't part of Ubuntu main, it's only in Universe). Cheers, Thomas Goirand (zigo)
On 11/18/20 4:06 PM, Dmitriy Rabotyagov wrote:
Well, at least that does not work for nova then... uwsgi output and conf [1], journalctl logs [2] [1] http://paste.openstack.org/show/800158/ [2] http://paste.openstack.org/show/800159/
I can assure you that what I wrote works. Look at the Neutron packages in Debian if you want to make sure of that... :) Thomas Goirand (zigo)
On 11/21/20 2:40 AM, Thomas Goirand wrote:
On 11/18/20 4:06 PM, Dmitriy Rabotyagov wrote:
Well, at least that does not work for nova then... uwsgi output and conf [1], journalctl logs [2] [1] http://paste.openstack.org/show/800158/ [2] http://paste.openstack.org/show/800159/
I can assure you that what I wrote works. Look at the Neutron packages in Debian if you want to make sure of that... :)
Thomas Goirand (zigo)
Hi, See what' Gibi wrote in this thread, and his patch here: https://review.opendev.org/c/openstack/nova/+/763750 The issue isn't uwsgi and the --pyargs, but nova-api not taking args into account, which is now addressed by the above patch. Cheers, Thomas Goirand (zigo)
On 11/11/20 5:35 PM, Balázs Gibizer wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi, This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level. Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list? Regards, Javier [1] - https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since
service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around
On 11/11/20 5:35 PM, Balázs Gibizer wrote: that this
topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files: * nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api * nova.conf for the nova-scheduler and the top level nova-conductor (super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy * nova-cpu.conf for the nova-compute service The nova team suggest to use a similar strategy to separate files. So at the moment we are not planning to change what config files the wsgi apps will read. Cheers, gibi
Regards, Javier
[1] - https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Fri, 13 Nov 2020 at 10:06, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since
service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around
On 11/11/20 5:35 PM, Balázs Gibizer wrote: that this
topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files:
* nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api
* nova.conf for the nova-scheduler and the top level nova-conductor
(super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy
* nova-cpu.conf for the nova-compute service
IIUC for nova-metadata-api "it depends": local_metadata_per_cell=True it needs nova-cell<cell-id>.conf local_metadata_per_cell=False it needs nova.conf Cheers, Ollie
The nova team suggest to use a similar strategy to separate files. So
at the moment we are not planning to change what config files the wsgi
apps will read.
Cheers, gibi
Regards, Javier
[1] -
https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Fri 13 Nov 2020, 16:18 Oliver Walsh, <owalsh@redhat.com> wrote:
On Fri, 13 Nov 2020 at 10:06, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since
service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around
On 11/11/20 5:35 PM, Balázs Gibizer wrote: that this
topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files:
* nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api
* nova.conf for the nova-scheduler and the top level nova-conductor
(super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy
* nova-cpu.conf for the nova-compute service
IIUC for nova-metadata-api "it depends": local_metadata_per_cell=True it needs nova-cell<cell-id>.conf local_metadata_per_cell=False it needs nova.conf
Cheers, Ollie
Also Sean and Dan mentioned the other day that the cell level nova-conductor requires api db access, which I really did not expect. Cheers, Ollie
The nova team suggest to use a similar strategy to separate files. So
at the moment we are not planning to change what config files the wsgi
apps will read.
Cheers, gibi
Regards, Javier
[1] -
https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Fri 13 Nov 2020, 16:18 Oliver Walsh, <owalsh@redhat.com> wrote:
On Fri, 13 Nov 2020 at 10:06, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since
service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around
On 11/11/20 5:35 PM, Balázs Gibizer wrote: that this
topic. So you can find more detailed reasoning there[3].
Cheers, gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files:
* nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api
* nova.conf for the nova-scheduler and the top level nova-conductor
(super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy
* nova-cpu.conf for the nova-compute service
IIUC for nova-metadata-api "it depends": local_metadata_per_cell=True it needs nova-cell<cell-id>.conf
But hang on a second... metadata-api is a wsgi app, which hardcodes 'nova.conf', so how can this work in devstack? local_metadata_per_cell=False it needs nova.conf
Cheers, Ollie
The nova team suggest to use a similar strategy to separate files. So
at the moment we are not planning to change what config files the wsgi
apps will read.
Cheers, gibi
Regards, Javier
[1] -
https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Fri 13 Nov 2020, 16:18 Oliver Walsh, <owalsh@redhat.com> wrote:
On Fri, 13 Nov 2020 at 10:06, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
On 11/11/20 5:35 PM, Balázs Gibizer wrote: > Dear packagers and deployment engine developers, > > Since Icehouse nova-compute service does not need any database > configuration as it uses the message bus to access data in the database > via the conductor service. Also, the nova configuration guide states > that the nova-compute service should not have the > [api_database]connection config set. Having any DB credentials > configured for the nova-compute is a security risk as well since that > service runs close to the hypervisor. Since Rocky[1] nova-compute > service fails if you configure API DB credentials and set upgrade_level > config to 'auto'. > > Now we are proposing a patch[2] that makes nova-compute fail at startup > if the [database]connection or the [api_database]connection is > configured. We know that this breaks at least the rpm packaging, debian > packaging, and puppet-nova. The problem there is that in an all-in-on > deployment scenario the nova.conf file generated by these tools is > shared between all the nova services and therefore nova-compute sees DB > credentials. As a counter-example, devstack generates a separate > nova-cpu.conf and passes that to the nova-compute service even in an > all-in-on setup. > > The nova team would like to merge [2] during Wallaby but we are OK to > delay the patch until Wallaby Milestone 2 so that the packagers and > deployment tools can catch up. Please let us know if you are impacted > and provide a way to track when you are ready with the modification that > allows [2] to be merged. > > There was a long discussion on #openstack-nova today[3] around this > topic. So you can find more detailed reasoning there[3]. > > Cheers, > gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files:
* nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api
* nova.conf for the nova-scheduler and the top level nova-conductor
(super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy
* nova-cpu.conf for the nova-compute service
IIUC for nova-metadata-api "it depends": local_metadata_per_cell=True it needs nova-cell<cell-id>.conf
But hang on a second... metadata-api is a wsgi app, which hardcodes 'nova.conf', so how can this work in devstack?
On Fri, 2020-11-13 at 19:59 +0000, Oliver Walsh wrote: the nova.conf has the db creds in devstack. nova-compute is run with its own config and does not use nova.conf at all we did not really expect nova.conf to be passed as a paramter to the comptue service.
local_metadata_per_cell=False it needs nova.conf
Cheers, Ollie
The nova team suggest to use a similar strategy to separate files. So
at the moment we are not planning to change what config files the wsgi
apps will read.
Cheers, gibi
Regards, Javier
[1] -
https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
On Fri 13 Nov 2020, 22:06 Sean Mooney, <smooney@redhat.com> wrote:
On Fri 13 Nov 2020, 16:18 Oliver Walsh, <owalsh@redhat.com> wrote:
On Fri, 13 Nov 2020 at 10:06, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Thu, Nov 12, 2020 at 06:09, Javier Pena <jpena@redhat.com> wrote:
On 11/11/20 5:35 PM, Balázs Gibizer wrote: > Dear packagers and deployment engine developers, > > Since Icehouse nova-compute service does not need any database > configuration as it uses the message bus to access data in the database > via the conductor service. Also, the nova configuration guide states > that the nova-compute service should not have the > [api_database]connection config set. Having any DB credentials > configured for the nova-compute is a security risk as well
since
that > service runs close to the hypervisor. Since Rocky[1] nova-compute > service fails if you configure API DB credentials and set upgrade_level > config to 'auto'. > > Now we are proposing a patch[2] that makes nova-compute fail at startup > if the [database]connection or the [api_database]connection is > configured. We know that this breaks at least the rpm
debian > packaging, and puppet-nova. The problem there is that in an all-in-on > deployment scenario the nova.conf file generated by these tools is > shared between all the nova services and therefore nova-compute sees DB > credentials. As a counter-example, devstack generates a separate > nova-cpu.conf and passes that to the nova-compute service even in an > all-in-on setup. > > The nova team would like to merge [2] during Wallaby but we are OK to > delay the patch until Wallaby Milestone 2 so that the
and > deployment tools can catch up. Please let us know if you are impacted > and provide a way to track when you are ready with the modification that > allows [2] to be merged. > > There was a long discussion on #openstack-nova today[3] around this > topic. So you can find more detailed reasoning there[3]. > > Cheers, > gibi
IMO, that's ok if, and only if, we all agree on how to implement it. Best would be if we (all downstream distro + config management) agree on how to implement this.
How about, we all implement a /etc/nova/nova-db.conf, and get all services that need db access to use it (ie: starting them with --config-file=/etc/nova/nova-db.conf)?
Hi,
This is going to be an issue for those services we run as a WSGI app. Looking at [1], I see the app has a hardcoded list of config files to read (api-paste.ini and nova.conf), so we'd need to modify it at the installer level.
Personally, I like the nova-db.conf way, since it looks like it reduces the amount of work required for all-in-one installers to adapt, but that requires some code change. Would the Nova team be happy with adding a nova-db.conf file to that list?
Devstack solves the all-in-one case by using these config files:
* nova.conf and api_paste.ini for the wsgi apps e.g. nova-api and nova-metadata-api
* nova.conf for the nova-scheduler and the top level nova-conductor
(super conductor) * nova-cell<cell-id>.conf for the cell level nova-conductor and the proxy services, e.g. nova-novncproxy
* nova-cpu.conf for the nova-compute service
IIUC for nova-metadata-api "it depends": local_metadata_per_cell=True it needs nova-cell<cell-id>.conf
But hang on a second... metadata-api is a wsgi app, which hardcodes 'nova.conf', so how can this work in devstack?
On Fri, 2020-11-13 at 19:59 +0000, Oliver Walsh wrote: packaging, packagers the nova.conf has the db creds in devstack. nova-compute is run with its own config and does not use nova.conf at all
Yes, if its a global metadata api, but for a cell local metadata api it needs to use nova-cell<cell-id>.conf
we did not really expect nova.conf to be passed as a paramter to the comptue service.
local_metadata_per_cell=False it needs nova.conf
Cheers, Ollie
The nova team suggest to use a similar strategy to separate files. So
at the moment we are not planning to change what config files the wsgi
apps will read.
Cheers, gibi
Regards, Javier
[1] -
https://opendev.org/openstack/nova/src/branch/master/nova/api/openstack/wsgi...
If I understand well, these services would need access to db: - conductor - scheduler - novncproxy - serialproxy - spicehtml5proxy - api - api-metadata
Is this list correct? Or is there some services that also don't need it?
Cheers,
Thomas Goirand (zigo)
IIUC from Sean's reply most of the heavyweight configuration management frameworks already address the security concern. For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches. The tripleo changes in https://review.opendev.org/#/c/718552/ are not strictly necessary to remove the db creds from nova.conf. However I need to go further, also removing the hieradata that contains the db creds since completely removing the db creds from the compute hosts is the ultimate goal here. So when the configuration management frameworks are all good then what are we actually concerned about security-wise? Is it just operators that roll their own deployments? Cheers, Ollie On Wed, 11 Nov 2020 at 16:37, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
[1]
https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3]
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... --
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches. I don't know whether we can backport that to stable branches. I agreed to remove nova::db from the base nova class because we already deprecated database parameters in the nova class, but in stable branches we have these parameters still valid and removal of nova::db can cause some issues with these parameters. (I guess pick dosn't work if the nova class was defined AFTER nova::db class and there are no hieradata used) So I'd say it's better to leave it in stable branches if nova doesn't require absence of db parameters in stable branches.
Note that we can remove these parameters from compute nodes in TripleO deployment if we completely remove these hieradata from compute nodes. Also from puppet's perspective we still need some fixes because the standalone deployment can still be broken with the change in nova. We need to know what would be the decision made in each distro packaging and use the proper config files to store db parameters or the other parameters. On Fri, Nov 13, 2020 at 1:43 AM Oliver Walsh <owalsh@redhat.com> wrote:
IIUC from Sean's reply most of the heavyweight configuration management frameworks already address the security concern.
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches.
The tripleo changes in https://review.opendev.org/#/c/718552/ are not strictly necessary to remove the db creds from nova.conf. However I need to go further, also removing the hieradata that contains the db creds since completely removing the db creds from the compute hosts is the ultimate goal here.
So when the configuration management frameworks are all good then what are we actually concerned about security-wise? Is it just operators that roll their own deployments?
Cheers, Ollie
On Wed, 11 Nov 2020 at 16:37, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
[1]
https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3]
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... --
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
-- ---------- Takashi Kajinami Senior Software Maintenance Engineer Customer Experience and Engagement Red Hat e-mail: tkajinam@redhat.com
On Fri 13 Nov 2020, 01:25 Takashi Kajinami, <tkajinam@redhat.com> wrote:
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches. I don't know whether we can backport that to stable branches. I agreed to remove nova::db from the base nova class because we already deprecated database parameters in the nova class, but in stable branches we have these parameters still valid and removal of nova::db can cause some issues with these parameters. (I guess pick dosn't work if the nova class was defined AFTER nova::db class and there are no hieradata used)
I believe you are correct about the pick function. However I cannot think of any valid use case where the nova class would be used without also using one of the service specific classes (nova::api/nova::compute/nova::scheduler). i.e if you previously did something like this it would still work: class { 'nova': database_connection => "foobar" } include nova::api if you previously did this it didn't work and continues to not work: include nova::api class { 'nova': database_connection => "foobar" } if you previously did this you would get db conf, now you will not: class { 'nova': database_connection => "foobar" } I'm not sure what purpose of this would be. I guess if you want to generate a nova.conf that contains just the config options that are common to all services... If that is the use case then the DB creds should not be included, they are not common to all nova services. If you are still concerned then in the backports we could continue including nova::db when any of the deprecated params are set.
So I'd say it's better to leave it in stable branches if nova doesn't require absence of db parameters in stable branches.
Note that we can remove these parameters from compute nodes in TripleO deployment if we completely remove these hieradata from compute nodes.
Compute nodes would be ok but Controllers have nova-compute for ironic. Single-node all-in-one deployments are obviously an issue too. Cheers, Ollie
Also from puppet's perspective we still need some fixes because the standalone deployment can still be broken with the change in nova.
We need to know what would be the decision made in each distro packaging
and use the proper config files to store db parameters or the other parameters.
On Fri, Nov 13, 2020 at 1:43 AM Oliver Walsh <owalsh@redhat.com> wrote:
IIUC from Sean's reply most of the heavyweight configuration management frameworks already address the security concern.
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches.
The tripleo changes in https://review.opendev.org/#/c/718552/ are not strictly necessary to remove the db creds from nova.conf. However I need to go further, also removing the hieradata that contains the db creds since completely removing the db creds from the compute hosts is the ultimate goal here.
So when the configuration management frameworks are all good then what are we actually concerned about security-wise? Is it just operators that roll their own deployments?
Cheers, Ollie
On Wed, 11 Nov 2020 at 16:37, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
[1]
https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3]
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... --
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
-- ---------- Takashi Kajinami Senior Software Maintenance Engineer Customer Experience and Engagement Red Hat e-mail: tkajinam@redhat.com
On Fri 13 Nov 2020, 01:25 Takashi Kajinami, <tkajinam@redhat.com> wrote:
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches. I don't know whether we can backport that to stable branches. I agreed to remove nova::db from the base nova class because we already deprecated database parameters in the nova class, but in stable branches we have these parameters still valid and removal of nova::db can cause some issues with these parameters. (I guess pick dosn't work if the nova class was defined AFTER nova::db class and there are no hieradata used)
I believe you are correct about the pick function. However I cannot think of any valid use case where the nova class would be used without also using one of the service specific classes (nova::api/nova::compute/nova::scheduler).
i.e if you previously did something like this it would still work:
class { 'nova': database_connection => "foobar" } include nova::api
if you previously did this it didn't work and continues to not work:
include nova::api class { 'nova': database_connection => "foobar" }
I thought this could have worked somehow before we removed nova::db from
On Fri, Nov 13, 2020 at 11:19 PM Oliver Walsh <owalsh@redhat.com> wrote: the nova class but as you mentioned this was invalid method even if we have nova::db included, so I was wrong about that. If we don't break any existing usage then I think we are good to backport the fix to stable branches.
if you previously did this you would get db conf, now you will not:
class { 'nova': database_connection => "foobar" }
I'm not sure what purpose of this would be. I guess if you want to generate a nova.conf that contains just the config options that are common to all services... If that is the use case then the DB creds should not be included, they are not common to all nova services.
Yeah I don't expect this usage in real deployment, either.
If you are still concerned then in the backports we could continue including nova::db when any of the deprecated params are set.
So I'd say it's better to leave it in stable branches if nova doesn't require absence of db parameters in stable branches.
Note that we can remove these parameters from compute nodes in TripleO deployment if we completely remove these hieradata from compute nodes.
Compute nodes would be ok but Controllers have nova-compute for ironic. Single-node all-in-one deployments are obviously an issue too.
Cheers, Ollie
Also from puppet's perspective we still need some fixes because the standalone deployment can still be broken with the change in nova.
We need to know what would be the decision made in each distro packaging
and use the proper config files to store db parameters or the other parameters.
On Fri, Nov 13, 2020 at 1:43 AM Oliver Walsh <owalsh@redhat.com> wrote:
IIUC from Sean's reply most of the heavyweight configuration management frameworks already address the security concern.
For tripleo the first issue to address is in puppet-nova where the dbs are currently configured for every nova service. The fix seems trivial - https://review.opendev.org/755689. I also think that should be completely safe to backport this to all stable branches.
The tripleo changes in https://review.opendev.org/#/c/718552/ are not strictly necessary to remove the db creds from nova.conf. However I need to go further, also removing the hieradata that contains the db creds since completely removing the db creds from the compute hosts is the ultimate goal here.
So when the configuration management frameworks are all good then what are we actually concerned about security-wise? Is it just operators that roll their own deployments?
Cheers, Ollie
On Wed, 11 Nov 2020 at 16:37, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
Dear packagers and deployment engine developers,
Since Icehouse nova-compute service does not need any database configuration as it uses the message bus to access data in the database via the conductor service. Also, the nova configuration guide states that the nova-compute service should not have the [api_database]connection config set. Having any DB credentials configured for the nova-compute is a security risk as well since that service runs close to the hypervisor. Since Rocky[1] nova-compute service fails if you configure API DB credentials and set upgrade_level config to 'auto'.
Now we are proposing a patch[2] that makes nova-compute fail at startup if the [database]connection or the [api_database]connection is configured. We know that this breaks at least the rpm packaging, debian packaging, and puppet-nova. The problem there is that in an all-in-on deployment scenario the nova.conf file generated by these tools is shared between all the nova services and therefore nova-compute sees DB credentials. As a counter-example, devstack generates a separate nova-cpu.conf and passes that to the nova-compute service even in an all-in-on setup.
The nova team would like to merge [2] during Wallaby but we are OK to delay the patch until Wallaby Milestone 2 so that the packagers and deployment tools can catch up. Please let us know if you are impacted and provide a way to track when you are ready with the modification that allows [2] to be merged.
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
Cheers, gibi
[1]
https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... [2] https://review.opendev.org/#/c/762176 [3]
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2... --
http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2...
-- ---------- Takashi Kajinami Senior Software Maintenance Engineer Customer Experience and Engagement Red Hat e-mail: tkajinam@redhat.com
-- ---------- Takashi Kajinami Senior Software Maintenance Engineer Customer Experience and Engagement Red Hat e-mail: tkajinam@redhat.com
On 11/13/20 2:25 AM, Takashi Kajinami wrote:
We need to know what would be the decision made in each distro packaging and use the proper config files to store db parameters or the other parameters.
My decision (for Debian) is to move toward using: - nova.conf (as usual, but without any db info) - nova-db.conf - nova-api-db.conf I'll write a function inside openstack-pkg-tools to get this done, and the config files stored in the nova-common package (and the matching debconf scripts poking into the correct new configuration files). I hope Canonical people will follow my lead (could you confirm please?) so we don't have to write special cases for Ubuntu or Debian in puppet-openstack. I will implement this starting with Victoria and on, and patch puppet-openstack. I'll manage to get --config-file as parameters when starting daemons (it's possible to do so, even when using uwsgi). I still don't understand the nova-cell<ID>-db.conf thing though. Isn't the connection info for the cells located in the nova-api database? Cheers, Thomas Goirand (zigo)
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
I just had a long voice chat with Ollie about this, trying to understand some of the challenges that still exist with some of the things we've discussed and the proposed forward steps. There are lots of things to clarify, I think, and we could honestly probably use a wider voice chat among the people that need to work on this. However, I'll try here. First off, I want to address the "do it like devstack" recommendation, and the subsequent suggestion of standardizing on something like a "nova-db.conf" file arrangement to keep the db credentials in one place. As Ollie has pointed out, devstack doesn't support all the various deployment arrangements (such as "one metadata api per cell") and thus doesn't have a prescription for how to arrange the config files in those scenarios. Specifically, an all-in-one deployment that includes multiple API services with different configs would currently have to hack around the hardcoded nova.conf file with the environment variable that allows them to specify a directory other than /etc/nova.conf. Devstack doesn't have to do this because it doesn't support that arrangement of services, but if it did, it would have to. So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." That doesn't mean "nova-compute should point to /etc/nova/nova-cpu.conf because that's what devstack does". Especially in a containerized setup, I would expect everything to use /etc/nova/nova.conf, since those are namespaced per-service because of the containerization. Further, just because devstack doesn't run a metadata per cell (or any other such optional arrangement), obviously that doesn't mean you can't. That leads me to the first action item I think we need: ACTION 1: Make the wsgi app able to use something other than nova.conf All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs. The next item is related: ACTION 2: We need to document a minimal viable config for each service Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow. Now, on to the question about the hard-fail patch for compute: https://review.opendev.org/#/c/762176/ The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now. Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to: ACTION 3: Not merge that hard fail until X We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time. To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider: ACTION 4: A workaround flag to opt-in to the strict checking Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived. We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check. So, I know that was a lot of words, but I think if we can agree to the above four items, we'll have a path forward that doesn't overly burden any one specific group while still allowing us to chart a path to getting where we want to be. I think we need acks from a bunch of groups, probably at least: - Nova (gibi, stephenfin) - TripleO (owalsh) - Debian (zigo) - Kolla (yoctozepto) - OSA (noonedeadpunk) - rdo-ci (jpena) - puppet-nova (tkajinam) Are people okay with these action items? --Dan
So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." I have one concern with the way we generate the default config, if we expect separated config files. Currently in most of the distros we use oslo-config-generator to render the default conf file. Since oslo-config-generator rendors all config options implemented in nova, we don't have any good way to generate the sample config file for each service. We didn't care which file we put the option since all services use the single file, but now we should always refer to that documentation unless we implement generation of service-specific files or we write down the services requiring that option in parameter description. # We have the same problem with nova-db.conf approach.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. ... The issue with TripleO was fixed by the change in puppet-nova[1], and nova.conf for nova-compute service no longer includes database credentials. We can backport the fix is needed, as per discussion with owalsh on the previous mails. [1] https://review.opendev.org/#/c/755689/
But from puppet-nova's perspective the sandalone deployment or ironic deployment(*1) still needs some fix to address the separate config files, because all processes run on host, not on container, and refer to the same default conf file, nova.conf. Since puppet-nova follows the way how distro packaging provides the default config files, we need to fix current implementation in puppet-nova after each distro packages provide the updated version with new config file structure. (*1) In usual deployment with ironic, we have nova-compute running on the controller nodes, where nova-api is also running, IIUC. On Thu, Nov 19, 2020 at 4:27 AM Dan Smith <dms@danplanet.com> wrote:
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
I just had a long voice chat with Ollie about this, trying to understand some of the challenges that still exist with some of the things we've discussed and the proposed forward steps.
There are lots of things to clarify, I think, and we could honestly probably use a wider voice chat among the people that need to work on this. However, I'll try here.
First off, I want to address the "do it like devstack" recommendation, and the subsequent suggestion of standardizing on something like a "nova-db.conf" file arrangement to keep the db credentials in one place. As Ollie has pointed out, devstack doesn't support all the various deployment arrangements (such as "one metadata api per cell") and thus doesn't have a prescription for how to arrange the config files in those scenarios. Specifically, an all-in-one deployment that includes multiple API services with different configs would currently have to hack around the hardcoded nova.conf file with the environment variable that allows them to specify a directory other than /etc/nova.conf. Devstack doesn't have to do this because it doesn't support that arrangement of services, but if it did, it would have to.
So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." That doesn't mean "nova-compute should point to /etc/nova/nova-cpu.conf because that's what devstack does". Especially in a containerized setup, I would expect everything to use /etc/nova/nova.conf, since those are namespaced per-service because of the containerization. Further, just because devstack doesn't run a metadata per cell (or any other such optional arrangement), obviously that doesn't mean you can't.
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
The next item is related:
ACTION 2: We need to document a minimal viable config for each service
Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow.
Now, on to the question about the hard-fail patch for compute:
https://review.opendev.org/#/c/762176/
The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to:
ACTION 3: Not merge that hard fail until X
We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time.
To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider:
ACTION 4: A workaround flag to opt-in to the strict checking
Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived.
We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check.
So, I know that was a lot of words, but I think if we can agree to the above four items, we'll have a path forward that doesn't overly burden any one specific group while still allowing us to chart a path to getting where we want to be.
I think we need acks from a bunch of groups, probably at least:
- Nova (gibi, stephenfin) - TripleO (owalsh) - Debian (zigo) - Kolla (yoctozepto) - OSA (noonedeadpunk) - rdo-ci (jpena) - puppet-nova (tkajinam)
Are people okay with these action items?
--Dan
-- ---------- Takashi Kajinami Senior Software Maintenance Engineer Customer Experience and Engagement Red Hat e-mail: tkajinam@redhat.com
I think it's important that Action 1 and Action 2 should be made before X then (perfectly during W), otherwise futher delaying of the strick check does not make much sense as it still be kind of urgent change.
Yep, the goal would be to do items 1, 2, and 4 in W and then not merge the hard fail patch until X. Thanks! --Dan
On Wed, Nov 18, 2020 at 11:24, Dan Smith <dms@danplanet.com> wrote:
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
I just had a long voice chat with Ollie about this, trying to understand some of the challenges that still exist with some of the things we've discussed and the proposed forward steps.
Thank you Dan for pushing this forward.
There are lots of things to clarify, I think, and we could honestly probably use a wider voice chat among the people that need to work on this. However, I'll try here.
First off, I want to address the "do it like devstack" recommendation, and the subsequent suggestion of standardizing on something like a "nova-db.conf" file arrangement to keep the db credentials in one place. As Ollie has pointed out, devstack doesn't support all the various deployment arrangements (such as "one metadata api per cell") and thus doesn't have a prescription for how to arrange the config files in those scenarios. Specifically, an all-in-one deployment that includes multiple API services with different configs would currently have to hack around the hardcoded nova.conf file with the environment variable that allows them to specify a directory other than /etc/nova.conf. Devstack doesn't have to do this because it doesn't support that arrangement of services, but if it did, it would have to.
So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." That doesn't mean "nova-compute should point to /etc/nova/nova-cpu.conf because that's what devstack does". Especially in a containerized setup, I would expect everything to use /etc/nova/nova.conf, since those are namespaced per-service because of the containerization. Further, just because devstack doesn't run a metadata per cell (or any other such optional arrangement), obviously that doesn't mean you can't.
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
I will look into this change and propose patch a patch.
The next item is related:
ACTION 2: We need to document a minimal viable config for each service
Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow.
I've opened a doc bug to track the minimal config documentation work: https://bugs.launchpad.net/nova/+bug/1904179 But I would like to get help from others to start proposing this doc.
Now, on to the question about the hard-fail patch for compute:
https://review.opendev.org/#/c/762176/
The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to:
ACTION 3: Not merge that hard fail until X
We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time.
I'm OK not to merge the breaking patch in W.
To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider:
ACTION 4: A workaround flag to opt-in to the strict checking
Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived.
We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check.
I will make the suggested changes in my patch.
So, I know that was a lot of words, but I think if we can agree to the above four items, we'll have a path forward that doesn't overly burden any one specific group while still allowing us to chart a path to getting where we want to be.
I think we need acks from a bunch of groups, probably at least:
- Nova (gibi, stephenfin) - TripleO (owalsh) - Debian (zigo) - Kolla (yoctozepto) - OSA (noonedeadpunk) - rdo-ci (jpena) - puppet-nova (tkajinam)
Are people okay with these action items?
I'm on OK with this direction. Cheers, gibi
--Dan
I've opened a doc bug to track the minimal config documentation work: https://bugs.launchpad.net/nova/+bug/1904179 But I would like to get help from others to start proposing this doc.
Yep, I've been kinda thinking about how to arrange it since yesterday. I've only got today and Monday left in the year, so no promises, but I'll try to at least make a start on it. --Dan
On Thu, Nov 19, 2020 at 13:07, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Wed, Nov 18, 2020 at 11:24, Dan Smith <dms@danplanet.com> wrote:
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
I will look into this change and propose patch a patch.
The next item is related:
ACTION 2: We need to document a minimal viable config for each service
Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow.
I've opened a doc bug to track the minimal config documentation work: https://bugs.launchpad.net/nova/+bug/1904179 But I would like to get help from others to start proposing this doc.
Dan started the documentation work here https://review.opendev.org/#/c/763412/
Now, on to the question about the hard-fail patch for compute:
https://review.opendev.org/#/c/762176/
The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to:
ACTION 3: Not merge that hard fail until X
We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time.
I'm OK not to merge the breaking patch in W.
I've pushed a patch for the X cycle with a big Workflow -1 https://review.opendev.org/#/c/763559/
To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider:
ACTION 4: A workaround flag to opt-in to the strict checking
Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived.
We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check.
I will make the suggested changes in my patch.
I've updated the patch with the workaround config option: https://review.opendev.org/#/c/762176 Cheers, gibi
On Thu, Nov 19, 2020 at 13:07, Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Wed, Nov 18, 2020 at 11:24, Dan Smith <dms@danplanet.com> wrote:
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
I will look into this change and propose patch a patch.
Patch is up https://review.opendev.org/c/openstack/nova/+/763750 Cheers, gibi
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
I just had a long voice chat with Ollie about this, trying to understand some of the challenges that still exist with some of the things we've discussed and the proposed forward steps.
There are lots of things to clarify, I think, and we could honestly probably use a wider voice chat among the people that need to work on this. However, I'll try here.
First off, I want to address the "do it like devstack" recommendation, and the subsequent suggestion of standardizing on something like a "nova-db.conf" file arrangement to keep the db credentials in one place. As Ollie has pointed out, devstack doesn't support all the various deployment arrangements (such as "one metadata api per cell") and thus doesn't have a prescription for how to arrange the config files in those scenarios. Specifically, an all-in-one deployment that includes multiple API services with different configs would currently have to hack around the hardcoded nova.conf file with the environment variable that allows them to specify a directory other than /etc/nova.conf. Devstack doesn't have to do this because it doesn't support that arrangement of services, but if it did, it would have to.
So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." That doesn't mean "nova-compute should point to /etc/nova/nova-cpu.conf because that's what devstack does". Especially in a containerized setup, I would expect everything to use /etc/nova/nova.conf, since those are namespaced per-service because of the containerization. Further, just because devstack doesn't run a metadata per cell (or any other such optional arrangement), obviously that doesn't mean you can't.
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
The next item is related:
ACTION 2: We need to document a minimal viable config for each service
Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow.
Now, on to the question about the hard-fail patch for compute:
https://review.opendev.org/#/c/762176/
The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to:
ACTION 3: Not merge that hard fail until X
We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time.
To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider:
ACTION 4: A workaround flag to opt-in to the strict checking
Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived.
We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check.
So, I know that was a lot of words, but I think if we can agree to the above four items, we'll have a path forward that doesn't overly burden any one specific group while still allowing us to chart a path to getting where we want to be.
I think we need acks from a bunch of groups, probably at least:
- Nova (gibi, stephenfin) - TripleO (owalsh) - Debian (zigo) - Kolla (yoctozepto) - OSA (noonedeadpunk) - rdo-ci (jpena) - puppet-nova (tkajinam)
Are people okay with these action items?
Hi Dan, Thanks for the detailed proposal. It looks good to me. Echoing Takashi's concerns, it would be great if action 2 could also include generating some separate oslo-config-generator configuration files. That would help distributions generate different nova-*.conf files, and then deployment projects could follow. Regards, Javier
--Dan
On Fri, Nov 20, 2020 at 04:46, Javier Pena <jpena@redhat.com> wrote:
There was a long discussion on #openstack-nova today[3] around this topic. So you can find more detailed reasoning there[3].
I just had a long voice chat with Ollie about this, trying to understand some of the challenges that still exist with some of the things we've discussed and the proposed forward steps.
There are lots of things to clarify, I think, and we could honestly probably use a wider voice chat among the people that need to work on this. However, I'll try here.
First off, I want to address the "do it like devstack" recommendation, and the subsequent suggestion of standardizing on something like a "nova-db.conf" file arrangement to keep the db credentials in one place. As Ollie has pointed out, devstack doesn't support all the various deployment arrangements (such as "one metadata api per cell") and thus doesn't have a prescription for how to arrange the config files in those scenarios. Specifically, an all-in-one deployment that includes multiple API services with different configs would currently have to hack around the hardcoded nova.conf file with the environment variable that allows them to specify a directory other than /etc/nova.conf. Devstack doesn't have to do this because it doesn't support that arrangement of services, but if it did, it would have to.
So, I wanted to clarify something that came out of the IRC discussion, which is "do it like devstack". When we say that, or at least when *I* said it, we meant "have different config files for each service so that you can get the config elements appropriate for each service set correctly." That doesn't mean "nova-compute should point to /etc/nova/nova-cpu.conf because that's what devstack does". Especially in a containerized setup, I would expect everything to use /etc/nova/nova.conf, since those are namespaced per-service because of the containerization. Further, just because devstack doesn't run a metadata per cell (or any other such optional arrangement), obviously that doesn't mean you can't.
That leads me to the first action item I think we need:
ACTION 1: Make the wsgi app able to use something other than nova.conf
All of our other services support a --config-file flag, and I don't see why we wouldn't allow this if it's necessary for deployments. In the pure API arrangement, it shouldn't really be necessary to change this, but clearly you may need to have multiple API workers with different configs, as is the case with metadata-per-cell, or even potentially some admin-focused private API endpoint. If it makes it easier for deployment tools to start the API workers with a specific config file, we should let them do that. You can already work around that hard-coded filename by setting OS_NOVA_CONFIG_DIR to something other than /etc/nova, but we shouldn't require them to create directories just to have separate configs.
The next item is related:
ACTION 2: We need to document a minimal viable config for each service
Ollie points out that, obviously, handing the deployer a config reference with 1.21 bajillion config options does not clearly indicate which services need which thing, and especially, which things are _not_allowed_ to be set for a service (such as db credentials on the compute). We can't feasibly audit every config option we have, but it would be very helpful to have a reference that shows what a completely minimal configuration for each service looks like. We could do that by tagging services per config options (which I suggested earlier, and would still be good) but I think maybe a standalone document would be easier for people to follow.
Now, on to the question about the hard-fail patch for compute:
https://review.opendev.org/#/c/762176/
The Nova devs have wanted people to _not_ have db credentials in the config file that nova-compute can see for a "Very Long Time." We have even made some assumptions in the code that rely on these credentials being not set on the compute, which is at least partially why we're having this conversation (again) now.
Despite the fact that nobody *should* be setting these credentials in their config file, we know that at in least some circumstances, they are. TripleO is setting them (always I think?) because puppet-nova does that. OSA has said that they're setting them somewhat incidentally when they do all-in-one deployments. The latter is unlikely to affect any real deployments, but the former definitely _will_, and anyone else that isn't present in this discussion may be including credentials unnecessarily, which we will break when we merge that patch. Thus, I think that, even though it feels long overdue for devs, the most prudent and cautious approach will be to:
ACTION 3: Not merge that hard fail until X
We have the warning, we have the reno. We expect that the deployment tools will be working to eliminate the credentials for the compute config, but merging that will make it an emergency for them. We've waited years at this point, we can wait one more cycle for safety. As Ollie pointed out, we've not been super clear about messaging this, despite it being a well-worn assumption amongst the developers for a long time.
To aid in smoking out any of the jobs or configs that deployment tools may still have which will break when we merge that patch, we should also consider:
ACTION 4: A workaround flag to opt-in to the strict checking
Basically, this would be a one or two-cycle workaround, which when enabled, would opt-in to the (above) behavior of causing compute to fail on startup if db credentials are present. This would allow the deployment tool maintainers, as well as people responsible for CI jobs to smoke test the new behavior early, before we merge it and cause an emergency for them. We can set this as on by default in our devstack jobs to signal that it is good with the new behavior, and also encourage the other deployment projects to set it as well once they're generating clean configs for their nova-computes. Once we merge the patch to actually fail all the time, we can remove this workaround config, according to the original intent of the workarounds group, that those flags are short-lived.
We could do this by altering gibi's patch to only fail if the workaround is enabled, and then follow up in X by removing the workaround check.
So, I know that was a lot of words, but I think if we can agree to the above four items, we'll have a path forward that doesn't overly burden any one specific group while still allowing us to chart a path to getting where we want to be.
I think we need acks from a bunch of groups, probably at least:
- Nova (gibi, stephenfin) - TripleO (owalsh) - Debian (zigo) - Kolla (yoctozepto) - OSA (noonedeadpunk) - rdo-ci (jpena) - puppet-nova (tkajinam)
Are people okay with these action items?
Hi Dan,
Thanks for the detailed proposal. It looks good to me.
Echoing Takashi's concerns, it would be great if action 2 could also include generating some separate oslo-config-generator configuration files. That would help distributions generate different nova-*.conf files, and then deployment projects could follow.
We quickly touch this topic before on IRC and it is considered a huge work to tag each nova config option with the service it uses. So I would not think it will happen without figuring out a gradual approach and having people signing up to help. Cheers, gibi
Regards, Javier
--Dan
We quickly touch this topic before on IRC and it is considered a huge work to tag each nova config option with the service it uses. So I would not think it will happen without figuring out a gradual approach and having people signing up to help.
It's also subject to change, so there needs to be maintenance of it going forward. It can also change _according_ to the config (i.e. configdrive uses metadata api bits on the compute, but not otherwise). --Dan
On 11/18/20 8:24 PM, Dan Smith wrote:
which things are _not_allowed_ to be set for a service (such as db credentials on the compute).
I still don't understand why this is forbidden. Sure, I understand what people wrote: that it is a security problem. Can't nova-compute just *ignore* the db credentials, and then everyone is done with it, and moves on? That's a much more easy way to handle this problem, IMO. Cheers, Thomas Goirand (zigo)
Hello, Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to make sure the database parameters for the classes is set. We've been running without database credentials in nova.conf on our compute nodes for years. Best regards Tobias ________________________________ From: Thomas Goirand <zigo@debian.org> Sent: Saturday, November 21, 2020 2:47:23 AM To: openstack maillist Subject: Re: [nova][tripleo][rpm-packaging][kolla][puppet][debian][osa] Nova enforces that no DB credentials are allowed for the nova-compute service On 11/18/20 8:24 PM, Dan Smith wrote:
which things are _not_allowed_ to be set for a service (such as db credentials on the compute).
I still don't understand why this is forbidden. Sure, I understand what people wrote: that it is a security problem. Can't nova-compute just *ignore* the db credentials, and then everyone is done with it, and moves on? That's a much more easy way to handle this problem, IMO. Cheers, Thomas Goirand (zigo)
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias, That's not what I'm suggesting. I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix. Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over? What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation. Cheers, Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host. Cheers, gibi
Cheers,
Thomas Goirand (zigo)
On 11/23/20 11:42 AM, Balázs Gibizer wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
Cheers, gibi
In general, I would agree with this. However, because of the way cold migrations are working, nova compute boxes are authenticated between each other through ssh. You'd be limiting access to the db, but someone with nova or root ssh access could destroy all compute nodes. So, like it or not, it's game-over anyways. If you want to fix things, make it so that Nova doesn't require such ssh authentication anymore, because that's more urgent, *THEN* we can revisit this topic. Cheers, Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 13:21, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 11:42 AM, Balázs Gibizer wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
Cheers, gibi
In general, I would agree with this. However, because of the way cold migrations are working, nova compute boxes are authenticated between each other through ssh. You'd be limiting access to the db, but someone with nova or root ssh access could destroy all compute nodes. So, like it or not, it's game-over anyways. If you want to fix things, make it so that Nova doesn't require such ssh authentication anymore, because that's more urgent, *THEN* we can revisit this topic.
While I agree that we should do changes, if possible, to avoid the need of ssh between the computes, I don't agree that a compromised compute is a game over for the whole cloud. If the deployment is split up to cells, then the game over is limited to the cell the compromised compute is in. Cheers, gibi
Cheers,
Thomas Goirand (zigo)
Hello, This is very true and good feedback, something that nags me everytime I think about it. Having a shared storage and using rbd images backend and still needing SSH between compute nodes. I'm hoping to have some time helping out to clear that up in the libvirt driver. Best regards ________________________________ From: Thomas Goirand <zigo@debian.org> Sent: Monday, November 23, 2020 1:21:14 PM To: openstack maillist Subject: Re: [nova][tripleo][rpm-packaging][kolla][puppet][debian][osa] Nova enforces that no DB credentials are allowed for the nova-compute service On 11/23/20 11:42 AM, Balázs Gibizer wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
Cheers, gibi
In general, I would agree with this. However, because of the way cold migrations are working, nova compute boxes are authenticated between each other through ssh. You'd be limiting access to the db, but someone with nova or root ssh access could destroy all compute nodes. So, like it or not, it's game-over anyways. If you want to fix things, make it so that Nova doesn't require such ssh authentication anymore, because that's more urgent, *THEN* we can revisit this topic. Cheers, Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 3:47 AM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
I can agree with this in theory, however I don't think it's nova's responsibility to enforce this. IMHO a warning about this condition should be sufficient from a project standpoint. It's up to the operator to ensure this does not happen and not the project. The project can't foresee how the service is actually going to be deployed. In theory this is moot if the compute service is running on the same host as the api and not in something like a container. Escaping the service and having access to the host won't prevent the hacker from reading /etc/nova/nova-db.conf instead of /etc/nova/nova-compute.conf.
Cheers, gibi
Cheers,
Thomas Goirand (zigo)
On Mon, 2020-11-23 at 06:15 -0700, Alex Schultz wrote:
On Mon, Nov 23, 2020 at 3:47 AM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
I can agree with this in theory, however I don't think it's nova's responsibility to enforce this.
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services it activly breaks the nova compute agent if they are present. it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years. the fact it worked in some case does not change teh fact this was unsupported following a depercation cycle and activly depenedon in code after allowing operators, packagers and deployment tool maintianer years to ensure its no longer presnt. we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
IMHO a warning about this condition should be sufficient from a project standpoint. It's up to the operator to ensure this does not happen and not the project.
that would be true if it was not something that the code relied on to function correctly. local condocutor mode was removed in grizzly, since then the db creds have been unsued in the compute node. when cells v2 was intoduced they were used to determin if we would check the version in the local cell or in all cells as aprt of rpc automatic upgarde level calulation. we now always to the auto discovery which cause it to break.
The project can't foresee how the service is actually going to be deployed.
In theory this is moot if the compute service is running on the same host as the api and not in something like a container. not really we have expected nova-compute to not use nova.conf in an all in one deployment since rocky unless its in a container where it has a rendered version that only contains the section relevent to it. Escaping the service and having access to the host won't prevent the hacker from reading /etc/nova/nova-db.conf instead of /etc/nova/nova-compute.conf. it wont but /etc/nova/nova-db.conf should not be on the compute node unless you are also deploying a service that will actully use it there.
we can define which methods of deployment we will support however. that is vald to do but it shoudl still not be bassed to the nova-comptue binary.
Cheers, gibi
Cheers,
Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 6:42 AM Sean Mooney <smooney@redhat.com> wrote:
On Mon, 2020-11-23 at 06:15 -0700, Alex Schultz wrote:
On Mon, Nov 23, 2020 at 3:47 AM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
I can agree with this in theory, however I don't think it's nova's responsibility to enforce this.
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services it activly breaks the nova compute agent if they are present.
Seems like a poor choice to have made to use db creds to determine functionality but OK.
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years. the fact it worked in some case does not change teh fact this was unsupported following a depercation cycle and activly depenedon in code after allowing operators, packagers and deployment tool maintianer years to ensure its no longer presnt.
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
IMHO a warning about this condition should be sufficient from a project standpoint. It's up to the operator to ensure this does not happen and not the project.
that would be true if it was not something that the code relied on to function correctly. local condocutor mode was removed in grizzly, since then the db creds have been unsued in the compute node. when cells v2 was intoduced they were used to determin if we would check the version in the local cell or in all cells as aprt of rpc automatic upgarde level calulation. we now always to the auto discovery which cause it to break.
The project can't foresee how the service is actually going to be deployed.
we can define which methods of deployment we will support however.
No? I don't think that's ever been a thing for openstack services.
In theory this is moot if the compute service is running on the same host as the api and not in something like a container. not really we have expected nova-compute to not use nova.conf in an all in one deployment since rocky unless its in a container where it has a rendered version that only contains the section relevent to it.
file names were place holders. And since you don't control the deployment, you don't pick the names (see this thread)...
Escaping the service and having access to the host won't prevent the hacker from reading /etc/nova/nova-db.conf instead of /etc/nova/nova-compute.conf. it wont but /etc/nova/nova-db.conf should not be on the compute node unless you are also deploying a service that will actully use it there. that is vald to do but it shoudl still not be bassed to the nova-comptue binary.
You're missing the point if the operator is deploying nova-api and compute on the same host, they will be there. It may not be a best practice but it is something that people do for their own reasons. Nova should not artificially restrict this for no other reason than you think it shouldn't be done or you wrote code assuming this. The whole point of openstack was to allow folks to build their own clouds. If deployment decisions are being made at a project level now, then that seems to be a break in how things have been done historically. Having handled tooling around the deployment of openstack now for nearly 6 years, I'm not certain projects should necessarily be dictating this. I raised my concerns about this when this concept was first explained to me in #puppet-openstack and I've basically washed my hands of it. I disagree with this entire thing, but my take was if you're going to do it then nova developers needs to ensure it's supported in all the places and properly explained to operators which seems like the plan from this thread. I still don't think it's a good idea to hard fail but carry on.
Cheers, gibi
Cheers,
Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 6:42 AM Sean Mooney <smooney@redhat.com> wrote:
On Mon, 2020-11-23 at 06:15 -0700, Alex Schultz wrote:
On Mon, Nov 23, 2020 at 3:47 AM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor then limit how may other services get compromised outside of the compromised compute host.
I can agree with this in theory, however I don't think it's nova's responsibility to enforce this.
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services it activly breaks the nova compute agent if they are present.
Seems like a poor choice to have made to use db creds to determine functionality but OK.
On Mon, 2020-11-23 at 07:02 -0700, Alex Schultz wrote: the code in question use the config options to know if it can connect directly to the db to lookup the min service versionor if it must do an rpc via the conductor. for the api it would be ineffecnt for ti to call the conductor since it has direct db acess. for the compute nodes they have to call the conductor since they dont. the only other way to adress this woudl be to check the binary name or something else equally unclean.
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years. the fact it worked in some case does not change teh fact this was unsupported following a depercation cycle and activly depenedon in code after allowing operators, packagers and deployment tool maintianer years to ensure its no longer presnt.
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
IMHO a warning about this condition should be sufficient from a project standpoint. It's up to the operator to ensure this does not happen and not the project.
that would be true if it was not something that the code relied on to function correctly. local condocutor mode was removed in grizzly, since then the db creds have been unsued in the compute node. when cells v2 was intoduced they were used to determin if we would check the version in the local cell or in all cells as aprt of rpc automatic upgarde level calulation. we now always to the auto discovery which cause it to break.
The project can't foresee how the service is actually going to be deployed.
we can define which methods of deployment we will support however.
No? I don't think that's ever been a thing for openstack services.
In theory this is moot if the compute service is running on the same host as the api and not in something like a container. not really we have expected nova-compute to not use nova.conf in an all in one deployment since rocky unless its in a container where it has a rendered version that only contains the section relevent to it.
file names were place holders. And since you don't control the deployment, you don't pick the names (see this thread)...
Escaping the service and having access to the host won't prevent the hacker from reading /etc/nova/nova-db.conf instead of /etc/nova/nova-compute.conf. it wont but /etc/nova/nova-db.conf should not be on the compute node unless you are also deploying a service that will actully use it there. that is vald to do but it shoudl still not be bassed to the nova-comptue binary.
You're missing the point if the operator is deploying nova-api and compute on the same host, they will be there. It may not be a best practice but it is something that people do for their own reasons.
im not missing that point my point was if you wnat to do that its fine nova-compute should just not use the same config file as nova-api. yes its a deployment choice what the file names are but its was and has been a requirement that nova-compute should not have the db creds so if you have a service that needs them you had two choices to not violate that. 1 have nova-compute use its own config file e..g n-cpu.conf and only add the section that it needs 2 have nova-compute use nova.conf and have the second service use a secondary config file with there are two best practices with regards to the config 1 for each service to have its own config file with only the subset of info they require or 2 for nova.conf to have the general setting common to all services and the service config to have the addtional setting relevent only to them optionally factoring out possible shared info into extra config like nova-db.conf i personally have a very stong preferecne for each service having its own cofnig with a shared nothing approch on the filesystem and using templates to generage teh config. if you are doing it by had i see the other approch as vaild too where you compose multiple files each of which has a minimal subset of options for a singel feature set or service. the db creds are one example of this, the pci passthrough alias defintion which are shared between nova-comptue and the nova-api are another example. we are not going to stop supporting topologies our your freedom as an operator to deploy the service in a way that meets your needs however we do reserve the right to deprecate and remote options and either ignore or treat them as an error in the future. if we removed a commandline option form a cli we would warn and or treat that as an error in a similar way inline with our upgrade and deperacation proceduces.
Nova should not artificially restrict this for no other reason than you think it shouldn't be done or you wrote code assuming this. The whole point of openstack was to allow folks to build their own clouds. If deployment decisions are being made at a project level now, then that seems to be a break in how things have been done historically. Having handled tooling around the deployment of openstack now for nearly 6 years, I'm not certain projects should necessarily be dictating this.
I raised my concerns about this when this concept was first explained to me in #puppet-openstack and I've basically washed my hands of it. I disagree with this entire thing, but my take was if you're going to do it then nova developers needs to ensure it's supported in all the places and properly explained to operators which seems like the plan from this thread. I still don't think it's a good idea to hard fail but carry on.
Cheers, gibi
Cheers,
Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 07:02, Alex Schultz <aschultz@redhat.com> wrote:
On Mon, Nov 23, 2020 at 6:42 AM Sean Mooney <smooney@redhat.com> wrote:
On Mon, Nov 23, 2020 at 3:47 AM Balázs Gibizer <balazs.gibizer@est.tech> wrote:
On Mon, Nov 23, 2020 at 11:18, Thomas Goirand <zigo@debian.org>
wrote:
On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias,
That's not what I'm suggesting.
I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix.
Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over?
What are we trying to secure here? If that's what I'm
some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation.
I always understood this as having a goal to limit the attack surface. So if a VM escapes out of the sandbox and access the hypervisor
On Mon, 2020-11-23 at 06:15 -0700, Alex Schultz wrote: thinking (ie: then
limit how may other services get compromised outside of the compromised compute host.
I can agree with this in theory, however I don't think it's nova's responsibility to enforce this.
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services it activly breaks the nova compute agent if they are present.
Seems like a poor choice to have made to use db creds to determine functionality but OK.
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years. the fact it worked in some case does not change teh fact this was unsupported following a depercation cycle and activly depenedon in code after allowing operators, packagers and deployment tool maintianer years to ensure its no longer presnt.
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
IMHO a warning about this condition should be sufficient from a project standpoint. It's up to the operator to ensure this does not happen and not the project.
that would be true if it was not something that the code relied on to function correctly. local condocutor mode was removed in grizzly, since then the db creds have been unsued in the compute node. when cells v2 was intoduced they were used to determin if we would check the version in the local cell or in all cells as aprt of rpc automatic upgarde level calulation. we now always to the auto discovery which cause it to break.
The project can't foresee how the service is actually going to be deployed.
we can define which methods of deployment we will support however.
No? I don't think that's ever been a thing for openstack services.
In theory this is moot if the compute service is running on the same host as the api and not in something like a container. not really we have expected nova-compute to not use nova.conf in an all in one deployment since rocky unless its in a container where it has a rendered version that only contains the section relevent to it.
file names were place holders. And since you don't control the deployment, you don't pick the names (see this thread)...
Escaping the service and having access to the host won't prevent the hacker from reading /etc/nova/nova-db.conf instead of /etc/nova/nova-compute.conf. it wont but /etc/nova/nova-db.conf should not be on the compute node unless you are also deploying a service that will actully use it there. that is vald to do but it shoudl still not be bassed to the nova-comptue binary.
You're missing the point if the operator is deploying nova-api and compute on the same host, they will be there. It may not be a best practice but it is something that people do for their own reasons. Nova should not artificially restrict this for no other reason than you think it shouldn't be done or you wrote code assuming this. The whole point of openstack was to allow folks to build their own clouds. If deployment decisions are being made at a project level now, then that seems to be a break in how things have been done historically. Having handled tooling around the deployment of openstack now for nearly 6 years, I'm not certain projects should necessarily be dictating this.
To be fair with this change we are not dictating that nova-api and nova-compute cannot be deployed to the same host. We only dictate that they cannot use the same config file if they are deployed together. I don't see this (e.g. two binaries on the same host needs two separate config files) as a big restriction, but I might be mistaken. Also this is not the first time nova-compute refuse to start if provided with an invalid configuration. There are a bunch of those cases in the compute manager or in the virt driver e.g. [1]. [1] https://github.com/openstack/nova/blob/ab90c7af5626db190752528bba1d4982a8a0d...
I raised my concerns about this when this concept was first explained to me in #puppet-openstack and I've basically washed my hands of it. I disagree with this entire thing, but my take was if you're going to do it then nova developers needs to ensure it's supported in all the places and properly explained to operators which seems like the plan from this thread. I still don't think it's a good idea to hard fail but carry on.
Cheers, gibi
Cheers,
Thomas Goirand (zigo)
Hi Sean, Thanks for your post. On 11/23/20 2:32 PM, Sean Mooney wrote:
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services
I'm a bit shocked to read what I've read in this thread, about the double-guess that Nova is doing. The way the Nova team has been describing, this really looks like hacks to hide the internals of Nova, and what the team is asking, is more band-aid for it. Probably things have been done this way to make it easier for the users, but at this point, it feels like we shouldn't attempt to hide facts anymore, and try to have everything explicit, which is going the opposite way of what you describe. Why don't we go the other way around, and get things like a superconductor=true configuration directive, for example? On 11/23/20 2:32 PM, Sean Mooney wrote:
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years.
In such case, detect that it's the nova-compute agent that's running, detect that it has access to db creds, and either: - display a big warning (I would prefer that) - display an error and quit (maybe bad in the case of an all-in-one setup) This is orthogonal to the fact that Nova code is doing a hack (which should be fixed) to check which daemon is currently running in what mode. On 11/23/20 2:32 PM, Sean Mooney wrote:
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
Probably. But that shouldn't be upstream author's business on how things are deployed. IMO, in the case of an all-in-one, nova-compute should continue to work and just ignore the db params, and at worse display a huge warning on the logs. With the light of this thread, my opinion now has shifted to *not* have special files for the db credential, to give Nova a chance to tell the users what to do if nova-compute detects a mistake. If we push the creds in /etc/nova/nova-db.conf, it wont be loaded by nova-compute, and it wont be able to warn the user that the file shouldn't be there on a compute node. Checking for the file existence only would be wrong (because it could have empty values and just be there ... just because it's there! :) ). Hoping sharing my view is constructive and adding value to the thread, Cheers, Thomas Goirand (zigo)
Hi Sean,
Thanks for your post.
On 11/23/20 2:32 PM, Sean Mooney wrote:
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services
I'm a bit shocked to read what I've read in this thread, about the double-guess that Nova is doing.
The way the Nova team has been describing, this really looks like hacks to hide the internals of Nova, and what the team is asking, is more band-aid for it. Probably things have been done this way to make it easier for the users, but at this point, it feels like we shouldn't attempt to hide facts anymore, and try to have everything explicit, which is going the opposite way of what you describe.
Why don't we go the other way around, and get things like a superconductor=true configuration directive, for example?
On Mon, 2020-11-23 at 18:34 +0100, Thomas Goirand wrote: this would break upgrades for everyone one of our upgrade constratins is you should never have to update your config as part of an upgrade. where config changes are needed we always give you at least 1 cycle to update your configs as part of a seperate maintance window. it would also force all deployment tools and packages to chagne as we would have new portentially incompatible options in config files. spcificlly packagers would have to ship different nova config file for supper conductors and cell conductors for that usecase.
On 11/23/20 2:32 PM, Sean Mooney wrote:
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years.
In such case, detect that it's the nova-compute agent that's running, detect that it has access to db creds, and either: - display a big warning (I would prefer that) - display an error and quit (maybe bad in the case of an all-in-one setup)
This is orthogonal to the fact that Nova code is doing a hack (which should be fixed) to check which daemon is currently running in what mode.
it is not a hack, checking the binary name would be a hack as that is common code that shoudl be independnt of what name you give the file on disk. this was the where it would have failed from rocky on https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... if you used upgrade level auto but set the api db creds in teh config of the compute agent https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... if the api db options are set we get the min servie verion across all cells via a direct api call service_version = service_obj.get_minimum_version_all_cells( context.get_admin_context(), ['nova-compute']) and if they are not we do an indirect cal via the cell conductor to get the min compute service version in the local cell not that it is a remoteable method https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... so when invoked form the compute agent it is converted to an rpc to the conductor to execute the lookup. get_minimum_version_all_cells is not remoteable since calling it form a compute node would result in an upcall to iterate over all the top level cells https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... it was part of moving to cell v2 https://github.com/openstack/nova/commit/d779b33d72d6ef41651ce7b93fe982f121b... resolving https://bugs.launchpad.net/nova/+bug/1807044 the reason for the new failure is that we now uncontionally check the min compute node version using the same logic that we used before. https://github.com/openstack/nova/blob/dc93e3b510f53d5b2198c8edd22528f0c8996... if CONF.api_database.connection is not None: scope = 'system' current_service_version = service.get_minimum_version_all_cells( ctxt, ['nova-compute']) else: scope = 'cell' # We in a cell so target our query to the current cell only current_service_version = service.Service.get_minimum_version( ctxt, 'nova-compute') again using the presne of the db configurtion as a sentinal for is this code allowed to query the db directly. the patch that is under review to convert this to a warning just catches the excption for teh blocked direct db acess and logs the warning then execute the else branch https://review.opendev.org/c/openstack/nova/+/762175/1/nova/utils.py#1073 it is not a hack to rely on something that we have depened on for 2 years and we have stated for many more namely that the compute agent may not have the db options set.
On 11/23/20 2:32 PM, Sean Mooney wrote:
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
Probably. But that shouldn't be upstream author's business on how things are deployed. IMO, in the case of an all-in-one, nova-compute should continue to work and just ignore the db params, and at worse display a huge warning on the logs.
well there i dissagree the warning/error message is the bare minium we should do and at most we should allow ti to contiue executing and optimally it should stop the service sicne we know the config is invalid.
With the light of this thread, my opinion now has shifted to *not* have special files for the db credential, to give Nova a chance to tell the users what to do if nova-compute detects a mistake.
If we push the creds in /etc/nova/nova-db.conf, it wont be loaded by nova-compute, and it wont be able to warn the user that the file shouldn't be there on a compute node.
that is true we cant warn for a bug in the deployment too or in your packages but that is not something nova should have to do. we can warn when you invoke us with incorreect import but we should not have to check for other mistakes you have made.
Checking for the file existence only would be wrong (because it could have empty values and just be there ... just because it's there! :) ).
Hoping sharing my view is constructive and adding value to the thread, Cheers,
it is and just bacuase i dont agree with your view point does not mean i dont want to hear you express it as it chanlagnes me and other to constantly revialuate our postion based on the different persepctive shared :)
Thomas Goirand (zigo)
On 11/23/20 7:21 PM, Sean Mooney wrote:
Hi Sean,
Thanks for your post.
On 11/23/20 2:32 PM, Sean Mooney wrote:
nova need to enforce it as we use the absense or present of the db creads to know if common code is running in the compute agent or in controller services
I'm a bit shocked to read what I've read in this thread, about the double-guess that Nova is doing.
The way the Nova team has been describing, this really looks like hacks to hide the internals of Nova, and what the team is asking, is more band-aid for it. Probably things have been done this way to make it easier for the users, but at this point, it feels like we shouldn't attempt to hide facts anymore, and try to have everything explicit, which is going the opposite way of what you describe.
Why don't we go the other way around, and get things like a superconductor=true configuration directive, for example?
On Mon, 2020-11-23 at 18:34 +0100, Thomas Goirand wrote: this would break upgrades for everyone one of our upgrade constratins is you should never have to update your config as part of an upgrade.
There's one way to avoid this. Have 3 values for the config option: - None: keep old behavior (that would be the default) - superconductor - cellconductor
where config changes are needed we always give you at least 1 cycle to update your configs as part of a seperate maintance window.
What I'm proposing just above hopefully addresses your concern.
it would also force all deployment tools and packages to chagne as we would have new portentially incompatible options in config files.
There's no problem on the packaging level. Though truth, there would be some work on the config management modules, but that's what they are for.
spcificlly packagers would have to ship different nova config file for supper conductors and cell conductors for that usecase.
Simply put: no. In Debian, we could even provide a debconf prompt to address the question (I believe I would do that...). Currently, I don't know any OS that is taking care of doing config file updates anyway. That's something I've been thinking of for a long time, though it's not an easy thing to write (in a shell script), and everyone uses config management systems anyways.
On 11/23/20 2:32 PM, Sean Mooney wrote:
it is a bug to have the db cred in the set fo configs passed to nova-comptue and it has been for years.
In such case, detect that it's the nova-compute agent that's running, detect that it has access to db creds, and either: - display a big warning (I would prefer that) - display an error and quit (maybe bad in the case of an all-in-one setup)
This is orthogonal to the fact that Nova code is doing a hack (which should be fixed) to check which daemon is currently running in what mode. it is not a hack, checking the binary name would be a hack as that is common code that shoudl be independnt of what name you give the file on disk.
Right, it shouldn't be based on the filename on disk, but there are many other ways to implement this. Gibi wrote about having a global CONF value, that would work. Passing additional parameters to the part of the code that connects (or not) to the db would be another way (but probably harder to implement).
On 11/23/20 2:32 PM, Sean Mooney wrote:
we could make this just a ERROR log without the hard fail but that would still not change the fact there is a bug in packages or deployment tools that should be fixed.
Probably. But that shouldn't be upstream author's business on how things are deployed. IMO, in the case of an all-in-one, nova-compute should continue to work and just ignore the db params, and at worse display a huge warning on the logs. well there i dissagree the warning/error message is the bare minium we should do and at most we should allow ti to contiue executing and optimally it should stop the service sicne we know the config is invalid.
Worst case, this disagreement could be solved by a configuration value, on by default if you like: die_if_nova_compute_has_db_access=True :) I believe everyone will agree that it's super easy to change such a config value if you want to deploy all-in-one, and it's probably not much to ask to someone deploying in a non-standard way. Cheers, Thomas Goirand (zigo)
Sorry, It was not suppose to be a reply to your specifically but to thread as a whole. Best regards ________________________________ From: Thomas Goirand <zigo@debian.org> Sent: Monday, November 23, 2020 11:18:25 AM To: openstack maillist Subject: Re: [nova][tripleo][rpm-packaging][kolla][puppet][debian][osa] Nova enforces that no DB credentials are allowed for the nova-compute service On 11/23/20 9:30 AM, Tobias Urdin wrote:
Hello,
Just to clarify that this is already possible when using puppet-nova, it's up to the deployment to
make sure the database parameters for the classes is set.
We've been running without database credentials in nova.conf on our compute nodes for years.
Best regards
Tobias
Hi Tobias, That's not what I'm suggesting. I'm suggesting that nova-compute code from upstream simply ignores completely anything related to db connection, so we're done with the topic. That is, if nova-compute process having access to the db is the issue we're trying to fix. Or is it that the security problem is having the db credentials written in a file on the compute node? If so, isn't having hacked root (or nova) access to a compute node already game-over? What are we trying to secure here? If that's what I'm thinking (ie: some VM code to escape from guest, and potentially the hacker can gain access to the db), then IMO that's not the way to enforce things. It's not the role of upstream Nova to do this apart from a well enough written documentation. Cheers, Thomas Goirand (zigo)
On Sat, Nov 21, 2020 at 02:47, Thomas Goirand <zigo@debian.org> wrote:
On 11/18/20 8:24 PM, Dan Smith wrote:
which things are _not_allowed_ to be set for a service (such as db credentials on the compute).
I still don't understand why this is forbidden.
Sure, I understand what people wrote: that it is a security problem.
Can't nova-compute just *ignore* the db credentials, and then everyone is done with it, and moves on? That's a much more easy way to handle this problem, IMO.
It is still a security problem if nova-compute ignores the config as the config still exists on the hypervisor node (in some deployment scenarios) There are two existing technical reasons too: * Nova uses the [api_database]connection config to determine if the client of the compute RPC runs on the top level (i.e. super conductor) or locally in a cell (i.e. cell conductor or a nova-compute). [1] * The fairly recent change[2] that prevents older that N-1 services to start also depends on the [api_database]connection config to determine the scope (global vs. cell local) of the query that gathers the current service level of the cluster. I don't think that the [api_database]connection dependency can be removed from the super-conductor and cell-conductor differentiation perspective. From the nova-compute perspective we might be able to replace the [api_database]connection dependency with some hack. E.g to put the service name to the global CONF object at the start of the service binary and depend on that instead of other part of the config. But I feel pretty bad about this hack. Cheers, gibi [1] https://github.com/openstack/nova/blob/e16800cc0aebf1174c5c0b6c4b043b0962252... [2] https://github.com/openstack/nova/blob/e16800cc0aebf1174c5c0b6c4b043b0962252...
Cheers,
Thomas Goirand (zigo)
On 11/23/20 11:31 AM, Balázs Gibizer wrote:
It is still a security problem if nova-compute ignores the config as the config still exists on the hypervisor node (in some deployment scenarios)
Let's say we apply the patch you're proposing, and that nova-compute isn't loaded anymore with the db credentials, because it's on a separate file, and nova-compute doesn't load it. In such scenario, the /etc/nova/nova-db.conf could still be present with db credentials filled-in. So, the patch you're proposing is still not effective for wrong configuration of nova-compute hosts.
From the nova-compute perspective we might be able to replace the [api_database]connection dependency with some hack. E.g to put the service name to the global CONF object at the start of the service binary and depend on that instead of other part of the config. But I feel pretty bad about this hack.
Because of the above, I very much think it'd be the best way to go, but I understand your point of view. Going to the /etc/nova/nova-db.conf and nova-api-db.conf thing is probably good anyways. As for the nova-conductor thing, I very much would prefer if we had a clean and explicit "superconductor=true" directive, with possibly some checks to display big warnings in the nova-conductor.log file in case of a wrong configuration. If we don't have that, then at least things must be extensively documented, because that's really not obvious what's going on. Cheers, Thomas Goirand (zigo)
On Mon, Nov 23, 2020 at 13:47, Thomas Goirand <zigo@debian.org> wrote:
On 11/23/20 11:31 AM, Balázs Gibizer wrote:
It is still a security problem if nova-compute ignores the config as the config still exists on the hypervisor node (in some deployment scenarios)
Let's say we apply the patch you're proposing, and that nova-compute isn't loaded anymore with the db credentials, because it's on a separate file, and nova-compute doesn't load it.
In such scenario, the /etc/nova/nova-db.conf could still be present with db credentials filled-in. So, the patch you're proposing is still not effective for wrong configuration of nova-compute hosts.
Obviously we cannot prevent that the deployer stores the DB creds on a compute host as we cannot detect it in general. But we can detect it if it is stored in the config the nova-compute reads. I don't know why should we not make sure to tell the deployer not to do that as it is generally considered unsafe.
From the nova-compute perspective we might be able to replace the [api_database]connection dependency with some hack. E.g to put the service name to the global CONF object at the start of the service binary and depend on that instead of other part of the config. But I feel pretty bad about this hack.
Because of the above, I very much think it'd be the best way to go, but I understand your point of view. Going to the /etc/nova/nova-db.conf and nova-api-db.conf thing is probably good anyways.
As for the nova-conductor thing, I very much would prefer if we had a clean and explicit "superconductor=true" directive, with possibly some checks to display big warnings in the nova-conductor.log file in case of a wrong configuration. If we don't have that, then at least things must be extensively documented, because that's really not obvious what's going on.
I agree that superconductor=true would be a more explicit config option than [api_database]connection. However this would also enforce that deployers need a separate config file for nova-compute as there neither superconductor=true nor superconductor=false (meaning it is a cell conductor) make sense.
Cheers,
Thomas Goirand (zigo)
participants (11)
-
Alex Schultz
-
Balázs Gibizer
-
Dan Smith
-
Dmitriy Rabotyagov
-
Javier Pena
-
Oliver Walsh
-
Radosław Piliszek
-
Sean Mooney
-
Takashi Kajinami
-
Thomas Goirand
-
Tobias Urdin