Skip to content

Jules: Implement encrypted embedded tomes for Golem #937

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

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

Conversation

Cictrone
Copy link
Collaborator

This change introduces encryption for Golem's embedded tomes using AES-256-GCM. The encryption key and nonce are derived at compile time from environment variables GOLEM_ENC_KEY and GOLEM_KDF_SALT via PBKDF2HMAC-SHA256.

Golem's build.rs script now:

  1. Derives the AES key, AES nonce, and KDF salt.
  2. Stores these parameters in crypto_config.rs within $OUT_DIR.
  3. Locates plaintext main.eldritch files in implants/golem/embed_files_golem_prod/.
  4. Encrypts these tomes using the derived key and nonce.
  5. Embeds the resulting ciphertext into the Golem binary using the include_dir crate.

At runtime, Golem requires a --password <PASSWORD> CLI argument to execute embedded tomes.

  1. This password, along with the compile-time KDF salt, is used to derive a candidate AES key.
  2. The candidate key is compared against the compile-time derived AES key.
  3. If the keys match, Golem decrypts and executes the embedded tomes.
  4. If the keys do not match, or if the password is not provided, an appropriate error is issued, and Golem exits.

Tomes provided as direct file path arguments remain unencrypted and are processed as before.

The following dependencies were added to implants/golem/Cargo.toml:

  • aes-gcm, pbkdf2, sha2 for cryptographic operations.
  • rand for general randomness (though nonce is PBKDF2-derived).
  • hex for utility.
  • walkdir and include_dir to support the build-time encryption and embedding process.

Comprehensive tests have been added to implants/golem/tests/cli.rs to cover:

  • Successful decryption with the correct password.
  • Failure scenarios (incorrect password, no password).
  • Continued functionality of plaintext file-based tomes. A test tome was added to implants/golem/embed_files_golem_prod/test_tome/main.eldritch for this purpose.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

This change introduces encryption for Golem's embedded tomes using
AES-256-GCM. The encryption key and nonce are derived at compile time
from environment variables `GOLEM_ENC_KEY` and `GOLEM_KDF_SALT` via
PBKDF2HMAC-SHA256.

Golem's `build.rs` script now:
1.  Derives the AES key, AES nonce, and KDF salt.
2.  Stores these parameters in `crypto_config.rs` within `$OUT_DIR`.
3.  Locates plaintext `main.eldritch` files in
    `implants/golem/embed_files_golem_prod/`.
4.  Encrypts these tomes using the derived key and nonce.
5.  Embeds the resulting ciphertext into the Golem binary using the
    `include_dir` crate.

At runtime, Golem requires a `--password <PASSWORD>` CLI argument to
execute embedded tomes.
1.  This password, along with the compile-time KDF salt, is used to
    derive a candidate AES key.
2.  The candidate key is compared against the compile-time derived AES key.
3.  If the keys match, Golem decrypts and executes the embedded tomes.
4.  If the keys do not match, or if the password is not provided,
    an appropriate error is issued, and Golem exits.

Tomes provided as direct file path arguments remain unencrypted and are
processed as before.

The following dependencies were added to `implants/golem/Cargo.toml`:
- `aes-gcm`, `pbkdf2`, `sha2` for cryptographic operations.
- `rand` for general randomness (though nonce is PBKDF2-derived).
- `hex` for utility.
- `walkdir` and `include_dir` to support the build-time encryption
  and embedding process.

Comprehensive tests have been added to `implants/golem/tests/cli.rs`
to cover:
- Successful decryption with the correct password.
- Failure scenarios (incorrect password, no password).
- Continued functionality of plaintext file-based tomes.
A test tome was added to `implants/golem/embed_files_golem_prod/test_tome/main.eldritch`
for this purpose.
@@ -18,6 +18,13 @@ structopt = { workspace = true }
itertools = { workspace = true }
thiserror = { workspace = true }
lsp-types = { workspace = true }
aes-gcm = { version = "0.10.3", features = ["aes"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move these the workspace cargo.toml and use the dep-name = { workspace = true }


// Store Derived Crypto Parameters
let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
let crypto_config_path = out_dir.join("crypto_config.rs");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems crazy.

use sha2::Sha256;

// Include generated crypto configuration and embedded encrypted files
include!(concat!(env!("OUT_DIR"), "/crypto_config.rs"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you provide an example of what the generated crypto_config.rs will look like?

parsed_tomes.push(ParsedTome { eldritch: tome_contents });
}
Err(e) => {
eprintln!("Failed to convert decrypted tome to UTF-8 string: {}. Skipping.", e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this is the right way to print errors.
Double check how error printing is handled in other places.
Usually we need a cfg_attr(debug) or if it's an error passed up through grpc we use report.error()

@@ -68,6 +82,15 @@ fn main() -> anyhow::Result<()> {
.multiple_occurrences(false)
.required(false),
)
.arg(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably be an environment variable too, may not always want to pass the password in CLI.

@Cictrone
Copy link
Collaborator Author

for a first attempt from jules, i say not completely off the mark? but also some wild choices i agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants