-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add initial security best practices documentation #8952
Add initial security best practices documentation #8952
Conversation
Skipping CI for Draft Pull Request. |
0567672
to
32b45f3
Compare
32b45f3
to
7032ee4
Compare
@howardjohn: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@howardjohn I don't see any reason to not cherrypick to 1.9. Thoughts? |
/cherry-pick release-1.9 |
@jacob-delgado: new pull request created: #9003 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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 happy to see this started! Thank you thank you @howardjohn
@@ -7,36 +7,170 @@ owner: istio/wg-security-maintainers | |||
test: no | |||
--- | |||
|
|||
This section provides some deployment guidelines to help keep a service mesh secure. | |||
|
|||
## Use namespaces for isolation |
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.
I wonder if there should be an anti-patterns section (things not to do) and one of the items should be not relying too much on namespaces for isolation?
To reduce privileges granted to pods, Istio offers a [CNI plugin](/docs/setup/additional-setup/cni/) which removes this requirement. | ||
|
||
{{< warning >}} | ||
The Istio CNI plugin is currently an alpha feature. |
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.
@stewartbutler @mandarjog @justinpettit heads up, this will put pressure on carrying istio/cni into beta
|
||
All Istio features and APIs are assigned a [feature status](/about/feature-stages/), defining its stability, deprecation policy, and security policy. | ||
|
||
Because alpha and experimental features do not have as strong security guarantees, it is recommended to avoid them whenever possible. |
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.
Perhaps make this stronger. We do not create security releases for vulnerabilities in experimental features. We decide on a case by case basis whether to fix vulnerabilities in alpha features (basically, if we think there is broad adoption of an alpha feature, we'll patch vulnerabilities following principles of responsible disclosure)
Istio also offers a smaller image based on [distroless images](/docs/ops/configuration/security/harden-docker-images/) that reduces the dependencies in the image. | ||
|
||
{{< warning >}} | ||
Distroless images are currently an alpha feature. |
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.
@howardjohn @sdake can we take this to beta?
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.
opened a tracker in istio/enhancements#22
|
||
To fully lock down traffic, it is recommended to configure [authorization policies](/docs/tasks/security/authorization/). | ||
These allow creating fine-grained policies to allow or deny traffic. For example, you can allow only requests from the `app` namespace to access the `hello-world` service. | ||
|
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.
After mutual TLS, I think it'd be good to have a section for using the beta security policies.
We should cover the limitations of ALLOW policies / describe when it's safer to use DENY.
We should also describe how authentication policies are decoupled from authorization policies and make it clear that authentication policies without corresponding authorization policies are just security theater.
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.
I would need help with "We should cover the limitations of ALLOW policies / describe when it's safer to use DENY.", if one of the people mentioned above could take this on?
I could take on the later part if needed
|
||
* Redirection only handles TCP based traffic. Any UDP or ICMP packets will not be captured or modified. | ||
* Inbound capture is disabled on many [ports used by the sidecar](/docs/ops/deployment/requirements/#ports-used-by-istio) as well as port 22. This list can be expanded by options like `traffic.sidecar.istio.io/excludeInboundPorts`. | ||
* Outbound capture may similarly be reduced through settings like `traffic.sidecar.istio.io/excludeOutboundPorts` or other means. |
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.
I wonder if there should be a stronger link between this section and the immediately following NetworkPolicy section? You could explain which of the issues raised here can be addressed with k8s NetworkPolicy
{{< warning >}} | ||
Distroless images are currently an alpha feature. | ||
{{< /warning >}} | ||
|
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.
Please add:
- lockdown Istiod debug API (and describe the functionality you will lose by doing so)
- locking down istiod insecure port
- warning about the limitations of Envoy Admin API - an attacker can extract information that can inform subsequent attacks (not ideal) and can quitquitquit the local sidecar (which isn't a big deal, you're just dos'ing yourself). also reference Admin endpoint security envoyproxy/envoy#2763
This enables applications that send plaintext HTTP traffic to be transparently "upgraded" to HTTPS. | ||
|
||
Care must be taken when configuring the `DestinationRule`'s `tls` setting to specify the `caCertificates` field. | ||
When this is not set, the servers certificate will not be verified. |
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.
Out of scope, but it feels like the API should require either caCertificates
or something like insecureSkipVerify: true
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.
That's the plan, but its challenging to change due to backwards compat.
|
||
### Restrict `Gateway` creation privileges | ||
|
||
It is recommended to restrict creation of Gateway resources to trusted cluster administrators. This can be achieved by [Kubernetes RBAC policies](https://kubernetes.io/docs/reference/access-authn-authz/rbac/) or tools like [Open Policy Agent](https://www.openpolicyagent.org/). |
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.
May be worth mentioning that OPA/Gatekeeper can also be used to enforce some of these policies, like restricting hosts
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.
I may be misunderstanding, but doesn't this line already recommend OPA?
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.
I mean that it mentions OPA as a way to restrict Gateway creation to administrators, but it might be worth saying that OPA could be used for more of these recommendations too
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.
Ah, got it. Agreed 👍
|
||
### Isolate sensitive services | ||
|
||
It may be desired to enforce stricter physical isolation for sensitive services. For example, you may want to run a |
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.
May be helpful to offer example reasons for why someone might want to do this
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.
I think there should also be a section here pointing to docs about setting up alerting as well, to catch when things are not working right. E.g. if the control plane stops pushing config, apps may continue to work but expected configuration may not propagate. It's not directly a security issue, but I'd consider any production cluster without istio alerts to be at risk.
* Update customize prom scraping instruction. (#8976) * Update customize prom scraping instruction. * Update content/en/docs/ops/integrations/prometheus/index.md Co-authored-by: Sven Mawson <sven@google.com> Co-authored-by: Sven Mawson <sven@google.com> * IstioCon blog post (#8984) Co-authored-by: Frank Budinsky <frankb@ca.ibm.com> * s/service-apis/gateway-api/ (#8988) * Update index.md * Update index.md * Revert "remove rbac instruction (#8442)" (#8990) This reverts commit a65a859. * Update observability best practices (#8897) * Update observability best practices * Fix linting issue * Try and clarify prometheus install * Update content/en/docs/ops/best-practices/observability/index.md Co-authored-by: Frank Budinsky <frankb@ca.ibm.com> * Move to observability page Co-authored-by: Frank Budinsky <frankb@ca.ibm.com> * Add doc about how to work around missing metric expiry. (#8948) * Add doc about how to work around missing metric expiry. * address comment. * lint * add spelling change * fix * Update content/en/faq/metrics-and-logs/telemetry-v1-vs-v2.md Co-authored-by: Douglas Reid <douglas-reid@users.noreply.github.com> Co-authored-by: Douglas Reid <douglas-reid@users.noreply.github.com> * Add initial security best practices documentation (#8952) * Clarify Prometheus TLS settings. (#8962) * Clarify Prometheus TLS settings. * Update content/en/docs/ops/integrations/prometheus/index.md Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> * Update content/en/docs/ops/integrations/prometheus/index.md Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> * Update content/en/docs/ops/integrations/prometheus/index.md Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> * Automator: update common-files@master in istio/istio.io@master (#8997) * Automator: update istio.io@ reference docs (#8998) * Automator: update istio.io@ reference docs (#9004) * Make attribute gen yaml file valid. (#9000) * Fix the client IP addresses for the authz ingress task (#9002) * Fix link to Configuration title (#9009) Signed-off-by: Radim Hrazdil <rhrazdil@redhat.com> * Automator: update istio.io@ reference docs (#9019) * Automator: update istio@ test reference (#9021) * Use istio-ecosystem wasm extensions repo in extensibility concept page. (#9018) * add a troubleshooting guide for multicluster (#8957) * add a troubleshooting guide for multicluster * fix meta * fix meta * address review comments * shift weight * rel link * lint * fix link * hard to tell what our mdlint customizations are... * fix mc guide link * add more context to high-level issues * cleanup phrasing * Remove fixed limitation warning (#9034) This issue no longer exists, I verified via the code and tested it myself as well. * Fixed "are is" to "are" and addressed an incorrect link (#9035) * fixing a typo * changed the link to go directly to canary upgrades page * Automator: update istio.io@ reference docs (#9036) * fix circuit breaker task (#9022) * fix circuit breaker task * gen * Add documentation for Analysis messsage IST0134 ServiceEntryAddresses… (#9020) * Add documentation for Analysis messsage IST0134 ServiceEntryAddressesRequired Signed-off-by: zufardhiyaulhaq <zufardhiyaulhaq@gmail.com> * Apply suggestions from code review Co-authored-by: Ram Vennam <rvennam@us.ibm.com> * Update content/en/docs/reference/config/analysis/ist0134/index.md Co-authored-by: Ram Vennam <rvennam@us.ibm.com> Co-authored-by: Ram Vennam <rvennam@us.ibm.com> * Add blog for zero configuration Istio (#9025) * Add blog for zero configuration Istio The intent here is to show off what Istio provides out of the box, to attempt to counteract some of the reputation Istio has gotten for being over complicated/requiring too many CRDs. * fix links * Address comments * Fix examples for newer kubectl (#9045) * Fix istio.io tests when moving to later kubectl (#9046) * Automator: update istio.io@ reference docs (#9047) * Ignore error on first kiali apply (#9048) * Ignore some errors (#9049) * Fix syntax on local rate limiting (#9044) * Add cross references to virtual machine docs (#8913) * Add cross references to virtual machine docs * Sven's suggestions * Update content/en/docs/ops/diagnostic-tools/virtual-machines/index.md Co-authored-by: Sven Mawson <sven@google.com> Co-authored-by: Sven Mawson <sven@google.com> * update authz troubleshoot common problems (#9043) * update authz troubleshoot common problems * update * Add out-of-mesh server metadata info into telemetry v2 faq. (#9017) * Add out-of-mesh server metadata info into telemetry v2 faq. * Update content/en/faq/metrics-and-logs/telemetry-v1-vs-v2.md Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> * reword Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> * Release notes for 1.7.8 (#9054) * Announce EOL for Istio 1.7 (#9005) * Release notes for 1.7.7 * Update content/en/news/releases/1.7.x/announcing-1.7.7/index.md Co-authored-by: Brian Avery <bavery@redhat.com> * update * update * Announce EOL for Istio 1.7 * delay to 02-25 * Delete index.md Co-authored-by: Brian Avery <bavery@redhat.com> Co-authored-by: Pengyuan Bian <bianpengyuan@google.com> Co-authored-by: Sven Mawson <sven@google.com> Co-authored-by: Istio Automation <istio-testing-bot@google.com> Co-authored-by: Frank Budinsky <frankb@ca.ibm.com> Co-authored-by: craigbox <craigbox@google.com> Co-authored-by: John Howard <howardjohn@google.com> Co-authored-by: jacob-delgado <jacob.delgado@volunteers.acasi.info> Co-authored-by: Douglas Reid <douglas-reid@users.noreply.github.com> Co-authored-by: Eric Van Norman <ericvn@us.ibm.com> Co-authored-by: lei-tang <32078630+lei-tang@users.noreply.github.com> Co-authored-by: Radim Hrazdil <32546791+rhrazdil@users.noreply.github.com> Co-authored-by: Steven Landow <steven@stlcomputerservices.com> Co-authored-by: Kang-Bae <59033920+Kang-Bae@users.noreply.github.com> Co-authored-by: masquee <okayanz@outlook.com> Co-authored-by: Zufar Dhiyaulhaq <zufardhiyaulhaq@gmail.com> Co-authored-by: Ram Vennam <rvennam@us.ibm.com> Co-authored-by: Ryan Baker <ryan.baker@c2fo.com> Co-authored-by: Yangmin Zhu <ymzhu@google.com> Co-authored-by: Jimmy Chen <28548492+JimmyCYJ@users.noreply.github.com> Co-authored-by: Brian Avery <bavery@redhat.com>
No description provided.