Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secrets support for configuration files #2312

Merged
merged 4 commits into from
Jun 25, 2016
Merged

Conversation

kellerza
Copy link
Member

Description:
Rework of #2262, now based on yaml.py

Option to split secrets from configuration files. Secrets can be stored in (one or both):

  • secrets.yaml (in same folder as configuration.yaml)
  • keyring (optional, only if dep installed)**

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#

Example entry for configuration.yaml (if applicable):

http:
  api_key: !secret http_key

this secret can be store in secrets.yaml

debug: 0
http_key: mykey

or in keyring

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@Khabi
Copy link
Contributor

Khabi commented Jun 17, 2016

+1 I started working on something extremely similar, but looks like you beat me to the punch. :)

@kellerza
Copy link
Member Author

@Khabi I was a bit paranoid with my icloud password. Ideally it should be a app-specific password, but I'll probably have to look into pyicloud to see why that didn't work for me.

If you can help test this branch it would be great. You can add keyring password by executing util/secrets.py


if SECRET_DICT and secret_name in SECRET_DICT:
HISTORY[secret_name] = SSource.yaml
_LOGGER.debug('from secrets.yaml: %s = "%s"', secret_name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not print secrets to the debug log

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok if we only print where it was resolved from? secret.yaml / keyring

@kellerza
Copy link
Member Author

@balloob is it possible to get the hass instance from the loader?

I currently check which keys in the secrets file was not used, but can only do that after home assistant started. Without hass I'm not sure how to bind to this event?

@kellerza
Copy link
Member Author

I removed reporting on unused secrets for now.

@balloob
Copy link
Member

balloob commented Jun 25, 2016

Nice! I think that this is a great improvement and keeps the change manageable.

🐬 ㊙️ 🔒 🔑 🍻 ⚽

@balloob balloob merged commit 7b02dc4 into home-assistant:dev Jun 25, 2016
@balloob
Copy link
Member

balloob commented Jun 27, 2016

Just realized this PR brought us to a 1000 tests total!

giphy

@balloob
Copy link
Member

balloob commented Jul 1, 2016

We still need some documentation for this.

@fabaff
Copy link
Member

fabaff commented Jul 1, 2016

@fabaff
Copy link
Member

fabaff commented Jul 1, 2016

I'm not really able to get the keyring to work at first glance.

$ keyring-3.4 set homeassistant http_password
Password for 'http_password' in 'homeassistant': 12345

and

http:
  api_password: !secret http_password

gives me:

16-07-01 12:27:20 ERROR (MainThread) [homeassistant.util.yaml] Secret http_password not defined.

The password is available in the keyring.

$ keyring-3.4 get homeassistant http_password
12345

_LOGGER.debug('Secret %s retrieved from keyring.', node.value)
return pwd

_LOGGER.error('Secret %s not defined.', node.value)
Copy link
Member

@fabaff fabaff Jul 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that here it would be helpful if message provide some more details because the issue can be at various places (no secrets.yaml file, no keyring, no access to the keyring, or no entry).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add logger: debug to secrets.yaml, debug messages will get printed

@fabaff
Copy link
Member

fabaff commented Jul 1, 2016

It seems that the commandline tool works different.

>>> import keyring
>>> keyring.get_keyring()
<EncryptedKeyring at /home/fab/.local/share/python_keyring/crypted_pass.cfg>
>>> keyring.set_password("homeassistant", "http_password", "12345")
Please set a password for your new keyring: 
Please confirm the password: 
>>> keyring.get_password("homeassistant", "http_password")
'12345'

Now it works... HA start after the password for the keyring is provided.

@kellerza do you have a different experience with the command-line tool?

@kellerza
Copy link
Member Author

kellerza commented Jul 1, 2016

@fabaff guess you must be using the same user, same venv etc to get the same keyring file. Its a pity the keyring cmdline util cant print out keyring.get_keyring()

Mine works without any issues, just tested with a new secret.

Do you get prompted to enter your keyring password when you start hass?

@kellerza kellerza deleted the keyring2 branch July 1, 2016 20:04
@kellerza
Copy link
Member Author

kellerza commented Jul 3, 2016

@fabaff maybe we should add a keyring script? #2426

@balloob
Copy link
Member

balloob commented Jul 3, 2016

Good idea, and a validate config script please ;-)

@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants