Update security best practices document by jportner · Pull Request #100814 · elastic/kibana

jportner · 2021-05-27T18:45:18Z

I've reformatted the "Security best practices" page (security.asciidoc), adding references and additional info on attacks and Kibana's built-in security controls.

best_practices.mdx was derived from this; I will update it accordingly when the review is complete and the content is agreed upon.

Docs preview link

I've reformatted the document, adding references and additional info on attacks and Kibana's built-in security controls. TODO: update `best_practices.mdx` accordingly.

jportner

Self-review. I structured this so each section has the following information:

Reference to authoritative information and additional resources, wherever possible
A short description of the attack
A short description on Kibana's platform-wide defenses, if applicable
Best practices that developers should follow

In addition to the comments below, a couple of high-level thoughts:

I debated adding a section on Denial of Service. We have one other section on generic threats (Information Disclosure). DoS is typically introduced by third-party dependencies though, not by developers directly.
I debated adding a section on authorization. We have a separate Security development page that describes RBAC and authorization. Would it be useful to add a section here with a blurb that developers should make sure users are authorized, and a link to the aforementioned page?

jportner · 2021-05-27T18:59:53Z

docs/developer/best-practices/security.asciidoc

+When writing code for {kib}, be sure to follow these best practices to avoid common vulnerabilities. Refer to the included Open Web
+Application Security Project (OWASP) references to learn more about these types of attacks.


I thought about including a section on threats here at the top (such as this), but it felt like that might be a bit too much info for our audience, who are mostly not security experts.

docs/developer/best-practices/security.asciidoc

jportner · 2021-05-27T19:02:17Z

docs/developer/best-practices/security.asciidoc

+Sheet].
+* Allowing users to input markdown is a common culprit. If using a non-EUI markdown renderer, use a custom link renderer for rendered links.
+
+=== Information Disclosure ===


Everything else here is a class of attacks, but Information Disclosure is a generic threat. I opted to leave this here as I think it's a good concern for folks to keep top-of-mind.

kobelb

Thanks for taking this on @jportner, your improvements are much appreciated.

docs/developer/best-practices/security.asciidoc

stacey-gammon · 2021-05-27T20:40:35Z

I defer to Brandon's LGTM. Thanks for doing this Joe, and for reviewing Brandon!

