Skip to content
This repository was archived by the owner on May 16, 2023. It is now read-only.

[elasticsearch]: optionally disable SA token automount#1300

Merged
jmlrt merged 1 commit intoelastic:masterfrom
equinix-ms:elastic-opt-mount-sa-token
Oct 1, 2021
Merged

[elasticsearch]: optionally disable SA token automount#1300
jmlrt merged 1 commit intoelastic:masterfrom
equinix-ms:elastic-opt-mount-sa-token

Conversation

@jonkerj
Copy link
Copy Markdown
Contributor

@jonkerj jonkerj commented Jul 20, 2021 •
edited
Loading

Description of the change

It disables the automounting of the service account token in the pod. Elastic does not need this by itself. This commit disables it by default, but leaves it configurable if anyone needs to use it.

Benefits

By disabling the automount, potential attackers cannot access the Kubernetes API on behalf/through the pod.

Possible drawbacks

If anyone is using some sidecar or plugin to access the Kubernetes API, they will have to explicitly enable (--set rbac.automountToken=true) the automount of the SA token in the values.

Applicable issues

#1330

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${CHART}/examples/*/test/goss.yaml

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jul 20, 2021

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@jonkerj jonkerj changed the title [elasticsearch]: optionally enable SA token automount WIP: [elasticsearch]: optionally enable SA token automount Jul 20, 2021
@jonkerj jonkerj changed the title WIP: [elasticsearch]: optionally enable SA token automount [elasticsearch]: optionally enable SA token automount Jul 20, 2021
@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Aug 16, 2021

Hi devs, is there anything I can do to get this ball rolling?

@jonkerj jonkerj mentioned this pull request Aug 23, 2021
Closed
@jonkerj jonkerj changed the base branch from 7.13 to 7.14 August 27, 2021 08:11
@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Aug 27, 2021

Rebased/cherry-picked to 7.14

@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Sep 2, 2021

Hi devs, I'd really like someone to take a look at this security fix. Is there anything I can do to get this fixed?

@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Sep 13, 2021

@jmlrt any chance you could take a look at this PR? Looks like you are an active dev here, and I'd really like this feature at least considered 😉

jmlrt
jmlrt suggested changes Sep 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

Hi @jonkerj, sorry for the late answer.
This PR makes sense, however I have a few comments:

  • unless cases where a change is only valid for a specific version, PR should be opened to master branch then Elastic handle the backport to the other branches
  • the default value should be True to avoid a breaking change

@jmlrt jmlrt added elasticsearch enhancement New feature or request labels Sep 16, 2021
@cla-checker-service
Copy link
Copy Markdown

cla-checker-service Bot commented Sep 20, 2021 •
edited
Loading

💚 CLA has been signed

@jonkerj jonkerj changed the base branch from 7.14 to master September 20, 2021 08:42
@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Sep 20, 2021

Better late than never ;-) I've (re-)signed the CLA, fixed both things, and force pushed. Should be good now.

@jonkerj jonkerj changed the title [elasticsearch]: optionally enable SA token automount [elasticsearch]: optionally disable SA token automount Sep 20, 2021
jmlrt
jmlrt approved these changes Sep 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

LGTM⛴

@jmlrt
Copy link
Copy Markdown
Member

jmlrt commented Sep 20, 2021

jenkins test this please

jmlrt
jmlrt suggested changes Sep 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

Some changes are required to make Black formatter happy in elastic+helm-charts+pull-request+lint-python/1093.

Can you run black elasticsearch/tests/elasticsearch_test.py?

@jonkerj
[elasticsearch]: optionally disable SA token automount
6d7332a 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Sep 20, 2021

done

@jmlrt
Copy link
Copy Markdown
Member

jmlrt commented Sep 20, 2021

jenkins test this please

@jonkerj
Copy link
Copy Markdown
Contributor Author

jonkerj commented Oct 1, 2021

@jmlrt : is there anything else I can do?

@jmlrt
Copy link
Copy Markdown
Member

jmlrt commented Oct 1, 2021

Hi @jonkerj, sorry the PR is approved but CI tests are broken for now.
I'll admin-merge it because I don't have time to fix the jobs for now.

@jmlrt jmlrt merged commit 5cfd9f7 into elastic:master Oct 1, 2021
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Oct 7, 2021
@jonkerj @jmlrt
[elasticsearch]: optionally disable SA token automount (elastic#1300)
0e20093 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jmlrt jmlrt mentioned this pull request Oct 7, 2021
Merged
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Oct 7, 2021
@jonkerj @jmlrt
[elasticsearch]: optionally disable SA token automount (elastic#1300)
cef20cf 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jmlrt jmlrt mentioned this pull request Oct 7, 2021
Closed
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Oct 7, 2021
@jonkerj @jmlrt
[elasticsearch]: optionally disable SA token automount (elastic#1300)
a1823d1 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jmlrt jmlrt mentioned this pull request Oct 7, 2021
Merged
jmlrt pushed a commit to jmlrt/helm-charts that referenced this pull request Oct 7, 2021
@jonkerj @jmlrt
[elasticsearch]: optionally disable SA token automount (elastic#1300)
74abe43 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>
@jmlrt jmlrt mentioned this pull request Oct 7, 2021
Merged
jmlrt added a commit that referenced this pull request Oct 11, 2021
@jmlrt @jonkerj
[elasticsearch]: optionally disable SA token automount (#1300) (#1391)
c4ff1b2 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>

Co-authored-by: Jorik Jonker <jorik@kippendief.biz>
jmlrt added a commit that referenced this pull request Oct 11, 2021
@jmlrt @jonkerj
[elasticsearch]: optionally disable SA token automount (#1300) (#1393)
33038cd 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>

Co-authored-by: Jorik Jonker <jorik@kippendief.biz>
jmlrt added a commit that referenced this pull request Oct 12, 2021
@jmlrt @jonkerj
[elasticsearch]: optionally disable SA token automount (#1300) (#1394)
a8d5ee2 
ES has no direct interaction with the Kubernetes API, and as such, it
does not need a mounted service account token in its pods. By disabling
this automount, potential attackers cannot access the API on
behalf/through the Pod.

This commit allows users to opt out on SA token automount. It leaves the
current behaviour unchanged, to avoid breaking things.

Signed-off-by: Jorik Jonker <jorik.jonker@eu.equinix.com>

Co-authored-by: Jorik Jonker <jorik@kippendief.biz>
@jmlrt jmlrt removed the v7.16.0 label Dec 13, 2021
This was referenced Dec 14, 2021
Merged
Merged
Merged
Merged
@jmlrt jmlrt mentioned this pull request Mar 8, 2022
Merged
@jmlrt jmlrt mentioned this pull request Apr 21, 2022
Merged
This was referenced Sep 14, 2022
Merged
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@jmlrt jmlrt jmlrt requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backported elasticsearch enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jonkerj @elasticmachine @jmlrt