Hardened Security Context for Elasticsearch #6703

barkbay · 2023-04-19T08:39:35Z

If Elasticsearch configuration files are copied before symbolic links are created, it is possible to run all the Elasticsearch containers with the following restricted securityContext

    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        drop:
        - ALL
      privileged: false
      readOnlyRootFilesystem: true

Fixes #6126

See my comment here if you want to review this PR: #6703 (comment)

barkbay · 2023-04-19T09:22:51Z

buildkite test this -f p=gke,s=8.7.0,s=7.17.8

thbkrkr · 2023-04-19T13:25:09Z

buildkite test this -f p=gke,s=8.7.0,s=7.17.8

This will only run the e2e tests with s=7.17.8. When you want to test one variable with different values (here s the stack version), you need to declare this variable and its values in the --mixed|-m flag.

buildkite test this --fixed p=gke --mixed s=8.7.0,s=7.17.8
buildkite test this -f p=gke -m s=8.7.0,s=7.17.8 (with the short flags)

barkbay · 2023-04-19T13:40:16Z

@thbkrkr Thanks! I think I spotted a problem with this PR, I will use your advice next time 🙇

barkbay · 2023-04-20T07:29:56Z

TestVolumeEmptyDir failed because this "feature" is itself broken: emptyDir volume is not mounted #6186. I added a function to detect, on a best effort basis, whether or not data are in a volume.
TestKibanaStandalone also failed, but I'm not sure to understand why 🤷

thbkrkr · 2023-04-20T07:59:59Z

TestKibanaStandalone looks very similar to #6693 with all sub-tests failing as we don't have the end of the test execution in the log because the log stream was interrupted.

log

{
    "Time": "2023-04-19T17:16:36.671150963Z",
    "Action": "output",
    "Package": "github.com/elastic/cloud-on-k8s/v2/test/e2e/kb",
    "Test": "TestKibanaStandalone/ES_HTTP_certificate_authority_should_be_set_and_deployed",
    "Output": "=== RUN   TestKibanaStandalone/ES_HTTP_certificate_authority_should_be_set_and_deployed\n"
}
{
    "log.level": "info",
    "@timestamp": "2023-04-19T17:30:05.540Z",
    "log.logger": "e2e-pdzcj",
    "message": "Log stream ended",
    "service.version": "0.0.0-SNAPSHOT+00000000",
    "service.type": "eck",
    "ecs.version": "1.4.0",
    "pod_name": "eck-e2e-pdzcj-cl86s"
}
{
    "log.level": "info",
    "@timestamp": "2023-04-19T17:30:05.540Z",
    "log.logger": "e2e-pdzcj",
    "message": "Streaming pod logs",
    "service.version": "0.0.0-SNAPSHOT+00000000",
    "service.type": "eck",
    "ecs.version": "1.4.0",
    "pod_name": "eck-e2e-pdzcj-cl86s"
}

barkbay · 2023-04-20T08:42:55Z

buildkite test this --fixed p=gke --mixed s=8.7.0,s=7.17.8

barkbay · 2023-04-20T08:57:19Z

buildkite test this -f p=gke -m s=8.7.0,s=7.17.8

barkbay · 2023-04-21T05:53:46Z

Failures to investigate from the last e2e tests session:

I think this is because I added a check to validate that all the containers in the Elasticsearch Pod are running with the expected securityContext. That's probably not the case for Beat sidecars when stack monitoring is enabled.

barkbay · 2023-04-21T14:01:33Z

buildkite test this -f p=gke -m s=8.7.0,s=7.17.8

barkbay · 2023-04-23T16:56:16Z

buildkite test this -f p=gke -m s=8.7.0,s=7.17.8

barkbay · 2023-04-24T06:17:12Z

buildkite test this -f p=gke -m s=8.0.1,s=8.8.0-SNAPSHOT

….Drop

barkbay · 2023-04-24T08:27:01Z

Tests passed for 8.7.0, 7.17.8 on GKE 🎉
I started an e2e test session on 8.8.0-SNAPSHOT for which RunAsNonRoot is expected to be true thanks to Use UID for dockerfile to allow runAsNonRoot to be used. elasticsearch#95390
I performed preliminary tests on OpenShift and things seem to work as expected.

Some "noteworthy" things if you want to review this PR:

