different encryption key will be generated in each unit

[Nobuto Murata]

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)]

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.

[Dmitrii Shcherbakov (dmitriis)]

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.

[Dmitrii Shcherbakov (dmitriis)]

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)]

> 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.

[Ante Karamatić (ivoks)]

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)]

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).

[Ryan Beisner (1chb1n)]

s/Med/High/ :-)

[James Page (james-page)]

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.

[Alex Kavanagh (ajkavanagh)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/506123
Committed: https://git.openstack.org/cgit/openstack/charm-heat/commit/?id=275a8f61c69a435763f893c60335616db546423e
Submitter: Jenkins
Branch: master

commit 275a8f61c69a435763f893c60335616db546423e
Author: Alex Kavanagh <email address hidden>
Date: Thu Sep 21 12:11:17 2017 +0100

    Ensure auth_encryption_key is identical for all units

    This patch ensures that if multiple heat units are deployer, that each
    one will have the same auth_encryption_key in the /etc/heat/heat.conf.
    This is automatically generated on the (juju) leader and then remains
    unchanged for the application's duration. It can be overriden by the
    config setting 'encryption-key'.

    Testing is via the amulet 500 test (added) which checkes that the two
    units deployed have the same key.

    Change-Id: I89a11efe772314acd58ab9be21773eee89a23980
    Closes-Bug: #1714157

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/17.08
Review: https://review.openstack.org/507042

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/507042
Committed: https://git.openstack.org/cgit/openstack/charm-heat/commit/?id=a77b59dc2e72271c9bc7e82708ca5f36b6dd334b
Submitter: Jenkins
Branch: stable/17.08

commit a77b59dc2e72271c9bc7e82708ca5f36b6dd334b
Author: Alex Kavanagh <email address hidden>
Date: Thu Sep 21 12:11:17 2017 +0100

    Ensure auth_encryption_key is identical for all units

    This patch ensures that if multiple heat units are deployer, that each
    one will have the same auth_encryption_key in the /etc/heat/heat.conf.
    This is automatically generated on the (juju) leader and then remains
    unchanged for the application's duration. It can be overriden by the
    config setting 'encryption-key'.

    Testing is via the amulet 500 test (added) which checkes that the two
    units deployed have the same key.

    Change-Id: I89a11efe772314acd58ab9be21773eee89a23980
    Closes-Bug: #1714157
    (cherry picked from commit 275a8f61c69a435763f893c60335616db546423e)

[Dmitrii Shcherbakov (dmitriis)]

Should be fix-released for 17.08.