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 Model #10148

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Update Security Model #10148

wants to merge 5 commits into from

Conversation

picatz
Copy link
Contributor

@picatz picatz commented Mar 9, 2021

This PR updates the security model documentation wording, adds official enterprise annotations (EnterpriseAlert), and includes small markdown formatting fixes.

@picatz picatz added theme/docs Documentation issues and enhancements theme/security labels Mar 9, 2021
@picatz picatz requested a review from a team March 9, 2021 18:51
@picatz picatz self-assigned this Mar 9, 2021
So they'll be on the same line when rendered on the site.
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

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

LGTM. Like those Oxford commas ;-)

@angrycub
Copy link
Contributor

I'm on the other side of the fence on the commas. It looks like there are a lot more commas than there should be. Specifically outside of lists >= 3 elements.

@picatz
Copy link
Contributor Author

picatz commented Mar 15, 2021

😅 I can totally see what you mean @angrycub. For whatever reason I decided to be consistent with them after realizing I was being inconsistent overall.

Both are bad, but also good.

Copy link
Contributor

@angrycub angrycub left a comment

Choose a reason for hiding this comment

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

@picatz, I made a bunch of suggestions to hopefully help out with the commas. There are a couple of places where I made some content suggestions which you can use or not. Also, I was doing it in the webui, so hopefully I didn't add a content goof in the editing process.

website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
website/content/docs/internals/security.mdx Outdated Show resolved Hide resolved
Comment on lines +304 to 305
a client node is unencrypted in the agent's data, and configuration
directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit got "wat?" really fast...not because of your edits. I'm not 100% sure of the intention here. Perhaps it's to mention that the data directory and configuration are both stored to disk in the clear.

I think this was kind of the point, but it sort of fell apart at the end for me. I'd love to hear your thoughts on it, @picatz .

  • Client state contains information about the allocations running on a client.
  • The allocation directory, generally a child directory of data_dir but not required to be, can contain logs and files created or read during an allocations lifetime.
  • The configuration directory could have elements that should be protected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention was to highlight that information is stored in the clear on disk on client nodes, but information is encrypted on-the-wire. I think the wording could def be adjusted, just not exactly sure what that wording is yet. 🤔

Maybe:

Client state that contains all information of running allocations is stored in the clear on disk on client nodes. This information is encrypted over-the-wire when communicating with server nodes.

What do you think @angrycub? Maybe it needs to be a bit more verbose?

Going to think a bit more about the other suggestions.

Co-authored-by: Charlie Voiselle <[email protected]>
Still need to answer / think about the other questions.

Co-authored-by: Charlie Voiselle <[email protected]>
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@tgross tgross added the stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows theme/docs Documentation issues and enhancements theme/security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants