Skip to content
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

Update security docs #87215

Merged
merged 7 commits into from
Jan 5, 2021
Merged

Update security docs #87215

merged 7 commits into from
Jan 5, 2021

Conversation

jportner
Copy link
Contributor

@jportner jportner commented Jan 4, 2021

Summary

Resolves several open docs issues: #81533, #81531, and #85702.

Docs preview: https://kibana_87215.docs-preview.app.elstc.co/diff

Notes:

  • P12 support was not added to Kibana until 7.6, but the master branch docs assumes that P12 is supported so the docs backports to 7.5 and prior will need to be tweaked a bit.
  • In the docs for 7.3 and prior, the Saved Objects page does not include a section for "Copy to other Kibana spaces", so the backports to 7.3 and prior will need to be tweaked a bit.

Copy link
Contributor Author

@jportner jportner left a comment

Choose a reason for hiding this comment

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

Added some comments with a few notes about the changes.

the NDJSON includes related objects. Exported dashboards include their associated index patterns.
the NDJSON includes related child objects. Exported dashboards include their associated index patterns.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't really say "related child objects" anywhere else in Kibana as far as I can tell. The only place where child/parent relations appear to be described is in the Saved Objects Management page (click a row action to view related objects).

I figure that for the purposes of docs, "related child objects" is fine, unless someone has a better idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

By default, the NDJSON includes child objects that are related to the saved objects.

Comment on lines 32 to 31
You may choose to generate a certificate signing request (CSR) and private key using the {ref}/certutil.html[`elasticsearch-certutil`] tool.
You may choose to generate a signed certificate and private key using the [`elasticsearch-certutil`] tool.
Copy link
Contributor Author

@jportner jportner Jan 4, 2021

Choose a reason for hiding this comment

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

As suggested by Tim in the linked comment, I rewrote these instructions to simply generate a certificate for local usage, with an added note that it is not appropriate for a production environment. I think this suffices, but it's really not any less wordy.

To make this a bit more concise we could also consider changing the "Encrypt traffic between the browser and Kibana" section:

  • Move said note to the bottom of the section
  • Remove Step 2b for PEM certificates (since our example now explicitly generates a P12 file)

Edit: we could also change the "Encrypt traffic between Kibana and Elasticsearch" section and remove Step 3b completely, since we already mentioned how to extract the CA certificate from a P12 file (and that is likely a better option in the most common cases).

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'm personally OK with the length, and I like the simpler steps to generate a certificate for local usage, too

Comment on lines -41 to +42
The password for the built-in `kibana_system` user is typically set as part of
the security configuration process on {es}. For more information, see
{ref}/built-in-users.html[Built-in users].
NOTE: The password for the built-in `kibana_system` user is typically set as part of the security configuration process on {es}. For more
information, see {ref}/built-in-users.html[Built-in users].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to a Note admonition, but didn't modify the text.

Comment on lines 77 to 85
. [[kibana-roles]]Choose an authentication mechanism and grant users the privileges they need to
use {kib}.
. Temporarily log into {kib} using the built-in `elastic` superuser so you can create new users and assign roles. If you are running {kib}
locally, go to `https://localhost:5601` to view the login page.
+
--
For more information on Basic Authentication and additional methods of
authenticating {kib} users, see <<kibana-authentication>>.
NOTE: The password for the built-in `elastic` user is typically set as part of the security configuration process on {es}. For more
information, see {ref}/built-in-users.html[Built-in users].

To manage privileges, open the main menu, then click *Stack Management > Roles*.
. [[kibana-roles]]Create roles and users to grant access to {kib}.
+
--
To manage privileges in {kib}, open the main menu, then click *Stack Management > Roles*. The built-in `kibana_admin` role will grant
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made two changes here:

  • Added a new bullet that clarifies the user should temporarily log into Kibana as the superuser
  • Changed the "Choose an authentication mechanism and grant users the privileges they need to use {kib}." bullet to "Create roles and users to grant access to {kib}.", keeping the anchor intact

It seems to me that "Choose an authentication mechanism" is out of place here and not really appropriate for a quick-start guide.

Copy link
Member

Choose a reason for hiding this comment

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

I like both of these changes, and I agree that choosing an authentication mechanism is a bit out of place

@jportner jportner marked this pull request as ready for review January 4, 2021 19:47
@jportner jportner requested review from gchaps and legrego January 4, 2021 19:47
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

Thanks for taking these on, Joe! I like the changes you've made here.

@arisonl I know you recently configured a stack locally to use TLS, so the pain is fresh in your mind. What do you think about these doc improvements?

