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.

Sign up for GitHub

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

Generate datastore_crypto_key on install if not provided #266

Merged
merged 18 commits into from
Dec 3, 2021
Merged

Generate datastore_crypto_key on install if not provided #266

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2845c03
allow structured .Values.st2.datastore_crypto_key
cognifloyd Nov 11, 2021
1671330
generate datastore_crypto_key on install if not defined
cognifloyd Nov 11, 2021
f898ebd
typo fix
cognifloyd Nov 11, 2021
6a83c12
update helm to use randBytes from sprig 3.2+
cognifloyd Nov 11, 2021
b76fe6d
newer version of helm
cognifloyd Nov 11, 2021
98ee61e
fix template quotes
cognifloyd Nov 11, 2021
7f2fd1d
reduce escaping in json
cognifloyd Nov 11, 2021
4fba001
template default_datastore_crypto_key via yaml
cognifloyd Nov 11, 2021
ea8b857
add note about minimum required helm version
cognifloyd Nov 11, 2021
b99913b
add changelog entry
cognifloyd Nov 11, 2021
61f5a2c
datastore_crypto_key is always required now
cognifloyd Nov 12, 2021
bed1b47
move default_datastore_crypto_key from values to conf file
cognifloyd Nov 12, 2021
bb37eaa
simplify datastore_crypto_key selection logic
cognifloyd Nov 12, 2021
b6ddc52
Merge branch 'master' into generate-crypto-key
cognifloyd Nov 25, 2021
742d1ba
Merge branch 'master' into generate-crypto-key
cognifloyd Dec 2, 2021
ad8a4f8
Revert "datastore_crypto_key is always required now"
cognifloyd Dec 3, 2021
2d44709
Allow disabling the datastore_crypto_key
cognifloyd Dec 3, 2021
bdf62be
handle unset datastore_crypto_key so ne can compare strings
cognifloyd Dec 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
helm-lint:
working_directory: ~/stackstorm-ha
docker:
- image: lachlanevenson/k8s-helm:v3.4.2
- image: lachlanevenson/k8s-helm:v3.5.3
steps:
- checkout
- run:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* Template `~/.st2/config`. This allows customizing the settings used by the `st2client` and jobs pods for using the st2 apis. (#262) (by @cognifloyd)
* Fix indent for lifecycle postStart hook of `st2web` pod. (#268) (by @cognifloyd)
* Advanced Feature: Allow `st2web` to serve HTTPS when the ssl certs are provided via `st2web.extra_volumes`. To enable this, add `ST2WEB_HTTPS: "1"` to `st2web.env` in your values file. (#264) (by @cognifloyd)
* Auto-generate `datastore_crypto_key` on install if not provided. This way all HA installs will have a datastore_crypto_key configured. (#266) (by @cognifloyd)

## v0.70.0
* New feature: Shared packs volumes `st2.packs.volumes`. Allow using cluster-specific persistent volumes to store packs, virtualenvs, and (optionally) configs. This enables using `st2 pack install`. It even works with `st2packs` images in `st2.packs.images`. (#199) (by @cognifloyd)
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ It's more than welcome to fine-tune each component settings to fit specific avai

## Requirements
* [Kubernetes](https://kubernetes.io/docs/setup/pick-right-solution/) cluster
* [Helm](https://docs.helm.sh/using_helm/#install-helm) `v3.x`
* [Helm](https://docs.helm.sh/using_helm/#install-helm) `v3.5` or greater

## Usage
1) Edit `values.yaml` with configuration for the StackStorm HA K8s cluster.
Expand Down
13 changes: 13 additions & 0 deletions conf/datastore_crypto_key.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# This is used to generate st2.datastore_crypto_key on install if not defined in values.

# The formula is based on an st2-specific version of python's base64.urlsafe_b64encode
# randBytes returns a base64 encoded string
# 32 bytes = 256 bits / 8 bits/byte

aesKeyString: '{{ randBytes 32 | replace "+" "-" | replace "_" "/" | replace "=" "" }}'
mode: CBC
size: 256

hmacKey:
hmacKeyString: '{{ randBytes 32 | replace "+" "-" | replace "_" "/" | replace "=" "" }}'
size: 256
Comment on lines +1 to +13
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's making the impossible possible :)

Overall I'm worried about the implementation and if that's a good way or not to try this in the chart really.
Even the slightest security risk behind the implementation/diff is sufficient to avoid the drill here generating the K/V crypto key.

I'd rely on someone better from the @StackStorm/tsc with security to review this. Maybe @punkrokk ?

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 there is a security issue with this formula, then st2-core would need to change, and every installation everywhere would have to update their datastore crypto key. This chart would, of course, also have to be updated to match whatever formula st2-core uses to generate these keys. I think the formula is easier to understand here than in st2-core, so it should be fairly simple to migrate this if ever needed in the future.

That said, I look forward to hearing what @punkrokk or other @StackStorm/TSC members have to say about this.

Copy link
Member

Choose a reason for hiding this comment

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

No worries. To be clear, I'm good with this new feature itself.

However, I'm looking for feedback if the key generation here is cryptographically secure. I didn't look what st2 does under the hood, but I'd trust those who good at this and if trying to mimic the st2 behavior for st2-krypto-key-generation in the Helm template engine is good enough.

I think the folks would need to dig deeper into the code you provided 👍

2 changes: 0 additions & 2 deletions templates/configmaps_st2-conf.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ data:
{{- end }}
port = {{ index .Values "mongodb" "service" "port" }}
{{- end }}
{{- if .Values.st2.datastore_crypto_key }}
[keyvalue]
encryption_key_path = /etc/st2/keys/datastore_key.json
{{- end }}
{{- if .Values.st2.rbac.enabled }}
[rbac]
enable = True
Expand Down
28 changes: 0 additions & 28 deletions templates/deployments.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,9 @@ spec:
name: {{ .Release.Name }}-st2-urls
volumeMounts:
{{- include "st2-config-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- include "packs-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.packs.volumes.enabled }}
{{- include "pack-configs-volume-mount" . | nindent 8 }}
Expand All @@ -222,14 +220,12 @@ spec:
serviceAccountName: {{ template "stackstorm-ha.serviceAccountName" . }}
{{- end }}
volumes:
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- include "st2-config-volume" . | nindent 8 }}
{{- include "packs-volumes" . | nindent 8 }}
{{- if .Values.st2.packs.volumes.enabled }}
Expand Down Expand Up @@ -550,11 +546,9 @@ spec:
name: {{ .Release.Name }}-st2-urls
volumeMounts:
{{- include "st2-config-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- if .Values.st2rulesengine.postStartScript }}
- name: st2-post-start-script-vol
mountPath: /post-start.sh
Expand All @@ -571,14 +565,12 @@ spec:
{{- end }}
volumes:
{{- include "st2-config-volume" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- if .Values.st2rulesengine.postStartScript }}
- name: st2-post-start-script-vol
configMap:
Expand Down Expand Up @@ -759,11 +751,9 @@ spec:
name: {{ .Release.Name }}-st2-urls
volumeMounts:
{{- include "st2-config-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- range .Values.st2workflowengine.extra_volumes }}
- name: {{ required "Each volume must have a 'name' in st2workflowengine.extra_volumes" .name }}
{{- tpl (required "Each volume must have a 'mount' definition in st2workflowengine.extra_volumes" .mount | toYaml) $ | nindent 10 }}
Expand All @@ -784,14 +774,12 @@ spec:
{{- end }}
volumes:
{{- include "st2-config-volume" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- range .Values.st2workflowengine.extra_volumes }}
- name: {{ required "Each volume must have a 'name' in st2workflowengine.extra_volumes" .name }}
{{- tpl (required "Each volume must have a 'volume' definition in st2workflowengine.extra_volumes" .volume | toYaml) $ | nindent 10 }}
Expand Down Expand Up @@ -877,11 +865,9 @@ spec:
name: {{ .Release.Name }}-st2-urls
volumeMounts:
{{- include "st2-config-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- if .Values.st2scheduler.postStartScript }}
- name: st2-post-start-script-vol
mountPath: /post-start.sh
Expand All @@ -897,14 +883,12 @@ spec:
serviceAccountName: {{ template "stackstorm-ha.serviceAccountName" . }}
{{- end }}
volumes:
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- include "st2-config-volume" . | nindent 8 }}
{{- if .Values.st2scheduler.postStartScript }}
- name: st2-post-start-script-vol
Expand Down Expand Up @@ -1131,11 +1115,9 @@ spec:
volumeMounts:
{{- include "st2-config-volume-mounts" $ | nindent 8 }}
{{- include "packs-volume-mounts" $ | nindent 8 }}
{{- if $.Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- range $sensor.extra_volumes }}
- name: {{ required "Each volume must have a 'name' in $sensor.extra_volumes" .name }}
{{- tpl (required "Each volume must have a 'mount' definition in $sensor.extra_volumes" .mount | toYaml) $ | nindent 10 }}
Expand All @@ -1155,14 +1137,12 @@ spec:
serviceAccountName: {{ template "stackstorm-ha.serviceAccountName" $ }}
{{- end }}
volumes:
{{- if $.Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ $.Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- include "st2-config-volume" $ | nindent 8 }}
{{- include "packs-volumes" $ | nindent 8 }}
{{- range $sensor.extra_volumes }}
Expand Down Expand Up @@ -1270,11 +1250,9 @@ spec:
{{- include "st2-config-volume-mounts" . | nindent 8 }}
- name: st2-ssh-key-vol
mountPath: {{ tpl .Values.st2.system_user.ssh_key_file . | dir | dir }}/.ssh-key-vol/
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- include "packs-volume-mounts" . | nindent 8 }}
{{- if .Values.st2.packs.volumes.enabled }}
{{- include "pack-configs-volume-mount" . | nindent 8 }}
Expand All @@ -1296,14 +1274,12 @@ spec:
serviceAccountName: {{ template "stackstorm-ha.serviceAccountName" . }}
{{- end }}
volumes:
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- include "st2-config-volume" . | nindent 8 }}
- name: st2-ssh-key-vol
secret:
Expand Down Expand Up @@ -1542,11 +1518,9 @@ spec:
mountPath: /root/.st2/
- name: st2-ssh-key-vol
mountPath: {{ tpl .Values.st2.system_user.ssh_key_file . | dir | dir }}/.ssh-key-vol/
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
mountPath: /etc/st2/keys
readOnly: true
{{- end }}
{{- include "packs-volume-mounts" . | nindent 8 }}
{{- include "pack-configs-volume-mount" . | nindent 8 }}
{{- range .Values.st2client.extra_volumes }}
Expand All @@ -1569,14 +1543,12 @@ spec:
memory: "5Mi"
cpu: "5m"
volumes:
{{- if .Values.st2.datastore_crypto_key }}
- name: st2-encryption-key-vol
secret:
secretName: {{ .Release.Name }}-st2-datastore-crypto-key
items:
- key: datastore_crypto_key
path: datastore_key.json
{{- end }}
{{- include "st2-config-volume" . | nindent 8 }}
{{- if .Values.st2.rbac.enabled }}
- name: st2-rbac-roles-vol
Expand Down
13 changes: 10 additions & 3 deletions templates/secrets_datastore_crypto_key.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
{{- $deprecated_crypto_key := (default (dict) (default (dict) .Values.secrets).st2).datastore_crypto_key }}
{{- if $deprecated_crypto_key }}
{{- fail "Please update your values! The datastore_crypto_key value moved from secrets.st2.* to st2.*" }}
{{- else if .Values.st2.datastore_crypto_key }}
{{- end }}
---
apiVersion: v1
kind: Secret
metadata:
name: {{ .Release.Name }}-st2-datastore-crypto-key
{{- $name := print .Release.Name "-st2-datastore-crypto-key" }}
name: {{ $name }}
annotations:
description: StackStorm crypto key used to encrypt/decrypt KV records
labels:
Expand All @@ -20,6 +21,12 @@ metadata:
type: Opaque
data:
# Datastore key used to encrypt/decrypt record for the KV store
{{- $previous := lookup "v1" "Secret" .Release.Namespace $name }}
{{- if .Values.st2.datastore_crypto_key }}
datastore_crypto_key: {{ .Values.st2.datastore_crypto_key | b64enc }}

{{- else if $previous }}
datastore_crypto_key: {{ $previous.data.datastore_crypto_key }}
{{- else }}
datastore_crypto_key: {{ tpl (.Files.Get "conf/datastore_crypto_key.yaml") . | fromYaml | toRawJson | b64enc }}
{{- end }}

3 changes: 2 additions & 1 deletion values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ st2:
#password: Ch@ngeMe
# ST2 crypto key for the K/V datastore.
# See https://docs.stackstorm.com/datastore.html#securing-secrets-admin-only for more info.
# Warning! Replace with your own generated key!
# If set, st2.datastore_crypto_key always overrides any existing datastore_crypto_key.
# If not set, the datastore_crypto_key is auto-generated on install and preserved across upgrades.
#datastore_crypto_key: >-
# {"hmacKey": {"hmacKeyString": "", "size": 256}, "size": 256, "aesKeyString": "", "mode": "CBC"}
# SSH private key for the 'stanley' system user ('system_user.ssh_key_file' in st2.conf)
Expand Down