From 03c9bb84a8fe0dfd7959e52c4fa12ab56877e9ac Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 27 May 2021 14:40:26 -0400 Subject: [PATCH 1/8] Update security best practices document 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. --- dev_docs/best_practices.mdx | 29 +-- .../best-practices/security.asciidoc | 177 +++++++++++++----- 2 files changed, 126 insertions(+), 80 deletions(-) diff --git a/dev_docs/best_practices.mdx b/dev_docs/best_practices.mdx index 54aaaa6b9497ad..4adcff95ec7ff1 100644 --- a/dev_docs/best_practices.mdx +++ b/dev_docs/best_practices.mdx @@ -245,31 +245,4 @@ It may be tempting to get caught up in the dream of writing the next package whi Review the following items related to vulnerability and security risks. -- XSS - - Check for usages of `dangerouslySetInnerHtml`, `Element.innerHTML`, `Element.outerHTML` - - Ensure all user input is properly escaped. - - Ensure any input in `$.html`, `$.append`, `$.appendTo`, $.prepend`, `$.prependTo`is escaped. Instead use`$.text`, or don't use jQuery at all. -- CSRF - - Ensure all APIs are running inside the Kibana HTTP service. -- RCE - - Ensure no usages of `eval` - - Ensure no usages of dynamic requires - - Check for template injection - - Check for usages of templating libraries, including `_.template`, and ensure that user provided input isn't influencing the template and is only used as data for rendering the template. - - Check for possible prototype pollution. -- Prototype Pollution - more info [here](https://docs.google.com/document/d/19V-d9sb6IF-fbzF4iyiPpAropQNydCnoJApzSX5FdcI/edit?usp=sharing) - - Check for instances of `anObject[a][b] = c` where a, b, and c are user defined. This includes code paths where the following logical code steps could be performed in separate files by completely different operations, or recursively using dynamic operations. - - Validate any user input, including API url-parameters/query-parameters/payloads, preferable against a schema which only allows specific keys/values. At a very minimum, black-list `__proto__` and `prototype.constructor` for use within keys - - When calling APIs which spawn new processes or potentially perform code generation from strings, defensively protect against Prototype Pollution by checking `Object.hasOwnProperty` if the arguments to the APIs originate from an Object. An example is the Code app's [spawnProcess](https://github.com/elastic/kibana/blob/b49192626a8528af5d888545fb14cd1ce66a72e7/x-pack/legacy/plugins/code/server/lsp/workspace_command.ts#L40-L44). - - Common Node.js offenders: `child_process.spawn`, `child_process.exec`, `eval`, `Function('some string')`, `vm.runIn*Context(x)` - - Common Client-side offenders: `eval`, `Function('some string')`, `setTimeout('some string', num)`, `setInterval('some string', num)` -- Check for accidental reveal of sensitive information - - The biggest culprit is errors which contain stack traces or other sensitive information which end up in the HTTP Response -- Checked for Mishandled API requests - - Ensure no sensitive cookies are forwarded to external resources. - - Ensure that all user controllable variables that are used in constructing a URL are escaped properly. This is relevant when using `transport.request` with the Elasticsearch client as no automatic escaping is performed. -- Reverse tabnabbing - https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#tabnabbing - - When there are user controllable links or hard-coded links to third-party domains that specify target="\_blank" or target="\_window", the `a` tag should have the rel="noreferrer noopener" attribute specified. - - Allowing users to input markdown is a common culprit, a custom link renderer should be used -- SSRF - https://www.owasp.org/index.php/Server_Side_Request_Forgery - - All network requests made from the Kibana server should use an explicit configuration or white-list specified in the `kibana.yml` +TODO: copy content from security.asciidoc when review is complete diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index 79ecb082950646..fe84d38427d74e 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -1,55 +1,128 @@ [[security-best-practices]] == Security best practices -* XSS -** Check for usages of `dangerouslySetInnerHtml`, `Element.innerHTML`, -`Element.outerHTML` -** Ensure all user input is properly escaped. -** Ensure any input in `$.html`, `$.append`, `$.appendTo`, -latexmath:[$.prepend`, `$].prependTo`is escaped. Instead use`$.text`, or -don’t use jQuery at all. -* CSRF -** Ensure all APIs are running inside the {kib} HTTP service. -* RCE -** Ensure no usages of `eval` -** Ensure no usages of dynamic requires -** Check for template injection -** Check for usages of templating libraries, including `_.template`, and -ensure that user provided input isn’t influencing the template and is -only used as data for rendering the template. -** Check for possible prototype pollution. -* Prototype Pollution -** Check for instances of `anObject[a][b] = c` where a, b, and c are -user defined. This includes code paths where the following logical code -steps could be performed in separate files by completely different -operations, or recursively using dynamic operations. -** Validate any user input, including API -url-parameters/query-parameters/payloads, preferable against a schema -which only allows specific keys/values. At a very minimum, black-list -`__proto__` and `prototype.constructor` for use within keys -** When calling APIs which spawn new processes or potentially perform -code generation from strings, defensively protect against Prototype -Pollution by checking `Object.hasOwnProperty` if the arguments to the -APIs originate from an Object. An example is the Code app’s -https://github.com/elastic/kibana/blob/b49192626a8528af5d888545fb14cd1ce66a72e7/x-pack/legacy/plugins/code/server/lsp/workspace_command.ts#L40-L44[spawnProcess]. -*** Common Node.js offenders: `child_process.spawn`, -`child_process.exec`, `eval`, `Function('some string')`, -`vm.runIn*Context(x)` -*** Common Client-side offenders: `eval`, `Function('some string')`, -`setTimeout('some string', num)`, `setInterval('some string', num)` -* Check for accidental reveal of sensitive information -** The biggest culprit is errors which contain stack traces or other -sensitive information which end up in the HTTP Response -* Checked for Mishandled API requests -** Ensure no sensitive cookies are forwarded to external resources. -** Ensure that all user controllable variables that are used in -constructing a URL are escaped properly. This is relevant when using -`transport.request` with the {es} client as no automatic -escaping is performed. -* Reverse tabnabbing - -https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#tabnabbing -** When there are user controllable links or hard-coded links to -third-party domains that specify target="_blank" or target="_window", the a tag should have the rel="noreferrer noopener" attribute specified. -Allowing users to input markdown is a common culprit, a custom link renderer should be used -* SSRF - https://www.owasp.org/index.php/Server_Side_Request_Forgery -All network requests made from the {kib} server should use an explicit configuration or white-list specified in the kibana.yml \ No newline at end of file +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. + +=== Cross-site Scripting (XSS) === + +https://owasp.org/www-community/attacks/xss[_OWASP reference for XSS_] + +XSS is a class of attacks where malicious scripts are injected into vulnerable websites. {kib} defends against this primarily by widespread +use of the React framework to safely encode data that is rendered to pages, widespread use of the EUI framework to +https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive Content-Security-Policy +header. + +*Best practices:* + +* Check for usage of dangerous functions or assignments that can result in unescaped user input in the browser DOM. Avoid +using these at all costs: +** *React:* https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml[`dangerouslySetInnerHtml`]. +** *Browser DOM:* `Element.innerHTML` and `Element.outerHTML`. +** *jQuery:* `$.html`, `$.append`, `$.appendTo`, `$.prepend`, and `$.prependTo`. Consider using `$.text` instead, which is a safe +alternative to these functions. +* If using any of the aforementioned unsafe functions/assignments is absolutely necessary, follow +https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#xss-prevention-rules[these XSS prevention +rules] to ensure that user input is not inserted into unsafe locations and to make sure that it is escaped properly. +* Use EUI components to build your UI whenever possible, particularly when rendering `href` links. Otherwise, sanitize user input before +rendering links to ensure that they do not use the `javascript:` protocol. + +=== Cross-site Request Forgery (CSRF/XSRF) === + +https://owasp.org/www-community/attacks/csrf[_OWASP reference for CSRF_] + +CSRF is a class of attacks where a user is forced to execute an action on a vulnerable website that they're logged into, usually without +their knowledge. {kib} defends against this by requiring +https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#use-of-custom-request-headers[custom +request headers] for API endpoints. For more information, see <>. + +*Best practices:* + +* Ensure all HTTP routes are registered via the <> to take advantage of the aforementioned custom request +header security control. + +=== Remote Code Execution (RCE) === + +https://owasp.org/www-community/attacks/Command_Injection[_OWASP reference for Command Injection_], +https://owasp.org/www-community/attacks/Code_Injection[_OWASP reference for Code Injection_] + +RCE is a class of attacks where an attacker executes malicious code or commands on a vulnerable server. {kib} defends against this by using +ESLint rules to restrict vulnerable functions, and by hooking into / hardening usage of these in third-party dependencies. + +*Best practices:* + +* Ensure no usages of `eval` and `_.template` -- these are restricted by ESLint rules. +* Ensure no usages of dynamic `require`. +* Check for usages of templating libraries; ensure that user provided input isn't influencing the template and is only used as data for +rendering the template. + +=== Prototype Pollution === + +Prototype Pollution is an attack that is unique to JavaScript environments. Attackers can abuse JavaScript's prototype inheritance to +"pollute" objects in the whole application, which is often used as a vector for XSS or RCE vulnerabilities. {kib} defends against this by +hardening particularly sensitive functions (such as those exposed by `child_process`), and by requiring validation on all HTTP routes by +default. + +*Best practices:* + +* Check for instances of `anObject[a][b] = c` where a, b, and c are controlled by user input. This includes code paths where the following +logical code steps could be performed in separate files by completely different operations, or recursively using dynamic operations. +* Validate any user input, including API url parameters, query parameters, and payloads. Preferably, use a schema which only allows specific +keys/values. At a very minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object +keys. +* When calling APIs which spawn new processes or potentially perform code generation from strings, defensively protect against Prototype +Pollution by checking `Object.hasOwnProperty` if the arguments to the APIs originate from an Object. An example is the defunct Code app's +https://github.com/elastic/kibana/blob/b49192626a8528af5d888545fb14cd1ce66a72e7/x-pack/legacy/plugins/code/server/lsp/workspace_command.ts#L40-L44[`spawnProcess`] +function. +** Common Node.js offenders: `child_process.spawn`, `child_process.exec`, `eval`, `Function('some string')`, `vm.runInContext(x)`, +`vm.runInNewContext(x)`, `vm.runInThisContext()` +** Common client-side offenders: `eval`, `Function('some string')`, `setTimeout('some string', num)`, `setInterval('some string', num)` + +See also: + +* https://portswigger.net/daily-swig/prototype-pollution-the-dangerous-and-underrated-vulnerability-impacting-javascript-applications[Prototype +pollution: The dangerous and underrated vulnerability impacting JavaScript applications | portswigger.net] +* https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf[Prototype +pollution attack in NodeJS application | Olivier Arteau] + +=== Server-side Request Forgery (SSRF) === + +https://owasp.org/www-community/attacks/Server_Side_Request_Forgery[_OWASP reference for SSRF_] + +SSRF is a class of attacks where a vulnerable server is forced to make an unintended request, usually to an HTTP API. This is often used as +a vector for information disclosure or injection attacks. + +*Best practices:* + +* Ensure that all outbound requests from the {kib} server use hard-coded URLs if possible. +* If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that user input is escaped properly. Ideally +this allow-list should be set in `kibana.yml` so only server administrators can change it. +** This is particularly relevant when using `transport.request` with the {es} client, as no automatic escaping is performed. + +=== Reverse Tabnabbing === + +https://owasp.org/www-community/attacks/Reverse_Tabnabbing[_OWASP reference for Reverse Tabnabbing_] + +Reverse Tabnabbing is an attack where a link to a malicious page is used to rewrite a vulnerable parent page. This is often used as a vector +for phishing attacks. {kib} defends against this primarily by widespread use of the EUI framework, which automatically adds the `rel` +attribute to anchor tags, buttons, and other vulnerable DOM elements. + +*Best practices:* + +* Use EUI components to build your UI whenever possible. Otherwise, ensure that any DOM elements that have an `href` attribute also have the +`rel="noreferrer noopener"` attribute specified. For more information, see the +https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#tabnabbing[OWASP HTML5 Security Cheat +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 === + +Information Disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration +info, stack traces, or other data that the user has not been authorized to access. This is a widespread concern that cannot be addressed +with a single security control, but at a high level {kib} relies on the Hapi framework to automatically redact stack traces and detailed +error messages in HTTP 5xx response payloads. + +*Best practices:* + +* When writing your code, look for instances where sensitive information may be accidentally revealed, particularly in error messages, in +the UI, and in URL parameters that are exposed to users. From 3a91cbc435f421d8e53567a13311a3257cc17199 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 27 May 2021 15:39:44 -0400 Subject: [PATCH 2/8] PR review feedback --- docs/developer/best-practices/security.asciidoc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index fe84d38427d74e..169ea63c8b5a80 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -19,8 +19,6 @@ header. using these at all costs: ** *React:* https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml[`dangerouslySetInnerHtml`]. ** *Browser DOM:* `Element.innerHTML` and `Element.outerHTML`. -** *jQuery:* `$.html`, `$.append`, `$.appendTo`, `$.prepend`, and `$.prependTo`. Consider using `$.text` instead, which is a safe -alternative to these functions. * If using any of the aforementioned unsafe functions/assignments is absolutely necessary, follow https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#xss-prevention-rules[these XSS prevention rules] to ensure that user input is not inserted into unsafe locations and to make sure that it is escaped properly. @@ -55,6 +53,7 @@ ESLint rules to restrict vulnerable functions, and by hooking into / hardening u * Ensure no usages of dynamic `require`. * Check for usages of templating libraries; ensure that user provided input isn't influencing the template and is only used as data for rendering the template. +* Take extra caution when spawning child processes with any user input or parameters that are otherwise user-controlled. === Prototype Pollution === @@ -126,3 +125,5 @@ error messages in HTTP 5xx response payloads. * When writing your code, look for instances where sensitive information may be accidentally revealed, particularly in error messages, in the UI, and in URL parameters that are exposed to users. +* Make sure that sensitive request data is not forwarded to external resources. For example, copying client request headers and using them +to make an another request could accidentally expose the user's session cookie. From 054518c8d7baf29c2f4365ffa753937507c99ca9 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Fri, 28 May 2021 09:47:23 -0400 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Thomas Watson --- docs/developer/best-practices/security.asciidoc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index 169ea63c8b5a80..39a4bc6a47d564 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -10,7 +10,7 @@ https://owasp.org/www-community/attacks/xss[_OWASP reference for XSS_] XSS is a class of attacks where malicious scripts are injected into vulnerable websites. {kib} defends against this primarily by widespread use of the React framework to safely encode data that is rendered to pages, widespread use of the EUI framework to -https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive Content-Security-Policy +https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive `Content-Security-Policy` header. *Best practices:* @@ -25,7 +25,7 @@ rules] to ensure that user input is not inserted into unsafe locations and to ma * Use EUI components to build your UI whenever possible, particularly when rendering `href` links. Otherwise, sanitize user input before rendering links to ensure that they do not use the `javascript:` protocol. -=== Cross-site Request Forgery (CSRF/XSRF) === +=== Cross-Site Request Forgery (CSRF/XSRF) === https://owasp.org/www-community/attacks/csrf[_OWASP reference for CSRF_] @@ -49,7 +49,7 @@ ESLint rules to restrict vulnerable functions, and by hooking into / hardening u *Best practices:* -* Ensure no usages of `eval` and `_.template` -- these are restricted by ESLint rules. +* Ensure no usages of `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. * Ensure no usages of dynamic `require`. * Check for usages of templating libraries; ensure that user provided input isn't influencing the template and is only used as data for rendering the template. @@ -64,7 +64,7 @@ default. *Best practices:* -* Check for instances of `anObject[a][b] = c` where a, b, and c are controlled by user input. This includes code paths where the following +* Check for instances of `anObject[a][b] = c` where `a`, `b`, and `c` are controlled by user input. This includes code paths where the following logical code steps could be performed in separate files by completely different operations, or recursively using dynamic operations. * Validate any user input, including API url parameters, query parameters, and payloads. Preferably, use a schema which only allows specific keys/values. At a very minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object @@ -84,7 +84,7 @@ pollution: The dangerous and underrated vulnerability impacting JavaScript appli * https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf[Prototype pollution attack in NodeJS application | Olivier Arteau] -=== Server-side Request Forgery (SSRF) === +=== Server-Side Request Forgery (SSRF) === https://owasp.org/www-community/attacks/Server_Side_Request_Forgery[_OWASP reference for SSRF_] @@ -118,7 +118,7 @@ Sheet]. Information Disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration info, stack traces, or other data that the user has not been authorized to access. This is a widespread concern that cannot be addressed -with a single security control, but at a high level {kib} relies on the Hapi framework to automatically redact stack traces and detailed +with a single security control, but at a high level {kib} relies on the hapi framework to automatically redact stack traces and detailed error messages in HTTP 5xx response payloads. *Best practices:* From 1872fb2ba934d265cfd04ef4518aec7c86bc2ae6 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Tue, 1 Jun 2021 09:17:41 -0400 Subject: [PATCH 4/8] Fix whitespace --- docs/developer/best-practices/security.asciidoc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index 39a4bc6a47d564..c996781139935f 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -64,8 +64,9 @@ default. *Best practices:* -* Check for instances of `anObject[a][b] = c` where `a`, `b`, and `c` are controlled by user input. This includes code paths where the following -logical code steps could be performed in separate files by completely different operations, or recursively using dynamic operations. +* Check for instances of `anObject[a][b] = c` where `a`, `b`, and `c` are controlled by user input. This includes code paths where the +following logical code steps could be performed in separate files by completely different operations, or recursively using dynamic +operations. * Validate any user input, including API url parameters, query parameters, and payloads. Preferably, use a schema which only allows specific keys/values. At a very minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object keys. @@ -94,8 +95,8 @@ a vector for information disclosure or injection attacks. *Best practices:* * Ensure that all outbound requests from the {kib} server use hard-coded URLs if possible. -* If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that user input is escaped properly. Ideally -this allow-list should be set in `kibana.yml` so only server administrators can change it. +* If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that +user input is escaped properly. Ideally this allow-list should be set in `kibana.yml` so only server administrators can change it. ** This is particularly relevant when using `transport.request` with the {es} client, as no automatic escaping is performed. === Reverse Tabnabbing === From 2a5ae39d1d04a12a5c5b0483ad3733446e920c9c Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Tue, 1 Jun 2021 09:22:32 -0400 Subject: [PATCH 5/8] Update XSS section --- docs/developer/best-practices/security.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index c996781139935f..670cf7c571f2b1 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -24,6 +24,9 @@ https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_C rules] to ensure that user input is not inserted into unsafe locations and to make sure that it is escaped properly. * Use EUI components to build your UI whenever possible, particularly when rendering `href` links. Otherwise, sanitize user input before rendering links to ensure that they do not use the `javascript:` protocol. +* Ensure no usages of `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. +* Be careful of when using `setTimeout` and `setInterval` in client-side code. If an attacker can manipulate the arguments and pass a string +to one of these, it is evaluated dynamically, which is equivalent to the dangerous `eval` function. === Cross-Site Request Forgery (CSRF/XSRF) === From e4213bd5911fcfbdea63114b9f023b2230560fe8 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Thu, 10 Jun 2021 11:33:20 -0400 Subject: [PATCH 6/8] More PR review feedback --- .../best-practices/security.asciidoc | 100 +++++++++--------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/docs/developer/best-practices/security.asciidoc b/docs/developer/best-practices/security.asciidoc index 670cf7c571f2b1..fd83aa1dff49fd 100644 --- a/docs/developer/best-practices/security.asciidoc +++ b/docs/developer/best-practices/security.asciidoc @@ -8,25 +8,24 @@ Application Security Project (OWASP) references to learn more about these types https://owasp.org/www-community/attacks/xss[_OWASP reference for XSS_] -XSS is a class of attacks where malicious scripts are injected into vulnerable websites. {kib} defends against this primarily by widespread -use of the React framework to safely encode data that is rendered to pages, widespread use of the EUI framework to +XSS is a class of attacks where malicious scripts are injected into vulnerable websites. {kib} defends against this by using the React +framework to safely encode data that is rendered in pages, the EUI framework to https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive `Content-Security-Policy` header. -*Best practices:* +*Best practices* -* Check for usage of dangerous functions or assignments that can result in unescaped user input in the browser DOM. Avoid -using these at all costs: +* Check for dangerous functions or assignments that can result in unescaped user input in the browser DOM. Avoid using: ** *React:* https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml[`dangerouslySetInnerHtml`]. ** *Browser DOM:* `Element.innerHTML` and `Element.outerHTML`. -* If using any of the aforementioned unsafe functions/assignments is absolutely necessary, follow +* If using the aforementioned unsafe functions or assignments is absolutely necessary, follow https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#xss-prevention-rules[these XSS prevention -rules] to ensure that user input is not inserted into unsafe locations and to make sure that it is escaped properly. -* Use EUI components to build your UI whenever possible, particularly when rendering `href` links. Otherwise, sanitize user input before -rendering links to ensure that they do not use the `javascript:` protocol. -* Ensure no usages of `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. -* Be careful of when using `setTimeout` and `setInterval` in client-side code. If an attacker can manipulate the arguments and pass a string -to one of these, it is evaluated dynamically, which is equivalent to the dangerous `eval` function. +rules] to ensure that user input is not inserted into unsafe locations and that it is escaped properly. +* Use EUI components to build your UI, particularly when rendering `href` links. Otherwise, sanitize user input before rendering links to +ensure that they do not use the `javascript:` protocol. +* Don't use the `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. +* Be careful when using `setTimeout` and `setInterval` in client-side code. If an attacker can manipulate the arguments and pass a string to +one of these, it is evaluated dynamically, which is equivalent to the dangerous `eval` function. === Cross-Site Request Forgery (CSRF/XSRF) === @@ -37,10 +36,12 @@ their knowledge. {kib} defends against this by requiring https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#use-of-custom-request-headers[custom request headers] for API endpoints. For more information, see <>. -*Best practices:* +*Best practices* -* Ensure all HTTP routes are registered via the <> to take advantage of the aforementioned custom request -header security control. +* Ensure all HTTP routes are registered with the <> to take advantage of the custom request header +security control. +** Note that HTTP GET requests do *not* require the custom request header; any routes that change data should +https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods[adhere to the HTTP specification and use a different method (PUT, POST, etc.)] === Remote Code Execution (RCE) === @@ -48,33 +49,32 @@ https://owasp.org/www-community/attacks/Command_Injection[_OWASP reference for C https://owasp.org/www-community/attacks/Code_Injection[_OWASP reference for Code Injection_] RCE is a class of attacks where an attacker executes malicious code or commands on a vulnerable server. {kib} defends against this by using -ESLint rules to restrict vulnerable functions, and by hooking into / hardening usage of these in third-party dependencies. +ESLint rules to restrict vulnerable functions, and by hooking into or hardening usage of these in third-party dependencies. -*Best practices:* +*Best practices* -* Ensure no usages of `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. -* Ensure no usages of dynamic `require`. -* Check for usages of templating libraries; ensure that user provided input isn't influencing the template and is only used as data for +* Don't use the `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. +* Don't use dynamic `require`. +* Check for usages of templating libraries. Ensure that user-provided input doesn't influence the template and is used only as data for rendering the template. -* Take extra caution when spawning child processes with any user input or parameters that are otherwise user-controlled. +* Take extra caution when spawning child processes with any user input or parameters that are user-controlled. === Prototype Pollution === Prototype Pollution is an attack that is unique to JavaScript environments. Attackers can abuse JavaScript's prototype inheritance to -"pollute" objects in the whole application, which is often used as a vector for XSS or RCE vulnerabilities. {kib} defends against this by -hardening particularly sensitive functions (such as those exposed by `child_process`), and by requiring validation on all HTTP routes by -default. +"pollute" objects in the application, which is often used as a vector for XSS or RCE vulnerabilities. {kib} defends against this by +hardening sensitive functions (such as those exposed by `child_process`), and by requiring validation on all HTTP routes by default. -*Best practices:* +*Best practices* * Check for instances of `anObject[a][b] = c` where `a`, `b`, and `c` are controlled by user input. This includes code paths where the -following logical code steps could be performed in separate files by completely different operations, or recursively using dynamic +following logical code steps could be performed in separate files by completely different operations, or by recursively using dynamic operations. -* Validate any user input, including API url parameters, query parameters, and payloads. Preferably, use a schema which only allows specific -keys/values. At a very minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object +* Validate all user input, including API URL parameters, query parameters, and payloads. Preferably, use a schema that only allows specific +keys and values. At a minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object keys. -* When calling APIs which spawn new processes or potentially perform code generation from strings, defensively protect against Prototype -Pollution by checking `Object.hasOwnProperty` if the arguments to the APIs originate from an Object. An example is the defunct Code app's +* When calling APIs that spawn new processes or perform code generation from strings, protect against Prototype Pollution by checking if +`Object.hasOwnProperty` has arguments to the APIs that originate from an Object. An example is the defunct Code app's https://github.com/elastic/kibana/blob/b49192626a8528af5d888545fb14cd1ce66a72e7/x-pack/legacy/plugins/code/server/lsp/workspace_command.ts#L40-L44[`spawnProcess`] function. ** Common Node.js offenders: `child_process.spawn`, `child_process.exec`, `eval`, `Function('some string')`, `vm.runInContext(x)`, @@ -95,39 +95,41 @@ https://owasp.org/www-community/attacks/Server_Side_Request_Forgery[_OWASP refer SSRF is a class of attacks where a vulnerable server is forced to make an unintended request, usually to an HTTP API. This is often used as a vector for information disclosure or injection attacks. -*Best practices:* +*Best practices* -* Ensure that all outbound requests from the {kib} server use hard-coded URLs if possible. +* Ensure that all outbound requests from the {kib} server use hard-coded URLs. * If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that -user input is escaped properly. Ideally this allow-list should be set in `kibana.yml` so only server administrators can change it. +user input is escaped properly. Ideally, the allow-list should be set in `kibana.yml`, so only server administrators can change it. ** This is particularly relevant when using `transport.request` with the {es} client, as no automatic escaping is performed. +** Note that URLs are very hard to validate properly; exact match validation for user input is most preferable, while URL parsing or RegEx +validation should only be used if absolutely necessary. -=== Reverse Tabnabbing === +=== Reverse tabnabbing === https://owasp.org/www-community/attacks/Reverse_Tabnabbing[_OWASP reference for Reverse Tabnabbing_] -Reverse Tabnabbing is an attack where a link to a malicious page is used to rewrite a vulnerable parent page. This is often used as a vector -for phishing attacks. {kib} defends against this primarily by widespread use of the EUI framework, which automatically adds the `rel` -attribute to anchor tags, buttons, and other vulnerable DOM elements. +Reverse tabnabbing is an attack where a link to a malicious page is used to rewrite a vulnerable parent page. This is often used as a vector +for phishing attacks. {kib} defends against this by using the EUI framework, which automatically adds the `rel` attribute to anchor tags, +buttons, and other vulnerable DOM elements. -*Best practices:* +*Best practices* * Use EUI components to build your UI whenever possible. Otherwise, ensure that any DOM elements that have an `href` attribute also have the -`rel="noreferrer noopener"` attribute specified. For more information, see the +`rel="noreferrer noopener"` attribute specified. For more information, refer to the https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#tabnabbing[OWASP HTML5 Security Cheat 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. +* If using a non-EUI markdown renderer, use a custom link renderer for rendered links. -=== Information Disclosure === +=== Information disclosure === -Information Disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration -info, stack traces, or other data that the user has not been authorized to access. This is a widespread concern that cannot be addressed -with a single security control, but at a high level {kib} relies on the hapi framework to automatically redact stack traces and detailed -error messages in HTTP 5xx response payloads. +Information disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration +info, stack traces, or other data that the user is not authorized to access. This concern cannot be addressed with a single security +control, but at a high level, {kib} relies on the hapi framework to automatically redact stack traces and detailed error messages in HTTP +5xx response payloads. -*Best practices:* +*Best practices* -* When writing your code, look for instances where sensitive information may be accidentally revealed, particularly in error messages, in -the UI, and in URL parameters that are exposed to users. +* Look for instances where sensitive information might accidentally be revealed, particularly in error messages, in the UI, and URL +parameters that are exposed to users. * Make sure that sensitive request data is not forwarded to external resources. For example, copying client request headers and using them -to make an another request could accidentally expose the user's session cookie. +to make an another request could accidentally expose the user's credentials. From 9a4901f71b49c1265654156cbb5f141551737b18 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Wed, 16 Jun 2021 13:55:36 -0400 Subject: [PATCH 7/8] Add mdx version of security best practices --- dev_docs/best_practices.mdx | 135 +++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 3 deletions(-) diff --git a/dev_docs/best_practices.mdx b/dev_docs/best_practices.mdx index 4adcff95ec7ff1..82b2a67abcc5b3 100644 --- a/dev_docs/best_practices.mdx +++ b/dev_docs/best_practices.mdx @@ -241,8 +241,137 @@ There are some exceptions where a separate repo makes sense. However, they are e It may be tempting to get caught up in the dream of writing the next package which is published to npm and downloaded millions of times a week. Knowing the quality of developers that are working on Kibana, this is a real possibility. However, knowing which packages will see mass adoption is impossible to predict. Instead of jumping directly to writing code in a separate repo and accepting all of the complications that come along with it, prefer keeping code inside the Kibana repo. A [Kibana package](https://github.com/elastic/kibana/tree/master/packages) can be used to publish a package to npm, while still keeping the code inside the Kibana repo. Move code to an external repo only when there is a good reason, for example to enable external contributions. -## Hardening +## Security best practices -Review the following items related to vulnerability and security risks. +When writing code for Kibana, 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. -TODO: copy content from security.asciidoc when review is complete +### Cross-site Scripting (XSS) + +[_OWASP reference for XSS_](https://owasp.org/www-community/attacks/xss) + +XSS is a class of attacks where malicious scripts are injected into vulnerable websites. Kibana defends against this by using the React +framework to safely encode data that is rendered in pages, the EUI framework to +https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive `Content-Security-Policy` +header. + +**Best practices** + +* Check for dangerous functions or assignments that can result in unescaped user input in the browser DOM. Avoid using: + * *React:* [`dangerouslySetInnerHtml`](https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml). + * *Browser DOM:* `Element.innerHTML` and `Element.outerHTML`. +* If using the aforementioned unsafe functions or assignments is absolutely necessary, follow [these XSS prevention +rules](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#xss-prevention-rules) to ensure that +user input is not inserted into unsafe locations and that it is escaped properly. +* Use EUI components to build your UI, particularly when rendering `href` links. Otherwise, sanitize user input before rendering links to +ensure that they do not use the `javascript:` protocol. +* Don't use the `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. +* Be careful when using `setTimeout` and `setInterval` in client-side code. If an attacker can manipulate the arguments and pass a string to +one of these, it is evaluated dynamically, which is equivalent to the dangerous `eval` function. + +### Cross-Site Request Forgery (CSRF/XSRF) + +[_OWASP reference for CSRF_](https://owasp.org/www-community/attacks/csrf) + +CSRF is a class of attacks where a user is forced to execute an action on a vulnerable website that they're logged into, usually without +their knowledge. Kibana defends against this by requiring [custom request +headers](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#use-of-custom-request-headers) +for API endpoints. For more information, see [API Request +Headers](https://www.elastic.co/guide/en/kibana/master/api.html#api-request-headers). + +**Best practices** + +* Ensure all HTTP routes are registered with the [Kibana HTTP service](https://www.elastic.co/guide/en/kibana/master/http-service.html) to +take advantage of the custom request header security control. + * Note that HTTP GET requests do *not* require the custom request header; any routes that change data should [adhere to the HTTP +specification and use a different method (PUT, POST, etc.)](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) + +### Remote Code Execution (RCE) + +[_OWASP reference for Command Injection_](https://owasp.org/www-community/attacks/Command_Injection), +[_OWASP reference for Code Injection_](https://owasp.org/www-community/attacks/Code_Injection) + +RCE is a class of attacks where an attacker executes malicious code or commands on a vulnerable server. Kibana defends against this by using +ESLint rules to restrict vulnerable functions, and by hooking into or hardening usage of these in third-party dependencies. + +**Best practices** + +* Don't use the `eval`, `Function`, and `_.template` functions -- these are restricted by ESLint rules. +* Don't use dynamic `require`. +* Check for usages of templating libraries. Ensure that user-provided input doesn't influence the template and is used only as data for +rendering the template. +* Take extra caution when spawning child processes with any user input or parameters that are user-controlled. + +### Prototype Pollution + +Prototype Pollution is an attack that is unique to JavaScript environments. Attackers can abuse JavaScript's prototype inheritance to +"pollute" objects in the application, which is often used as a vector for XSS or RCE vulnerabilities. Kibana defends against this by +hardening sensitive functions (such as those exposed by `child_process`), and by requiring validation on all HTTP routes by default. + +**Best practices** + +* Check for instances of `anObject[a][b] = c` where `a`, `b`, and `c` are controlled by user input. This includes code paths where the +following logical code steps could be performed in separate files by completely different operations, or by recursively using dynamic +operations. +* Validate all user input, including API URL parameters, query parameters, and payloads. Preferably, use a schema that only allows specific +keys and values. At a minimum, implement a deny-list that prevents `__proto__` and `prototype.constructor` from being used within object +keys. +* When calling APIs that spawn new processes or perform code generation from strings, protect against Prototype Pollution by checking if +`Object.hasOwnProperty` has arguments to the APIs that originate from an Object. An example is the defunct Code app's +[`spawnProcess`](https://github.com/elastic/kibana/blob/b49192626a8528af5d888545fb14cd1ce66a72e7/x-pack/legacy/plugins/code/server/lsp/workspace_command.ts#L40-L44) +function. + * Common Node.js offenders: `child_process.spawn`, `child_process.exec`, `eval`, `Function('some string')`, `vm.runInContext(x)`, +`vm.runInNewContext(x)`, `vm.runInThisContext()` + * Common client-side offenders: `eval`, `Function('some string')`, `setTimeout('some string', num)`, `setInterval('some string', num)` + +See also: + +* [Prototype pollution: The dangerous and underrated vulnerability impacting JavaScript applications | +portswigger.net](https://portswigger.net/daily-swig/prototype-pollution-the-dangerous-and-underrated-vulnerability-impacting-javascript-applications) +* [Prototype pollution attack in NodeJS application | Olivier +Arteau](https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf) + +### Server-Side Request Forgery (SSRF) + +[_OWASP reference for SSRF_](https://owasp.org/www-community/attacks/Server_Side_Request_Forgery) + +SSRF is a class of attacks where a vulnerable server is forced to make an unintended request, usually to an HTTP API. This is often used as +a vector for information disclosure or injection attacks. + +**Best practices** + +* Ensure that all outbound requests from the Kibana server use hard-coded URLs. +* If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that +user input is escaped properly. Ideally, the allow-list should be set in `kibana.yml`, so only server administrators can change it. + * This is particularly relevant when using `transport.request` with the {es} client, as no automatic escaping is performed. + * Note that URLs are very hard to validate properly; exact match validation for user input is most preferable, while URL parsing or RegEx +validation should only be used if absolutely necessary. + +### Reverse tabnabbing + +[_OWASP reference for Reverse Tabnabbing_](https://owasp.org/www-community/attacks/Reverse_Tabnabbing) + +Reverse tabnabbing is an attack where a link to a malicious page is used to rewrite a vulnerable parent page. This is often used as a vector +for phishing attacks. Kibana defends against this by using the EUI framework, which automatically adds the `rel` attribute to anchor tags, +buttons, and other vulnerable DOM elements. + +**Best practices** + +* Use EUI components to build your UI whenever possible. Otherwise, ensure that any DOM elements that have an `href` attribute also have the +`rel="noreferrer noopener"` attribute specified. For more information, refer to the [OWASP HTML5 Security Cheat +Sheet](https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#tabnabbing). +* If using a non-EUI markdown renderer, use a custom link renderer for rendered links. + +### Information disclosure + +Information disclosure is not an attack, but it describes whenever sensitive information is accidentally revealed. This can be configuration +info, stack traces, or other data that the user is not authorized to access. This concern cannot be addressed with a single security +control, but at a high level, Kibana relies on the hapi framework to automatically redact stack traces and detailed error messages in HTTP +5xx response payloads. + +**Best practices** + +* Look for instances where sensitive information might accidentally be revealed, particularly in error messages, in the UI, and URL +parameters that are exposed to users. +* Make sure that sensitive request data is not forwarded to external resources. For example, copying client request headers and using them +to make an another request could accidentally expose the user's credentials. From 195b5bcb33023c2c1726bb4a33b48a53466f49a7 Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Wed, 16 Jun 2021 14:07:13 -0400 Subject: [PATCH 8/8] Fix formatting and MDX syntax error --- dev_docs/best_practices.mdx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dev_docs/best_practices.mdx b/dev_docs/best_practices.mdx index 82b2a67abcc5b3..d87c6eb618993d 100644 --- a/dev_docs/best_practices.mdx +++ b/dev_docs/best_practices.mdx @@ -251,15 +251,14 @@ Application Security Project (OWASP) references to learn more about these types [_OWASP reference for XSS_](https://owasp.org/www-community/attacks/xss) XSS is a class of attacks where malicious scripts are injected into vulnerable websites. Kibana defends against this by using the React -framework to safely encode data that is rendered in pages, the EUI framework to -https://elastic.github.io/eui/#/navigation/link#link-validation[automatically sanitize links], and a restrictive `Content-Security-Policy` -header. +framework to safely encode data that is rendered in pages, the EUI framework to [automatically sanitize +links](https://elastic.github.io/eui/#/navigation/link#link-validation), and a restrictive `Content-Security-Policy` header. **Best practices** * Check for dangerous functions or assignments that can result in unescaped user input in the browser DOM. Avoid using: - * *React:* [`dangerouslySetInnerHtml`](https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml). - * *Browser DOM:* `Element.innerHTML` and `Element.outerHTML`. + * **React:** [`dangerouslySetInnerHtml`](https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml). + * **Browser DOM:** `Element.innerHTML` and `Element.outerHTML`. * If using the aforementioned unsafe functions or assignments is absolutely necessary, follow [these XSS prevention rules](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#xss-prevention-rules) to ensure that user input is not inserted into unsafe locations and that it is escaped properly. @@ -283,7 +282,7 @@ Headers](https://www.elastic.co/guide/en/kibana/master/api.html#api-request-head * Ensure all HTTP routes are registered with the [Kibana HTTP service](https://www.elastic.co/guide/en/kibana/master/http-service.html) to take advantage of the custom request header security control. - * Note that HTTP GET requests do *not* require the custom request header; any routes that change data should [adhere to the HTTP + * Note that HTTP GET requests do **not** require the custom request header; any routes that change data should [adhere to the HTTP specification and use a different method (PUT, POST, etc.)](https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods) ### Remote Code Execution (RCE) @@ -343,7 +342,7 @@ a vector for information disclosure or injection attacks. * Ensure that all outbound requests from the Kibana server use hard-coded URLs. * If user input is used to construct a URL for an outbound request, ensure that an allow-list is used to validate the endpoints and that user input is escaped properly. Ideally, the allow-list should be set in `kibana.yml`, so only server administrators can change it. - * This is particularly relevant when using `transport.request` with the {es} client, as no automatic escaping is performed. + * This is particularly relevant when using `transport.request` with the Elasticsearch client, as no automatic escaping is performed. * Note that URLs are very hard to validate properly; exact match validation for user input is most preferable, while URL parsing or RegEx validation should only be used if absolutely necessary.