@lockewritesdocs FYSA - I know you were working on security docs as well

--------------------------------------------------------------------------------

TIP: If your PKCS#12 file isn't protected with a password, depending on how it was generated, you may need to set
Copy link
Member

Choose a reason for hiding this comment

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

Just tell me I'm being lazy and I'll test it myself, but does the PKCS#12 file generated by
elasticsearch-certutil require the empty string when no password is provided, or is it more nuanced than that? If the one we generate requires the empty string, then we might want to make that more explicit here

Copy link
Contributor Author

@jportner jportner Jan 4, 2021

Choose a reason for hiding this comment

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

Good question! I alluded to the difference here, but didn't specify what elasticsearch-certutil itself does when you don't enter a password.

I was pretty sure that would result in a keystore with an empty password, but I wanted to test and make sure. By manually replacing the files in packages/kbn-dev-utils/certs and rerunning the unit tests in src/core/server/utils/crypto/pkcs12.test.ts, I verified that is indeed the case.

How do you feel about changing the tip to this?

TIP: If you used elasticsearch-certutil to generate a PKCS#12 file and you did not specify a password, the file is encrypted, and you need to set server.ssl.keystore.password to an empty string.

There are other ways this would be needed (such as using OpenSSL, as described in the link above) but I figure we should keep it concise here.

Edit: changed in 11c2685.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for taking the time to verify this. I'm happy with the updated tip text

docs/user/security/securing-communications/index.asciidoc Outdated Show resolved Hide resolved
docs/user/security/securing-kibana.asciidoc Outdated Show resolved Hide resolved
docs/user/security/securing-communications/index.asciidoc Outdated Show resolved Hide resolved
Comment on lines 32 to 31
You may choose to generate a certificate signing request (CSR) and private key using the {ref}/certutil.html[`elasticsearch-certutil`] tool.
You may choose to generate a signed certificate and private key using the [`elasticsearch-certutil`] tool.
Copy link
Member

Choose a reason for hiding this comment

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

I'm personally OK with the length, and I like the simpler steps to generate a certificate for local usage, too

Comment on lines 77 to 85
. [[kibana-roles]]Choose an authentication mechanism and grant users the privileges they need to
use {kib}.
. Temporarily log into {kib} using the built-in `elastic` superuser so you can create new users and assign roles. If you are running {kib}
locally, go to `https://localhost:5601` to view the login page.
+
--
For more information on Basic Authentication and additional methods of
authenticating {kib} users, see <<kibana-authentication>>.
NOTE: The password for the built-in `elastic` user is typically set as part of the security configuration process on {es}. For more
information, see {ref}/built-in-users.html[Built-in users].

To manage privileges, open the main menu, then click *Stack Management > Roles*.
. [[kibana-roles]]Create roles and users to grant access to {kib}.
+
--
To manage privileges in {kib}, open the main menu, then click *Stack Management > Roles*. The built-in `kibana_admin` role will grant
Copy link
Member

Choose a reason for hiding this comment

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

I like both of these changes, and I agree that choosing an authentication mechanism is a bit out of place

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM, with a few minor comments.

the NDJSON includes related objects. Exported dashboards include their associated index patterns.
the NDJSON includes related child objects. Exported dashboards include their associated index patterns.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

By default, the NDJSON includes child objects that are related to the saved objects.

docs/user/security/securing-communications/index.asciidoc Outdated Show resolved Hide resolved
docs/user/security/securing-kibana.asciidoc Outdated Show resolved Hide resolved
docs/user/security/securing-kibana.asciidoc Outdated Show resolved Hide resolved
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM once Gail's comments are addressed. Thanks for taking the time to fix these docs issues!

Comment on lines 169 to 170
TIP: If you used `elasticsearch-certutil` to generate a PKCS#12 file and you did not specify a password, the file is encrypted, and you need
to set server.ssl.keystore.password to an empty string.
NOTE: If you used `elasticsearch-certutil` to generate a PKCS#12 file and you did not specify a password, the file is encrypted, and you
need to set `server.ssl.truststore.password` to an empty string.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I was adding the code tags I noticed that server.ssl.keystore.password was wrong, it should have been server.ssl.truststore.password this whole time 😅

jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit to jportner/kibana that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
jportner added a commit that referenced this pull request Jan 5, 2021
jportner added a commit that referenced this pull request Jan 5, 2021
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
jportner added a commit that referenced this pull request Jan 5, 2021
# Conflicts:
#	docs/management/managing-saved-objects.asciidoc
#	docs/user/security/securing-communications/index.asciidoc
#	docs/user/security/securing-kibana.asciidoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants