-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
Enabled internal TLS between k8s pods by default #401
base: master
Are you sure you want to change the base?
Conversation
templates/configmaps_st2-urls.yaml
Outdated
{{- if and .Values.st2.tls.enabled .Values.st2auth.tls.enabled }} | ||
ST2_AUTH_URL: https://{{ .Release.Name }}-st2auth:9100/ | ||
{{- else }} | ||
ST2_AUTH_URL: http://{{ .Release.Name }}-st2auth:9100/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can reduce some of the repetition here by front loading the calculation:
{{- if and .Values.st2.tls.enabled .Values.st2auth.tls.enabled }} | |
ST2_AUTH_URL: https://{{ .Release.Name }}-st2auth:9100/ | |
{{- else }} | |
ST2_AUTH_URL: http://{{ .Release.Name }}-st2auth:9100/ | |
{{- $proto := "http" }} | |
{{- if and .Values.st2.tls.enabled .Values.st2auth.tls.enabled }} | |
{{- $proto := "https" }} | |
{{- end }} | |
ST2_AUTH_URL: {{ $proto }}://{{ .Release.Name }}-st2auth:9100/ |
and the same for the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh. I see you're changing the ports for st2api and st2stream as well. Hmm.
st2auth already exposes the tls bits. Could we add tls to st2api and st2stream as well to avoid adding another process to the mix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't follow - do you mean enhance the st2apit
and st2stream
libraries themselves to support TLS out-of-the-box instead of having to stick stunnel
infront of them on the k8s
end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't follow - do you mean enhance the
st2apit
andst2stream
libraries themselves to support TLS out-of-the-box instead of having to stickstunnel
infront of them on thek8s
end?
Yes. st2auth
supports tls, so st2api
and st2stream
should do the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{{- if and .Values.st2.tls.enabled .Values.st2auth.tls.enabled }} | |
ST2_AUTH_URL: https://{{ .Release.Name }}-st2auth:9100/ | |
{{- else }} | |
ST2_AUTH_URL: http://{{ .Release.Name }}-st2auth:9100/ | |
{{- $proto := "http" }} | |
{{- $port := "9100" }} | |
{{- if and .Values.st2.tls.enabled .Values.st2auth.tls.enabled }} | |
{{- $proto := "https" }} | |
{{- $port := "9110" }} | |
{{- end }} | |
ST2_AUTH_URL: {{ $proto }}://{{ .Release.Name }}-st2auth:{{ $port }}/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, it looks like we need to copy the st2auth ssl config opts to st2api and st2stream.
st2auth seems to define the ssl bits here:
The SSL bits would need to go in these st2api files:
The SSL bits would need to go in these st2stream files:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cognifloyd I've opened StackStorm/st2#6204 to add SSL/TLS support to api
and stream
if you could take a look that would be great :)
templates/ca.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fascinating. I should look into installing cert-manager again to see if that would be work in my cluster.
Right now, I'm using this chart to generate a CA+cert that I load into the st2web pods:
https://github.com/cognifloyd/helm-gen-self-signed-cert/
helm repo add self-signed-cert https://cognifloyd.github.io/helm-gen-self-signed-cert/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well for us, only issue we encounter is using self-signed certs, doesn't work - cert-manager
seems to forcibly sets isCA
false in the certificate (regardless if you tell it not to) which breaks providing the self-signed cert as a CA, to self validate the cert.
Instead you have to do what we do here and create a internal CA and perform "proper" cert signing and provide the CA then. Although this does have the benefit for corporate users where if they have a corporate CA, they can use that and have "correctly" signed certs - so I guess its a better implementation in the end.
I did want to avoid adding a hard requirement for cert-manager
- so I've left that as an optional feature, although I would recommend using it, since cert-manager
is open source and has no associated cost and does massively simplify certificate management 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that I forgot what I did 😅 - this does require cert-manager
nvm
71cd602
to
a9fef7b
Compare
ebd2243
to
d6fd653
Compare
0a7c28b
to
22bd9ba
Compare
values.yaml
Outdated
@@ -380,7 +389,19 @@ st2web: | |||
securityContext: {} # NB: nginx requires some capabilities, drop ALL will cause issues. | |||
# mount extra volumes on the st2web pod(s) (primarily useful for k8s-provisioned secrets) | |||
## Note that Helm templating is supported in 'mount' and 'volume' | |||
extra_volumes: [] | |||
extra_volumes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra_volumes is only for user-provided volumes so that if someone sets it to an empty list it doesn't break chart functionality.
22bd9ba
to
f7b456d
Compare
@cognifloyd thanks for the additional comments, I'll take a look at them on Monday, and I also have a go at updating st2api and stream to add TLS support then aswell 😊 |
0310b41
to
86aaa90
Compare
66a9a9e
to
664ef16
Compare
0e8bc77
to
6b5c2ec
Compare
6b5c2ec
to
911a419
Compare
Continuing on from the theme in (#400) this PR is another attempt at improving the security hardening of StackStorm in
k8s
.By default - all inter-pod communication between pods within a
k8s
deployment use non-encrypted protocols. (i.e. Betweenst2
andMongo
/RabbitMQ
andRedis
, and to the internalapi
,auth
andstream
endpoints withinst2
)This is fine if your security model is hard (secure) on the outside (public facing), but crunchy (insecure) on the inside (private facing) - (like an Armadillo 😉).
However, ideally a modern approach would be zero-trust and secure communication everywhere.
This PR therefore, enables by default, Encryption/TLS everywhere*. It makes use of cert-manager to enabled automatic generation of certificates, either using a user provided CA (to allow use of a corporate CA) or if the user already has a cert issuer setup - one can provide that.
TLS is enabled/configured globally for
st2
endpoints (api
,auth
,stream
) under thest2
block invalues.yaml
And then the individual endpoints can be toggled on/off under their respective blocks in
values.yaml
.Sadly the
mongo
andrabbitmq
TLS can't be setup from thest2
config directly - so they're present (and have defaults configured) under their respective blocks as well.The
st2
templates have then been updated to automatically configure all the required settings to use these certificates to enabled TLS - and the cert/key/ca are also automatically mounted to all the containers.Note since the
api
andstream
endpoints don't support encryption, we have added a TLS proxy layer (using ghostunnel) infront of them to enable inter-pod encryption while allowing unencrypted communication on the intra-pod commuication between thest2
component and the proxy layer.*Note that the OpenStack Tooz library, which
st2
uses to talk to Redis, doesn't have support to configure the settings needed to enable TLS when using Sentinel - so Redis traffic is still unencrypted in this PR. However, we have an internal build ofTooz
that adds this support, and we've also got the changes to the StackStorm helm template to support that working internally. Once we have our changes toTooz
merged upstream, we can provide a PR tostackstorm-k8s
andst2
core library to enable this support as well. (You can get a sneak preview of those changes here https://github.com/jk464/stackstorm-k8s/tree/feature/redis-tls)