allow externalizing/encrypting config secret values

[liggitt]

This PR allows externalizing/encrypting secret fields in the config. Previously, something like LDAP config had to do this:

  ...
  bindPassword: "Passw0rd!"
  ...

With this PR, it can do that, or do any of these:

$ export BIND_PASSWORD_ENV_VAR_NAME=Passw0rd!
$ echo -n "Passw0rd!" > bindPassword.txt
$ oadm ca encrypt --genkey=bindPassword.key --out=bindPassword.encrypted
> Data to encrypt: Passw0rd!

  bindPassword:
    env: BIND_PASSWORD_ENV_VAR_NAME

  bindPassword:
    file: bindPassword.txt

  bindPassword:
    file: bindPassword.encrypted
    keyFile: bindPassword.key

This PR adds support for these formats to the bindPassword and clientSecret fields in the master and LDAP group sync config files.

Docs in openshift/openshift-docs#1669

[liggitt]

[test][extended:ldap_groups]

[liggitt]

@deads2k for config/serialization

@sgallagher for encrypt/decrypt commands and output (encrypted using https://golang.org/pkg/crypto/x509/#EncryptPEMBlock)

@detiber / @sdodson for interactions with ansible... not sure how the bindPassword (for LDAP) and clientSecret (OAuth/OpenID) fields are handled by ansible if they could be either a string or a struct.

[deads2k]

This is a bad name. I like GetClearText better. An ExternalizableString isn't very helpful to me reading it.

[liggitt]

I'm open to better names for the type as well. StringSource? ResolveStringSource()? I could see using these for env-specific things in combination with an etcd-stored config as well

[deads2k]

I'm open to better names for the type as well. StringSource? ResolveStringSource()? I could see using these for env-specific things in combination with an etcd-stored config as well

Those are better.

[deads2k]

I'd feel a lot better about this with an extended test using an actual master-config.yaml on disk with an encrypted value. Gitlab in a container could be generally useful, so could a secured ldap. I could see this as a followup issue for someone who wants to get to know the config and authentication integration scenarios.

[sdodson]

Just curious, is there a card or something that outlines the intention here?

[liggitt]

@sdodson: https://trello.com/c/Wz0KUuIr/626-allow-externalizing-encrypting-secret-values-in-config-files

[deads2k]

Need docs for the types, but otherwise I'm happy with them.

I'd still a test or issue for a test that actually makes use of this from a yaml file.

@sgallagher I didn't look at the command in any detail.

[liggitt]

@deads2k not quite from yaml on disk, but this is close: https://github.com/openshift/origin/pull/7608/files#diff-7e8d11670ba88f9227b91aeea7c786ba

did I miss an easy way to make the integration stuff persist/read the master config from disk?

[liggitt]

nevermind, got it round-tripping in the integration test at https://github.com/openshift/origin/pull/7608/files#diff-7e8d11670ba88f9227b91aeea7c786baR135

[liggitt]

tests and doc added, any last comments from @deads2k or @sgallagher?

[liggitt]

will open a docs PR separately

[liggitt]

open questions:

1. when reading cleartext data from stdin or from a file, should we strip a trailing newline? echo foo > secret.txt leaves a trailing newline in the file, which is unfortunate added warnings
2. Are there better names for the PEM blocks than what I used ("ENCRYPTED STRING" / "ENCRYPTING KEY")?
3. Will ansible tolerate idp config snippets that have structured data for bindPassword or clientSecret? looks like it just plumbs the value through, whatever it is

[deads2k]

two open questions:

Three or is one of those just a test? Oh, you need the trailing whitespace/newline to support entering usernames, don't you? :)

I don't see any of those problems as blockers for this pull.

[deads2k]

@sgallagher you going to go through the command?

The types looked fine to me.

[openshift-bot]

continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/1733/) (Extended Tests: ldap_groups)

[sgallagher]

LGTM

[liggitt]

[merge]

[liggitt]

flake #7706
re[merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5148/) (Image: devenv-rhel7_3575)

[openshift-bot]

Evaluated for origin test up to 70fd4c7

[openshift-bot]

Evaluated for origin merge up to 70fd4c7