All stack monitoring sidecar containers (both Metricbeat and Filebeat) now have their /usr/share/metricbeat/data directory mounted in an emptyDir to allow readOnlyRootFilesystem to be true.
However, only Elasticsearch sidecar containers have their securityContext updated. I haven't had time to do extensive testing on other components, I think that if we want to target 2.8.0 it is safer to limit this change to Elasticsearch.
ReadOnlyRootFilesystem is set to true iif it is obvious from the operator point of view that data are written in a volume: https://github.com/elastic/cloud-on-k8s/pull/6703/files#diff-0e1da5b5ab6c4b7fa20fcc3151370567119207c7a353e1e9bf9bb2bdb9a53d8eR106-R109 (see also my comment about TestVolumeEmptyDir here: Hardened Security Context for Elasticsearch #6703 (comment))
Containers SecurityContext is set automatically iff none is initially set. In other words, it's not merged with a SecurityContext that would come from the podTemplate: https://github.com/elastic/cloud-on-k8s/pull/6703/files#diff-e0b41f58404fb3608247e1e16a04b3b27a1abcafd46e1507d5b86b8c05aa30ffR349-R359

barkbay · 2023-04-24T10:55:11Z

buildkite test this -f p=ocp,s=8.7.0

barkbay · 2023-04-25T05:58:52Z

Results from the last e2e test sessions:

OCP4 🟢
7.17.8/8.0.1/8.7.0 on GKE 🟢
8.8.0-SNAPSHOT 🟡 - 140/144 tests passed, TestKibanaStandalone failed. As for this failure, I don't think it is related to the changes in this PR.

thbkrkr

LGTM, this is great!

Looks also like we're writing code specifically to handle #6186 but why not fix it first? We would no longer need to detect if the data directory is mounted in a volume as this is normally impossible, no? So we could call WithContainersSecurityContext after stackmon.WithMonitoring, to reuse the same code to set the SecurityContext for beats sidecars.

barkbay · 2023-04-26T06:18:32Z

Looks also like we're writing code specifically to handle #6186 but why not fix it first?

At first I thought this code was specifically handling #6186. But I've been wondering if, in general, we should not set readOnlyRootFilesystem to true only if we are pretty confident in the fact that the data are written in a volume, not in the container. I'm thinking about users that will upgrade to 2.8.0: if we set readOnlyRootFilesystem to true too "aggressively" we may end up with issues during upgrades if Elasticsearch is not configured properly. That's why I decided to set readOnlyRootFilesystem to true only if the default data volume is mounted as expected. If a user has a more involved storage configuration, with different volumes, it would have to test this flag manually first.

I have to admit this is still brittle, as there are other ways to mis-configure Elasticsearch with the default volume, ending up with data being written into the container. As you suggest, we could also always enable readOnlyRootFilesystem and let the user fix the configuration if the Pods crash, I'm trying to avoid that situation by keeping this code.

thbkrkr

But I've been wondering if, in general, we should not set readOnlyRootFilesystem to true only if we are pretty confident in the fact that the data are written in a volume, not in the container. I'm thinking about users that will upgrade to 2.8.0: if we set readOnlyRootFilesystem to true too "aggressively" we may end up with issues during upgrades if Elasticsearch is not configured properly.

👍 👍

pebrc

Code LGTM I have not done many tests yet.

pkg/controller/elasticsearch/securitycontext/securitycontext.go

pkg/controller/elasticsearch/nodespec/podspec.go

pebrc · 2023-04-26T13:37:26Z

pkg/controller/elasticsearch/stackmon/sidecar.go

+		Privileged:               pointer.Bool(false),
+		RunAsNonRoot:             pointer.Bool(true),
+		ReadOnlyRootFilesystem:   pointer.Bool(true),
+		AllowPrivilegeEscalation: pointer.Bool(false),


Can we use a shared variable or function here given that we use the exact same context twice? Also potentially we could refer to the new securitycontext package here, to have all contexts in one place?

👍 done in 18818fc

test/e2e/test/elasticsearch/check_securitycontext.go

pebrc

LGTM I managed to run a few tests on GKE and OCP and things seem to work fine!

barkbay · 2023-04-27T09:33:16Z

I'll open an issue to update https://www.elastic.co/guide/en/cloud-on-k8s/current/k8s-security-context.html before the next release as it is definitely outdated.

Edit: I think we can actually work on this as part of #5913

This adjusts the number of volumes expected from Beats sidecars in the Logstash Stack Monitoring unit tests. Why? Because we don't test PRs with an automatic merge of the main branch (🐛🐞), we missed that the tests in #6732 had to be updated to take into account the changes made by #6703, which adds a new temp volume to the Beats sidecars.

barkbay added 2 commits April 19, 2023 10:04

Copy the Elasticsearch configuration and then create links

c5609db

Set default security context for containers

ec64a8c

botelastic bot added the triage label Apr 19, 2023

lint + update unit test

651f0c1

thbkrkr added the >enhancement Enhancement of existing functionality label Apr 19, 2023

botelastic bot removed the triage label Apr 19, 2023

barkbay added 3 commits April 20, 2023 08:46

Improve security context for the keystore init container

fedf519

Add dirty hack to detect if data are stored in a volume

8a7e4c5

Add E2E check for securityContext

20973fc

barkbay added 2 commits April 20, 2023 09:32

lint

561d019

Update unit test

41609e1

Fix test name

8257fe0

barkbay added 2 commits April 21, 2023 14:06

Merge remote-tracking branch 'origin/main' into allow-readonly

94568de

Add SecurityContext to sidecars containers

148cb2f

barkbay force-pushed the allow-readonly branch from 7684189 to 148cb2f Compare April 21, 2023 13:35

Only set a SecurityContext for Elasticsearch

10351e2

[E2E][OpenShift] Do not strictly compare securityContext.Capabilities…

ed60048

….Drop

barkbay force-pushed the allow-readonly branch from d929102 to ed60048 Compare April 24, 2023 07:36

barkbay added the v2.8.0 label Apr 24, 2023

barkbay marked this pull request as ready for review April 24, 2023 08:27

thbkrkr reviewed Apr 25, 2023

View reviewed changes

thbkrkr approved these changes Apr 26, 2023

View reviewed changes

pebrc reviewed Apr 26, 2023

View reviewed changes

barkbay added 4 commits April 27, 2023 08:27

Update comment re. ReadOnlyRootFilesystem

2b9a419

Update comment re. MinStackVersion

16648fe

[E2E] Fix typo

d021ad3

Refactor ES beats sidecars securityContext and improve unit tests

18818fc

barkbay force-pushed the allow-readonly branch from ab8c034 to 18818fc Compare April 27, 2023 06:44

pebrc approved these changes Apr 27, 2023

View reviewed changes

barkbay merged commit 7a57bbd into elastic:main Apr 27, 2023

barkbay deleted the allow-readonly branch April 27, 2023 09:37

barkbay mentioned this pull request Apr 27, 2023

Run as non-root Elasticsearch is outdated and not relevant when running on OpenShift #5913

Closed

thbkrkr mentioned this pull request Apr 27, 2023

Default to running Elasticsearch as ReadOnlyRootFilesystem: true #6688

Closed

2 tasks

pebrc mentioned this pull request Apr 28, 2023

runAsNonRoot breaks stack monitoring on older versions of the stack #6740

Closed

thbkrkr mentioned this pull request Apr 28, 2023

Update Logstash Stack Monitoring unit tests #6741

Merged

thbkrkr mentioned this pull request May 25, 2023

[DOC] Mention base_path change in 2.8.0 release notes #6844

Merged

barkbay changed the title ~~Copy Elasticsearch configuration files before creating links~~ Hardened Security Context for Elasticsearch May 25, 2023

naemono mentioned this pull request May 9, 2024

Hardened Security Context for Kibana #7787

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hardened Security Context for Elasticsearch #6703

Hardened Security Context for Elasticsearch #6703

barkbay commented Apr 19, 2023 •

edited

Loading

barkbay commented Apr 19, 2023

thbkrkr commented Apr 19, 2023

barkbay commented Apr 19, 2023

barkbay commented Apr 20, 2023

thbkrkr commented Apr 20, 2023

barkbay commented Apr 20, 2023

barkbay commented Apr 20, 2023

barkbay commented Apr 21, 2023

barkbay commented Apr 21, 2023

barkbay commented Apr 23, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 25, 2023

thbkrkr left a comment

barkbay commented Apr 26, 2023

thbkrkr left a comment

pebrc left a comment

pebrc Apr 26, 2023

barkbay Apr 27, 2023

pebrc left a comment

barkbay commented Apr 27, 2023 •

edited

Loading

Hardened Security Context for Elasticsearch #6703

Hardened Security Context for Elasticsearch #6703

Conversation

barkbay commented Apr 19, 2023 • edited Loading

barkbay commented Apr 19, 2023

thbkrkr commented Apr 19, 2023

barkbay commented Apr 19, 2023

barkbay commented Apr 20, 2023

thbkrkr commented Apr 20, 2023

barkbay commented Apr 20, 2023

barkbay commented Apr 20, 2023

barkbay commented Apr 21, 2023

barkbay commented Apr 21, 2023

barkbay commented Apr 23, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 24, 2023

barkbay commented Apr 25, 2023

thbkrkr left a comment

Choose a reason for hiding this comment

barkbay commented Apr 26, 2023

thbkrkr left a comment

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

pebrc Apr 26, 2023

Choose a reason for hiding this comment

barkbay Apr 27, 2023

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

barkbay commented Apr 27, 2023 • edited Loading

barkbay commented Apr 19, 2023 •

edited

Loading

barkbay commented Apr 27, 2023 •

edited

Loading