(just don't forget to fill back the mdx file before merging)

docs/developer/best-practices/security.asciidoc

Co-authored-by: Thomas Watson <w@tson.dk>

legrego

This is great, thanks for making these changes, Joe!

I debated adding a section on authorization. We have a separate Security development page that describes RBAC and authorization. Would it be useful to add a section here with a blurb that developers should make sure users are authorized, and a link to the aforementioned page?

I think this would be worthwhile, but I also think the Security development page could use a rewrite. I think it could benefit from a restructuring so that we can provide actionable "do this"/"don't do this" for our solution teams, rather than describing "how the sausage is made". I think the existing content is still valuable, but it's not very actionable in urs current form.

All that to say: perhaps we can add a link to the "new" Security development page once we take the time to write it?

azasypkin

Looks great, Joe! Left just a few very minor remarks.

docs/developer/best-practices/security.asciidoc

streamich

LGTM. I've made Risk Matrix to link to this doc.

docs/developer/best-practices/security.asciidoc

gchaps · 2021-06-01T16:59:40Z

docs/developer/best-practices/security.asciidoc

+
+=== Remote Code Execution (RCE) ===
+
+https://owasp.org/www-community/attacks/Command_Injection[_OWASP reference for Command Injection_],


I don't see the text "RCE" is the linked pages.

Yeah this one is a bit confusing. Colloquially we lump "Command injection" and "Code injection" together, referring to these types of attacks as "RCE". But OWASP doesn't have an RCE page.

I tried to imply this equivalency below where I stated:

RCE is a class of attacks where an attacker executes malicious code or commands on a vulnerable server.

Do you think there's a better / more clear way I could word this?

jportner · 2021-06-10T15:34:59Z

OK, I think I addressed everyone's feedback!

I'll leave this open for another few days and if nobody else has anything to add, I'll update the MDX file to match and merge 😄

jportner · 2021-06-10T15:34:59Z

OK, I think I addressed everyone's feedback!

I'll leave this open for another few days and if nobody else has anything to add, I'll update the MDX file to match and merge 😄

spalger · 2021-06-16T20:21:11Z

jenkins, test this

(restarting due to jenkins upgrade)

kibanamachine · 2021-06-16T22:23:51Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2021-06-16T22:26:36Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

…egrations-to-global-search * 'master' of github.com:elastic/kibana: (46 commits) [Lens] Add some more documentation for dynamic coloring (elastic#101369) hide not searchable results when no term (elastic#102401) [Lens] Fix Formula functional test with multiple suggestions (elastic#102378) Fix trusted apps modified by field displayed as a date field (elastic#102377) [Lens] Docs for time shift (elastic#102048) update readme of logs-metrics-ui (elastic#101968) Refactor observability plugin breadcrumbs (elastic#102290) [Index Patterns] Move rollup config to index pattern management v2 (elastic#102285) [Security Solution][Endpoint] Isolate Action should only be available to Platinum+ licenses (elastic#102374) [build] Updates Ironbank templates (elastic#102407) Update security best practices document (elastic#100814) [Enterprise Search] Set up initial KibanaPageTemplate (elastic#102170) [Reporting/Docs] Add section to troubleshooting guide to explain the StatusCodeError logs (elastic#102278) [DOCS] Updating Elastic Security Overview topic (elastic#101922) [Uptime] refactor Synthetics Integration package UI (elastic#102080) [Task Manager] Log at different levels based on the state (elastic#101751) [APM] Fixing time comparison types (elastic#101423) [RAC] Update alert documents in lifecycle rule type helper (elastic#101598) [ML] Functional tests - fix and re-activate alerting flyout test (elastic#102368) [Reporting] remove unused reference to path.data config (elastic#102267) ... # Conflicts: # x-pack/plugins/fleet/kibana.json

Update security best practices document

03c9bb8

I've reformatted the document, adding references and additional info on attacks and Kibana's built-in security controls. TODO: update `best_practices.mdx` accordingly.

jportner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes docs v7.14.0 labels May 27, 2021

jportner mentioned this pull request May 27, 2021

Add "Risk Matrix" section to the PR template #100649

Merged

jportner marked this pull request as ready for review May 27, 2021 18:55

jportner requested review from a team, kobelb and stacey-gammon and removed request for kobelb May 27, 2021 18:55

jportner commented May 27, 2021

View reviewed changes

kobelb reviewed May 27, 2021

View reviewed changes

docs/developer/best-practices/security.asciidoc Outdated Show resolved Hide resolved

docs/developer/best-practices/security.asciidoc Show resolved Hide resolved

PR review feedback

3a91cbc

kobelb approved these changes May 27, 2021

View reviewed changes

watson reviewed May 28, 2021

View reviewed changes

Apply suggestions from code review

054518c

Co-authored-by: Thomas Watson <w@tson.dk>

legrego reviewed Jun 1, 2021

View reviewed changes

jportner added 3 commits June 1, 2021 09:16

Merge branch 'master' into update-security-best-practices

6cc48ac

Fix whitespace

1872fb2

Update XSS section

2a5ae39

jportner requested a review from a team June 1, 2021 13:23

azasypkin reviewed Jun 1, 2021

View reviewed changes

docs/developer/best-practices/security.asciidoc Outdated Show resolved Hide resolved

docs/developer/best-practices/security.asciidoc Outdated Show resolved Hide resolved

docs/developer/best-practices/security.asciidoc Outdated Show resolved Hide resolved

streamich approved these changes Jun 1, 2021

View reviewed changes

gchaps reviewed Jun 1, 2021

View reviewed changes

jportner added 2 commits June 10, 2021 10:53

Merge branch 'master' into update-security-best-practices

27349a7

More PR review feedback

e4213bd

jportner added 3 commits June 16, 2021 13:13

Merge branch 'master' into update-security-best-practices

b44c13c

Add mdx version of security best practices

9a4901f

Fix formatting and MDX syntax error

195b5bc

jportner enabled auto-merge (squash) June 16, 2021 18:20

jportner added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 16, 2021

jportner merged commit d4cc686 into elastic:master Jun 16, 2021

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 16, 2021

Update security best practices document (elastic#100814)

558d268

kibanamachine mentioned this pull request Jun 16, 2021

[7.x] Update security best practices document (#100814) #102432

Merged

kibanamachine added a commit that referenced this pull request Jun 17, 2021

Update security best practices document (#100814) (#102432)

c34fab8

Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com>

jportner deleted the update-security-best-practices branch January 19, 2022 20:45

		When writing code for {kib}, be sure to follow these best practices to avoid common vulnerabilities. Refer to the included Open Web
		Application Security Project (OWASP) references to learn more about these types of attacks.


		=== Remote Code Execution (RCE) ===

		https://owasp.org/www-community/attacks/Command_Injection[_OWASP reference for Command Injection_],

Conversation

jportner commented May 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

jportner May 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jportner May 27, 2021

Choose a reason for hiding this comment

Uh oh!

kobelb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stacey-gammon commented May 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

streamich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gchaps Jun 1, 2021

Choose a reason for hiding this comment

Uh oh!

jportner Jun 1, 2021

Choose a reason for hiding this comment

Uh oh!

jportner commented Jun 10, 2021

Uh oh!

jportner commented Jun 10, 2021

Uh oh!

spalger commented Jun 16, 2021

Uh oh!

kibanamachine commented Jun 16, 2021

💚 Build Succeeded

Metrics [docs]

Uh oh!

kibanamachine commented Jun 16, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

jportner commented May 27, 2021 •

edited

Loading

stacey-gammon commented May 27, 2021 •

edited

Loading