-
Notifications
You must be signed in to change notification settings - Fork 18
Add end-to-end encryption as a principle #535
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
base: main
Are you sure you want to change the base?
Add end-to-end encryption as a principle #535
Conversation
… now called exporting
New Issues (7)Checkmarx found the following issues in this Pull Request
|
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.
This is a great addition! If there is ever any doubt this will plainly explain what we mean, which is one of the main goals of this document!
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
breaking this principle. It is possible for a Bitwarden server to create an authentication token, | ||
contact the Key Connector server, and retrieve key material that will allow decryption of a user's | ||
vault. For these reasons we encourage strict isolation of key connector servers to private networks | ||
and only to be used by advanced self-hosted users. |
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.
question/suggestion: should we consider a more structured approach to this list? Something like:
### Key connector
Key connector is a self-host test only feature that allows an Organization to log in and unlock with
SSO and no password input. We encourage strict isolation of key connector servers to private networks and only to be used by advanced self-hosted users.
#### Scope
This feature is limited to self-hosted instances and requires the administrator to set-up additional services and explicitly enable the feature.
#### Risk
Bitwarden servers can create an authentication token, contact the Key Connector server, and retrieve key material that will allow decryption of a user's vault.
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.
It was brought up outside of github to think about moving these exceptions somewhere else to allow them room to grow into a more defined format. I think I like that, but hesitated to do it here.
I'm not opposed to trying a bit of structure, but I think the top blurb would always be duplicated by the sections here because I don't want to bury the lede of scope and risk
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
Deploying contributing-docs with
|
Latest commit: |
c20ff13
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2e33e982.contributing-docs.pages.dev |
Branch Preview URL: | https://ps-include-end-to-end-encryp.contributing-docs.pages.dev |
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
729d737
to
599521c
Compare
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.
Some minor new comments and answers to previous ones
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
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.
Mostly nits and some smaller suggestions, otherwise I think we can rebase/merge main into this PR and then go ahead and merge
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
docs/architecture/security/principles/01-servers-are-not-trusted.mdx
Outdated
Show resolved
Hide resolved
data is fully isolated from the server and infrastructure. It cannot be read nor expanded outside of | ||
the user's client context. | ||
|
||
This is what we mean when we sometimes refer to "End-to-end encrypted" or "Zero Knowledge." |
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.
nit
This is what we mean when we sometimes refer to "End-to-end encrypted" or "Zero Knowledge." | |
This is what we mean when we sometimes refer to "End-to-end encrypted" or "Zero Knowledge". |
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.
TIL. It looks like this is a difference between American and British editing convention (https://style.mla.org/the-placement-of-a-comma-or-period-after-a-quotation/)
📔 Objective
Adds a definition of what we mean by end-to-end encryption as a security principle.
I moved this to principle 01 simply because I feel its the foundational principle Bitwarden was built on and is necessary to vette Bitwarden when considering it for use.
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes