chore: ADR for creation and distribution of secrets

[bnevis-i]

Fixes #132

Signed-off-by: Bryon Nevis [email protected]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://wiki.edgexfoundry.org/display/FA/Contributor%27s+Guide
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

What is the current behavior?

Issue Number:

What has been updated?

Are there any specific instructions or things that should be known prior to reviewing?

Other information

[andresrinivasan]

I'm going to updating Redis to be consistent with

Dynamic injection of secret into process environment space

Can we change the recommendation to that?

[bnevis-i]

I'm going to updating Redis to be consistent with

Dynamic injection of secret into process environment space

Can we change the recommendation to that?

Before I do so, where is the documentation that that can be done. The documentation suggest that the only two options are config file and command line.

[hutchic]

I agree with the compliant / non-compliant designation but I think we should preface those decisions with what quantifies something as compliant / non-compliant.

I think a section we're missing is the addition of some processes going forward so we avoid regressing. Stand, walk, run: first version maybe as a wiki page?

I'm kind of surprised / disappointed there isn't an OWASP cheatsheet for this already. Closest I could find was OWASP/CheatSheetSeries#124

[hutchic]

the host system is trusted

Is this an OK posture to take?

[bnevis-i]

This is not an OK position to take if one is actually building a production system based on EdgeX. However, as a middleware it is impossible for EdgeX to know the environment into which it is being deployed, nor does EdgeX mandate an underlying infrastructure. This statement basically declares the host OS as out of scope.

[bnevis-i]

I agree with the compliant / non-compliant designation but I think we should preface those decisions with what quantifies something as compliant / non-compliant.

Added language to describe what these terms mean.

I think a section we're missing is the addition of some processes going forward so we avoid regressing. Stand, walk, run: first version maybe as a wiki page?

Please elaborate.

I'm kind of surprised / disappointed there isn't an OWASP cheatsheet for this already. Closest I could find was OWASP/CheatSheetSeries#124

Yes, it is a broad problem to solve in a generic way across the industry.

[hutchic]

I think a section we're missing is the addition of some processes going forward so we avoid regressing.

The document has a priority list of how to create / distribute secrets and an audit in time of now. Do we need a process in place such that going forward a secret doesn't get hard coded (PR template updated, scheduled audits, tooling to detect passwords being commited etc)?

[bnevis-i]

The document has a priority list of how to create / distribute secrets and an audit in time of now. Do we need a process in place such that going forward a secret doesn't get hard coded (PR template updated, scheduled audits, tooling to detect passwords being commited etc)?

I am open to suggestions, but I won't want to propose anything that I'm unwilling to do myself. (e.g. watch every PR)

[hutchic]

some naive questions:

• is the edgex secret store in security enabled mode only?
• is the edgex secret store backend hashicorp vault for all secrets or just some?

[bnevis-i]

• is the edgex secret store in security enabled mode only?

Yes.

• is the edgex secret store backend hashicorp vault for all secrets or just some?

Yes, the backend is Vault. It is not possible to store all secrets in Vault, but as many as possible should be stored in Vault.
As a counter example, the access token needed to connect to Vault itself cannot be stored in Vault. As a fallback mechanism when security is not enabled but a registry is used, secrets will be fetched from Consul. IMHO this is worse than just leaving them on the file system, as Consul is a networked service. However, security disabled in security disabled.

[jim-wang-intel]

LGTM

[jim-wang-intel]

Should this be deprecated since we are using Redis from now on by default?

[bnevis-i]

Should this be deprecated since we are using Redis from now on by default?

Hard to tell what lines were highlighted, but:

Postgres: Kong only works with Postgres and Cassandra not Redis so Postgres has to stay.
MongoDB: We reverted the PR to remove PW generation for MongoDB just in case someone wants to keep using it, so I think this should stay as well.

[jim-wang-intel]

maybe doable in Windows through WLS2 but that's in future work .

[jim-wang-intel]

will be deprecated i guess

[bnevis-i]

Not in Hanoi release, at least. Maybe Ireland.

[jpwhitemn]

All - if this is no longer applicable to Hanoi and can go in with Geneva documentation, is this ready for update/ approval / merge?

[bnevis-i]

All - if this is no longer applicable to Hanoi and can go in with Geneva documentation, is this ready for update/ approval / merge?

This ADR was taken as a Hanoi deliverable and it was my intention that this be an ADR to cover the Hanoi-scoped work. It seems good enough to commit, but it does declare some of the current Geneva implementation as non-complaint with the direction we want to go. I'd rather it be included as part of the Hanoi doc.

[jpwhitemn]

@bnevis-i - I am ok with this as part of Hanoi. Is it ok to put the Hanoi tag back on it so it doesn't get pulled into Geneva docs?

[bnevis-i]

@bnevis-i - I am ok with this as part of Hanoi. Is it ok to put the Hanoi tag back on it so it doesn't get pulled into Geneva docs?

@jpwhitemn Now I understand....I removed the label because the Milestone field is now synced across all the projects and we can use Milestone now in according with the issue management standard.

[hutchic]

Could/should we add a reference to ETSI EN 303 645 ( https://www.etsi.org/newsroom/press-releases/1789-2020-06-etsi-releases-world-leading-consumer-iot-security-standard ). Everything in this document is within the specification / guidelines of the ETSI document -- just a thought

[bnevis-i]

This is missing how we would deal with credentials to secure message bus, i.e. MQTT.

Added a section talking about cryptographic vs plain secrets, specifically in the context of MQTT.

Summary: if can support a certificate revokation list, use client TLS certificates, else use usernames and passwords.

[jpwhitemn]

@bnevis-i - this is a great document. Lays out a lot of detail, in a well organized way and provides as much of a roadmap as a state of the current services.

Should the app services "insecure secrets" idea also be included in this?? @lenny-intel for comment.
Should mention be given in current practices and decision? It sounds like device services may adopt some of this idea for MQTT and HTTP device services.

The idea behind insecure secrets "are required for Store and Forward DB access and for authenticated MQTT exports with new MQTTSecretSend function when not using security services, i.e. Vault"

Should this be standard practice and a common consideration?

[tonyespy]

Here you go @bnevis-i!

Also as a side note, should we include the fact that Consul is currently being run with no security in this ADR, or is that being tracked separately?

[tonyespy]

Are certificates really secrets? Aren't they merely signed documents that usually embed a public key which can be used to verify? That said, there are other PKI materials which are secret (e.g. private keys).

[bnevis-i]

Good catch. Will update to "PKI private keys"

[tonyespy]

Note, the password is actually generated by the snap install hook with a call to the "apg" (automatic-password-generator) command. This is then used to initialize the db with this password using psql. Then as noted, the install hook writes it to the file noted above, which in turn sets an environment variable (KONG_PG_PASSWORD) prior to launching kong, which allows kong to interact with the db.

In order to make this compliant, we'd need to do setup in security-proxy-setup, and then provide some means for Kong to read the password from Vault.

[bnevis-i]

The only thing needed to make it compliant is to cache the password in Vault instead of the file system. Will update to note exactly what part of the above flow is non-compliant.

[tonyespy]

Sure, but doing so implies enabling PostgreSQL to read the password from Vault, which it turns out might not be all that difficult:

https://www.vaultproject.io/docs/secrets/databases/postgresql

[bnevis-i]

I was thinking something simpler. Since the only user of Postgres is Kong, I was thinking to use "apg" to generate the password, store it in Vault, and then use it to configure Postgres at install time. For Kong, use Andre's security-secrets-read to read the password when starting Kong, and inject it into an environment variable.

The Vault postgres database secrets engine needs a superuser password configured before it can create user accounts. Would complicate things since it would then be necessary to generate a user account for Kong, and then install the schema under that account, and then use that account password. I haven't experimented with it, but it is not clear to me if the "create credential" call to the database secrets engine caches the generated credential automatically, or whether it needs to be explicitly stored. If the later, it would have just been simpler, or a single-user-DB, to not have bothered.

[tonyespy]

Can these not be store in Vault itelf? I thought the master Vault token was all we really needed to secure via HW?

[bnevis-i]

Can these not be store in Vault itelf? I thought the master Vault token was all we really needed to secure via HW?

Sadly, no.

Vault is incapable of unsealing its own PKI. This is because Vault implements its /sys/{un}seal API over HTTP(S), which means the PKI must be available to serve the REST endpoint prior to the Vault master key being supplied to Vault to unseal it. This means that not only must the Vault master key be protected (to protect all the secrets), but also the private key for Vault's REST endpoint.

Consul is in the same position so long as Vault is configured to use a Consul storage backend. (Vault needs consul for its persistent store, but consul ideally would get its PKI private key out of Vault, where it running.) The proposal is to break this dependency so that only Vault needs to be involved in solving the problems with Vault.

Although Vault presently does not support it, even the cheapest USB HSMs support hardware-protected private keys for TLS usage. For example, in Apache it is easy to specify an HSM-protected private key for TLS: (taken from here):

  SSLCertificateFile /etc/pki/tls/apachecert.pem
  SSLCertificateKeyFile "pkcs11:type=private?pin-value=121212;token=VendorPKCS11;id=%51"

[tonyespy]

OK, thanks for the explanation. Are you going to file a separate issue for this work or just fold it into the existing Vault master token impl (edgexfoundry/edgex-go#1919)? Is there an ADR for the HW protected storage of the Vault master token?

[bnevis-i]

Is there an ADR for the HW protected storage of the Vault master token?

The HW protected storage work is close to 18 months old at this point and predates adoption of ADRs. However, I do have a threat model that I can upstream to edgex-docs now that edgex-docs is now markdown-based.

Are you going to file a separate issue for this work?

I am debating this. There are many possible implementations, one of which could be to just pull the key out of Vault when the Vault is unsealed. Under this imlementation, it would be part of the bootstrapping ADR work. I am hesitant to file an issue for something that is at least a year out in implementation.

[tonyespy]

There's no concept of volumes in snaps. The PKI materials are all written to directories under $SNAP_DATA/currrent/secrets.

[bnevis-i]

There's no concept of volumes in snaps. The PKI materials are all written to directories under $SNAP_DATA/currrent/secrets.

I would like a suggestion for a name to use in place of "secrets volume."

If supported, the ideal place for the to-be-named location would be /run/user/id/snap.$SNAP_NAME/secrets, falling back to $SNAP_DATA/currrent/secrets if the former is not available.

[bnevis-i]

There's no concept of volumes in snaps. The PKI materials are all written to directories under $SNAP_DATA/currrent/secrets.

@tonyespy if you could come up with a short name that is "the place that secrets are stored in a snap" that would be great.

[bnevis-i]

I've called this SECRETSLOC in the document for lack of a better name.

[tonyespy]

I just confirmed that ca.pem, and the server.crt and server.key files are being re-generated every "cold restart of the framework" in the snap, so with default configuration it might make sense to change the SECRETSLOC to /run/snap.$SNAP_NAME/secrets (note the lack of /user/id in the path as this doesn't really make sense in the context of a snap).

That said, I'd really like to see us address the issue of external certificates as part of Hanoi, as our current self-generated root CA approach doesn't really work well in production environments (edgexfoundry/edgex-go#1926). If this were to be implemented, it wouldn't make sense to use a tmpfs based directory...

I also noticed that the server.crt and server.key files are the same for kong and vault within the snap, so this might be a bug in the configuration.

[bnevis-i]

I just confirmed that ca.pem, and the server.crt and server.key files are being re-generated every "cold restart of the framework" in the snap, so with default configuration it might make sense to change the SECRETSLOC to /run/snap.$SNAP_NAME/secrets (note the lack of /user/id in the path as this doesn't really make sense in the context of a snap).

I have done snap run --env hello-world followed by env command. Filtering for /run I get:

GPG_AGENT_INFO=/run/user/1000/gnupg/S.gpg-agent:0:1
SSH_AUTH_SOCK=/run/user/1000/keyring/ssh
PWD=/run
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus
XDG_RUNTIME_DIR=/run/user/1000/snap.hello-world
XAUTHORITY=/run/user/1000/gdm/Xauthority

(BTW: XDG_RUNTIME_DIR doesn't exist either)

What variable is supposed to map to /run/snap.hello-world

[bnevis-i]

That said, I'd really like to see us address the issue of external certificates as part of Hanoi, as our current self-generated root CA approach doesn't really work well in production environments (edgexfoundry/edgex-go#1926). If this were to be implemented, it wouldn't make sense to use a tmpfs based directory...

I think this is pretty simple to fix. I added language to the document. Summary: it's already in the secret store. Just add a flag to keep the framework from overwriting it and a utility to install one manually.

I also noticed that the server.crt and server.key files are the same for kong and vault within the snap, so this might be a bug in the configuration.

Hmmm. Let me know.

[tonyespy]

See previous comment about how this works in the snap. I'm not sure we have an alternative way to inject the env var.

[bnevis-i]

Rewrote this slightly. Randomize at install time is fine.

[tonyespy]

See my previous comment about PostgreSQL and Vault support.

[tonyespy]

Should we also speak to re-factoring the existing implementation which relies on an external security-secret-store-read command called from the Redis startup script?

[bnevis-i]

Will discuss with @andresrinivasan.

[andresrinivasan]

Currently security-secretstore-read is invoked as part of the security-secretstore-setup container and creates a file with the secret the Redis server should use for authentication. The Redis startup script reads that file and sets the secret.

Per edgex-go #2636, I will be changing the strategy to directly set the secret in the Redis server. This means security-secretstore-setup will have a dependency on the Redis container. I believe this conforms to Distribution and consumption of secrets/Recommended practices/2

[bnevis-i]

Per edgex-go #2636, I will be changing the strategy to directly set the secret in the Redis server. This means security-secretstore-setup will have a dependency on the Redis container. I believe this conforms to Distribution and consumption of secrets/Recommended practices/2

@andresrinivasan Hopefully this was meant to be the other way -- that redis requires the the secretstore-setup utility run first.

[andresrinivasan]

@bnevis-i and I just chatted. There are two options

1. Override the Redis Dockefile command to start Redis and then immediately run the app currently-known-as security-secretstore-read to read the secret from vault and set it directly in Redis using SET CONFIG REQUIREPASS Redis command.

2. Keep security-secretstore-read where it is now but have it create a config file with the secret stored on the secure file system and start Redis with both its own config file and the generated config file with the secret from the secure file system.

As we chatted, Bryon and I agreed that (1) felt better and was Snap friendly with sequential commands (@tonyespy). Furthermore, the last time I checked, having multiple config files while technically supported had some issue and I received glares from my Redis Labs colleagues when I demonstrated using this. I will check if this is still not recommended but unless someone objects, I'll be implementing (1).

[tonyespy]

One other potential option would be to implement a Redis plugin for the Vault secrets engine which already supports many different databases:

https://www.vaultproject.io/docs/secrets/databases

That said, I'm fine with (1) as a short-term approach.

[bnevis-i]

Vault database secrets plugins assume that the database has a persistent superuser account, such that Vault can use it to provision user accounts. The Redis passwords is not persistent, nor are multiple users supported. I don't think it's a good fit.

[andresrinivasan]

Redis 6 supports both multiple users and persistent ACLs. I hadn't thought of the vault plugin when I was thinking about how the secret injection would work. I was also going to upgrade EdgeX to Redis 6 as part of this exercise anyway; is there a preference to use the vault plugin (did I mention I haven't thought about that at all and I don't know what it entails).

[bnevis-i]

Redis 6 supports both multiple users and persistent ACLs. I hadn't thought of the vault plugin when I was thinking about how the secret injection would work. I was also going to upgrade EdgeX to Redis 6 as part of this exercise anyway; is there a preference to use the vault plugin (did I mention I haven't thought about that at all and I don't know what it entails).

I think it would be a great value add to have a Redis6 plugin for Vault, preferrably built-in, as we don't extend Vault in any way currently. As far as turning to a database secrets engine to create accounts -- that should be covered in the bootstrapping ADR.

[bnevis-i]

I added a separate treatment for Redis5 and Redis6.

[bnevis-i]

New revision posted.

[jpwhitemn]

LGTM

[andresrinivasan]

As an alternative to a file system and something kept in memory with optional persistence, Redis Labs uses Redis instances for this purpose. The runtime overhead of another Redis instance is a couple of MB and storing a few key/value pairs won't change that. Simple shell scripts can return the values.

[bnevis-i]

While the Redis Labs approach is good, many open source projects can only consume data from files, making it the lowest-common denominator. I will try to fit this into the description of secretsloc.

[bnevis-i]

@andresrinivasan The way I have explained this is that SECRETSLOC is the equivalent of 169.254.169.254 in the public cloud -- it delivers bootstrapping secrets.

[bnevis-i]

@tonyespy @andresrinivasan @hutchic New revision posted.

[bnevis-i]

Also as a side note, should we include the fact that Consul is currently being run with no security in this ADR, or is that being tracked separately?

I noted that in the most recent update. We can deal with Consul in the bootstrapping ADR as it is tangential to what I am trying to describe here.

[tonyespy]

Looking really good. A couple of minor changes requested.

[tonyespy]

I would get rid of this bullet, as XDG_RUNTIME_DIR is defined as a path to a user-private user-writable directory that is bound to the user login time on the machine. As such, it's not really applicable to services. Instead the recommendation is that /run/snap.$SNAP_INSTANCE_NAME be used instead. Note this directory is not auto-created for the snap, however the base apparmor policy allows writes to this directory by default. For more information on XDG_RUNTIME_DIR vs. /run/snap.$SNAP_INSTANCE_NAME see: https://bugs.launchpad.net/snapd/+bug/1802112

[bnevis-i]

New bullet reads "/run/snap.$SNAP_NAME/ (a tmpfs volume on a Linux host)"

[tonyespy]

Should this ADR mention that similar support for user-managed secrets should be added to the device SDKs?

[bnevis-i]

Should this ADR mention that similar support for user-managed secrets should be added to the device SDKs?

No opinion. Hard to write about when no design has been proposed. @lenny-intel do you input on the above?

[tonyespy]

This was done in Geneva, there's no longer a top-level pki dir under $SNAP_DATA.

[bnevis-i]

Removed.

[tonyespy]

manually-suppinstalledlied (sic)

[bnevis-i]

Clearly I meant to replace the entire word, not add one word inside of another. Fixed.

[tonyespy]

What's the difference between (5) and (6)? Is the latter for use when RedisStreams is configured as the message bus?

[bnevis-i]

Clarified this is v5 vs v6 and v6 adds multiple user support.

[tonyespy]

LGTM