different encryption key will be generated in each unit

Bug #1714157 reported by Nobuto Murata on 2017-08-31
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack heat charm
High
Alex Kavanagh

Bug Description

If I understand it correctly all units must use the same auth_encryption_key to achieve HA.

$ juju version
2.2.3-xenial-amd64

$ juju deploy heat -n3

$ juju run --application heat 'grep auth_encryption_key /etc/heat/heat.conf'
- Stdout: |
    auth_encryption_key = C4PmHbY3gbBT38wh
  UnitId: heat/0
- Stdout: |
    auth_encryption_key = nWWz3TYF9WgdB4LV
  UnitId: heat/1
- Stdout: |
    auth_encryption_key = jN8YPVyt76mX4WpL
  UnitId: heat/2

$ juju run --unit heat/0 'leader-get'
heat-domain-admin-passwd: YWHmqkfK7BKk8HT5jZGxrVM6xT4mg53z

-> no leader auth_encryption_key in leader storage

Workaround is to set "encryption-key" in charm config.

Alex Kavanagh (ajkavanagh) wrote :

It sounds like a bug; but I can't find any confirmation that they should all be the same. Do you have a link that indicates this (although it does seem obvious!).

Setting to low as there is a workaround.

Changed in charm-heat:
importance: Undecided → Low
status: New → Incomplete
Dmitrii Shcherbakov (dmitriis) wrote :

If it's AES and symmetric crypto in general you bet it must be the same on every stateless service instance.

https://github.com/openstack/heat/blob/stable/pike/heat/common/crypt.py#L37
https://github.com/openstack/heat/blob/stable/pike/doc/source/configuration/tables/heat-crypt.rst

I encountered this bug myself but haven't reported it. Heat is not usable without this in the HA configuration.

Changed in charm-heat:
status: Incomplete → Confirmed
tags: added: cpec
Dmitrii Shcherbakov (dmitriis) wrote :

There is a workaround for this:

If encryption-key config option is set:
https://github.com/openstack/charm-heat/blob/stable/17.08/config.yaml#L101-L104

the key is not randomly generated and will be the same on all units:
https://github.com/openstack/charm-heat/blob/stable/17.08/hooks/heat_context.py#L62-L67

A fix for a randomly generated one is in using juju leader storage in the charm code.

Alex Kavanagh (ajkavanagh) wrote :

> There is a workaround for this:

> If encryption-key config option is set:
> https://github.com/openstack/charm-heat/blob/stable/17.08/config.yaml#L101-L104

The original comment already stated this; hence why it was marked as low, because there is a workaround at present.

Ryan Beisner (1chb1n) on 2017-09-18
Changed in charm-heat:
importance: Low → Medium
Ante Karamatić (ivoks) wrote :

I don't think storing a secret in a bundle is a sensible workaround. I do agree it solves this problem, but it makes a bundle non shareable. It also requires that bundles are then put under tighter security reviews in some use cases.

I think we should avoid secrets in files and prioritize bugs like this higher than just 'low'.

Ryan Beisner (1chb1n) on 2017-09-18
Changed in charm-heat:
assignee: nobody → Alex Kavanagh (ajkavanagh)
Ryan Beisner (1chb1n) wrote :

We appreciate the valuable input and feedback.

My view is that the workaround is in line with existing tooling and bundle norms, given that other sensitive things like priv certs also exist in bundles, and are not fundamentally different than this. Having said that, I also think it is important that this just-works out of the box.

Re-triaged to Med. Let's address the issue in the charm, then evaluate for a stable charm update (backport).

Changed in charm-heat:
importance: Medium → High
Ryan Beisner (1chb1n) wrote :

s/Med/High/ :-)

James Page (james-page) wrote :

Just for some further context; I agree we should avoid secrets via configuration - the use of encryption-key here is due to the age of the charm in that it pre-dates the leadership features in Juju 1.25; the only way to reliably manage secrets across a clustered service prior to that was to inject via configuration.

As the charms now require Juju with leadership, this is no longer a blocker.

Changed in charm-heat:
milestone: none → 18.02
Alex Kavanagh (ajkavanagh) wrote :

Okay, so my plan is to implement this as a 32 char pwgen, put it in leader settings and have non-leaders wait for the password to appear, along with all the checking that leadership hasn't changed, etc. Essentially, this is already done with the 'heat-domain-admin-password' setting.

I've also noticed what might be a race in the leader_elected() call:

@hooks.hook('leader-elected')
def leader_elected():
    if is_leader() and not leader_get('heat-domain-admin-passwd'):
        leader_set({'heat-domain-admin-passwd': pwgen(32)})

There is a small possibility, I believe, that leader_set(..) could fail if another leader is elected between the call to is_leader() and leader_set(). It is small, but could lead to an error out on the hook. The retry would probably take care of the issue, although it would look ugly in the logs.

Fix proposed to branch: master
Review: https://review.openstack.org/506123

Changed in charm-heat:
status: Confirmed → In Progress
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers