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

chore: move message decryption into Veritech server #3784

Merged
merged 2 commits into from
May 15, 2024

Conversation

fnichol
Copy link
Contributor

@fnichol fnichol commented May 14, 2024

This rather large change move the decryption of function execution requests from the Cyclone server to the Veritech server. Sensitive string redaction still takes place in the Cyclone server.

Prior to this change, portions of the function execution requests would be encrypted before being passed into a Veritech client call using a a public/private keypair called the "Cylone encryption key". The reason for this naming is that the request portions would be decrypted in a Cyclone server before being passed to a language server for final execution.

In order to remove any chance of a user-authored function being able to access such a decryption key, the request decryption activity is now handled back in the Veritech server before it sends the now fully decrypted request over a web socket to a Cyclone server. As a result, there is zero requirement for any message cryptography in the Cyclone server.

The second task that was performed in the Cyclone server was secret substring redaction, using a CycloneSecretStrings Rust type. With this change, the secret substring redaction still takes place in the Cyclone server but now this is realized by adding the set of "sensitive" secrets as part of every Cyclone request. This is how the Cyclone server knows which strings to redact while not participating in the original decryption of the data.

As a result of the change in responsibility, these encryption/decryption keys are now known as "Veritech encryption" and "Veritech decryption" keys. Consequently all variables, CLI arguments, and files in our source tree have been updated to eliminate any possibility of future confusion.

@fnichol
Copy link
Contributor Author

fnichol commented May 14, 2024

bors try

si-bors-ng bot added a commit that referenced this pull request May 14, 2024
@si-bors-ng
Copy link
Contributor

si-bors-ng bot commented May 14, 2024

try

Build failed:

@@ -52,6 +55,8 @@ pub struct Config {

cyclone_spec: CycloneSpec,

decryption_key_path: CanonicalFile,
Copy link
Contributor

@sprutton1 sprutton1 May 14, 2024

Choose a reason for hiding this comment

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

These aren't always paths. We pass them in as base64 strings in prod. SDF Server takes these in using the CryptoConfig object. Can we do that here, too? It will let us keep using our generated config file.

#[builder(default = "CryptoConfig::default()")]
    crypto: CryptoConfig,

Edit: Although, on closer inspection that config struct doesn't have decryption keys in it so I'm not sure how this is working currently

This rather large change move the decryption of function execution
requests from the Cyclone server to the Veritech server. Sensitive
string redaction still takes place in the Cyclone server.

Prior to this change, portions of the function execution requests would
be encrypted before being passed into a Veritech client call using a a
public/private keypair called the "Cylone encryption key". The reason
for this naming is that the request portions would be decrypted in a
Cyclone server before being passed to a language server for final
execution.

In order to remove any chance of a user-authored function being able to
access such a decryption key, the request decryption activity is now
handled back in the Veritech server before it sends the now fully
decrypted request over a web socket to a Cyclone server. As a result,
there is zero requirement for *any* message cryptography in the Cyclone
server.

The second task that was performed in the Cyclone server was secret
substring redaction, using a `CycloneSecretStrings` Rust type. With this
change, the secret substring redaction still takes place in the Cyclone
server but now this is realized by adding the set of "sensitive" secrets
as part of every Cyclone request. This is how the Cyclone server knows
which strings to redact while not participating in the original
decryption of the data.

As a result of the change in responsibility, these encryption/decryption
keys are now known as "Veritech encryption" and "Veritech decryption"
keys. Consequently all variables, CLI arguments, and files in our source
tree have been updated to eliminate any possibility of future confusion.

Signed-off-by: Fletcher Nichol <fletcher@systeminit.com>
@fnichol fnichol force-pushed the fnichol/veritech-decryption branch from 8922d52 to 41ed163 Compare May 14, 2024 19:17
@fnichol
Copy link
Contributor Author

fnichol commented May 14, 2024

bors retry

si-bors-ng bot added a commit that referenced this pull request May 14, 2024
@si-bors-ng
Copy link
Contributor

si-bors-ng bot commented May 14, 2024

try

Build succeeded:

@sprutton1
Copy link
Contributor

bors try

si-bors-ng bot added a commit that referenced this pull request May 14, 2024
@si-bors-ng
Copy link
Contributor

si-bors-ng bot commented May 14, 2024

try

Build succeeded:

@fnichol
Copy link
Contributor Author

fnichol commented May 15, 2024

bors merge

@si-bors-ng
Copy link
Contributor

si-bors-ng bot commented May 15, 2024

Build succeeded:

@si-bors-ng si-bors-ng bot merged commit d8e9f81 into main May 15, 2024
8 checks passed
@si-bors-ng si-bors-ng bot deleted the fnichol/veritech-decryption branch May 15, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants