Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Release
| [`rolling-upgrade-testing.yml`](rolling-upgrade-testing.yml) | Mixed-version perf tests for `thresholdWithEnclave` | Manual |
| [`pr-preview-deploy.yml`](pr-preview-deploy.yml) | Ephemeral PR environments | Workflow call |
| [`pr-preview-destroy.yml`](pr-preview-destroy.yml) | Cleanup PR environments | PR close, label removal, scheduled |
| [`rust-lint.yml`](rust-lint.yml) | `cargo fmt --check` + `cargo clippy -D warnings` + `cargo dylint` | PRs |
| [`rust-lint.yml`](rust-lint.yml) | `cargo fmt --check` + `cargo clippy -D warnings` + `make lint-dylint` | PRs |
| [`common-testing.yml`](common-testing.yml) | Reusable test runner | Workflow call |
| [`wasm-testing.yml`](wasm-testing.yml) | WASM test pipeline | Workflow call |
| [`ci_lint.yml`](ci_lint.yml) | actionlint + zizmor on workflow files | PRs |
Expand Down Expand Up @@ -166,7 +166,7 @@ Generates WASM test fixtures from Rust tests, builds `tkms` and `node-tkms` pack

### `rust-lint.yml`

`cargo fmt --all -- --check`, `cargo clippy --workspace --all-targets --all-features -- -D warnings`, and `cargo dylint --all`. Runs on every PR.
`cargo fmt --all -- --check`, `cargo clippy --workspace --all-targets --all-features -- -D warnings`, and `make lint-dylint`. Runs on every PR. The Makefile centralizes the `DYLINT_RUSTFLAGS` passed to the Dylint rustc driver, turning Dylint warnings to errors while disabling the broad tfhe-rs `serialize_without_versionize` lint.

### `docker-build.yml`

Expand Down
9 changes: 0 additions & 9 deletions .github/workflows/kind-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,6 @@ jobs:
# Verify the installed version
cargo nextest --version

- name: Setup cargo dylint
if: matrix.cargo-check == true
run: |
cargo install cargo-dylint dylint-link --force

- name: clippy and rustfmt versions
if: matrix.cargo-check == true
run: |
Expand All @@ -243,10 +238,6 @@ jobs:
PACKAGE_NAME: 'kms-core-client'
run: cargo clippy --all-targets --all-features --package "${PACKAGE_NAME}" -- -D warnings

- name: Linting dylint
if: matrix.cargo-check == true
run: cargo dylint --all

# ==========================================================================
# Test Execution
# Runs the Kubernetes integration tests using the configured environment
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/rust-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ jobs:
echo "EXECUTING: cargo clippy --workspace --all-targets --all-features -- -D warnings"
cargo clippy --workspace --all-targets --all-features -- -D warnings

- name: Install cargo-dylint
- name: Install Dylint
run: |
echo "EXECUTING: cargo install cargo-dylint dylint-link --locked"
cargo install cargo-dylint dylint-link --locked
echo "EXECUTING: make install-dylint"
make install-dylint
Comment thread
kc1212 marked this conversation as resolved.

- name: Dylint
run: |
echo "EXECUTING: cargo dylint --all"
cargo dylint --all
echo "EXECUTING: make lint-dylint"
make lint-dylint
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ syn = { version = "2.0", features = ["full"] } # Syn macro parsing - MEDIUM RIS
sysinfo = "0.36.1" # System information gathering - MEDIUM RISK: Reputable individual maintainer (GuillaumeGomez), 71M+ downloads
tempfile = "=3.20.0" # Temporary file handling - MEDIUM RISK: Individual maintainers (Stebalien, KodrAus), 345M+ downloads, test-only dependency
test-context = "=0.4.1" # Test context utilities - MEDIUM RISK: Individual maintainers (markhildreth, JasperV), test-only dependency
# NOTE: when changing the tfhe dependency, consider also updating the pinned version of dylint lints from tfhe (see dylint.toml)
tfhe = "=1.6.1" # Fully Homomorphic Encryption library - LOW RISK: Zama
tfhe-csprng = "=0.9.0" # Cryptographically secure PRNG for TFHE - LOW RISK: Zama
tfhe-versionable = "=0.7.0" # TFHE versioning support - LOW RISK: Zama
Expand Down
27 changes: 24 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,33 @@ check-git-lfs:
pull-lfs-files: check-git-lfs
git lfs pull

linting-all:

# `DYLINT_RUSTFLAGS` is consumed by `cargo-dylint` and forwarded to the `rustc`
# driver that runs each Dylint library. Here it turns warnings to errors so Dylint
# findings fail CI, while disabling the tfhe-rs `serialize_without_versionize`
# lint, which is intentionally too broad for this workspace. `-Aunknown-lints`
# is paired with it because `cargo dylint --all` loads multiple lint
# libraries/toolchains; libraries that do not define
# `serialize_without_versionize` would otherwise emit an "unknown lint" warning
# for the command-line allow.
DYLINT_RUSTFLAGS ?= -D warnings -Aunknown-lints -Aserialize_without_versionize

lint:
cargo clippy --all-targets --all-features -- -D warnings

linting-package:
lint-package:
@if [ -z "$(PACKAGE)" ]; then \
echo "Error: PACKAGE is not set. Usage: make clippy-package PACKAGE=<package-name>"; \
echo "Error: PACKAGE is not set. Usage: make lint-package PACKAGE=<package-name>"; \
exit 1; \
fi
cargo clippy --all-targets --all-features --package $(PACKAGE) -- -D warnings

# The nightly toolchain and components needed to build each Dylint library are
# auto-installed by rustup on the first `cargo dylint --all` run, driven by the
# `rust-toolchain` file each library ships (e.g. `tfhe-lints` in the tfhe-rs
# repo at the tag pinned in `dylint.toml`).
install-dylint:
cargo install cargo-dylint dylint-link --locked

lint-dylint:
DYLINT_RUSTFLAGS="$(DYLINT_RUSTFLAGS)" cargo dylint --all
4 changes: 2 additions & 2 deletions ai-docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ enum whose variants are its historical layouts (`V0`, `V1`, …).
`Unversionize` dispatches to the right variant by tag on read. On-disk and
on-wire encoding goes through the pinned-`bincode` wrapper
[bc2wrap](bc2wrap/) so the binary layout is deterministic. Examples of
versioned types: `BackupCiphertextVersioned`,
`InternalCustodianContextVersioned`, `AppKeyBlobVersioned`.
versioned types: `BackupCiphertextVersions`,
`InternalCustodianContextVersions`, `AppKeyBlobVersions`.

**Freeze-and-replay harness.** [backward-compatibility/](backward-compatibility/)
is a separate Cargo workspace (excluded from the root — see [Cargo.toml](Cargo.toml)
Expand Down
12 changes: 10 additions & 2 deletions ai-docs/COMMANDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,23 @@ cargo clippy --all-targets --all-features -- -D warnings
All-features clippy as a Makefile target:

```
make linting-all
make lint
```

Lint a single crate:

```
make linting-package PACKAGE=<crate-name>
make lint-package PACKAGE=<crate-name>
```

Installing and running dylint:

```
make install-dylint
make lint-dylint
```


## Testing

Typical test run — uses the `testing` feature, includes unit and integration tests (some integration tests need Redis running locally):
Expand Down
4 changes: 2 additions & 2 deletions backward-compatibility/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ check-git-lfs:
pull-lfs-files: check-git-lfs
git lfs pull

linting:
lint:
RUSTFLAGS="-Aclippy::doc-lazy-continuation" cargo clippy --all-features -- -D warnings

generate-data: check-duplicate
cargo run --release --features="generate"

check-duplicate:
cargo check
./scripts/check_duplicate_tfhe_versions.sh
./scripts/check_duplicate_tfhe_versions.sh
14 changes: 7 additions & 7 deletions core/grpc/src/rpc_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl fmt::Display for KMSType {

/// The format of what will be stored, and returned in gRPC, as a result of CRS generation in the KMS
#[derive(Clone, Debug, Hash, PartialEq, Eq, Serialize, Deserialize, VersionsDispatch)]
pub enum SignedPubDataHandleInternalVersioned {
pub enum SignedPubDataHandleInternalVersions {
V0(SignedPubDataHandleInternal),
}

Expand All @@ -67,7 +67,7 @@ pub enum SignedPubDataHandleInternalVersioned {
/// It's needed because we are not able to derive versioned
/// for the protobuf type.
#[derive(Clone, Debug, Hash, PartialEq, Eq, Serialize, Deserialize, Versionize)]
#[versionize(SignedPubDataHandleInternalVersioned)]
#[versionize(SignedPubDataHandleInternalVersions)]
pub struct SignedPubDataHandleInternal {
// Digest (the 256-bit hex-encoded value, computed using compute_info/handle)
// This lower-case hex values without the 0x prefix.
Expand Down Expand Up @@ -105,7 +105,7 @@ impl SignedPubDataHandleInternal {
/// for more details.
#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Version)]
#[repr(transparent)]
pub struct CrsGenSignedPubDataHandleInternalWrapper(pub SignedPubDataHandleInternal);
pub struct CrsGenMetadataV0(pub SignedPubDataHandleInternal);

// This function needs to use the non-wasm feature because tonic is not available in wasm builds.
#[cfg(feature = "non-wasm")]
Expand Down Expand Up @@ -190,7 +190,7 @@ pub fn alloy_to_protobuf_domain(domain: &Eip712Domain) -> anyhow::Result<Eip712D
Deserialize,
VersionsDispatch,
)]
pub enum PubDataTypeVersioned {
pub enum PubDataTypeVersions {
V0(PubDataType),
}

Expand All @@ -217,7 +217,7 @@ pub enum PubDataTypeVersioned {
EnumIter,
Versionize,
)]
#[versionize(PubDataTypeVersioned)]
#[versionize(PubDataTypeVersions)]
pub enum PubDataType {
ServerKey,
PublicKey,
Expand Down Expand Up @@ -277,7 +277,7 @@ impl Default for PubDataType {
}

#[derive(Debug, Clone, Serialize, Deserialize, VersionsDispatch)]
pub enum PrivDataTypeVersioned {
pub enum PrivDataTypeVersions {
V0(PrivDataTypeV0),
V1(PrivDataType),
}
Expand All @@ -294,7 +294,7 @@ pub enum PrivDataTypeVersioned {
/// Thus some data may indeed be safe to release publicly, but a malicious replacement could completely
/// compromise the entire system.
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, Serialize, Deserialize, EnumIter, Versionize)]
#[versionize(PrivDataTypeVersioned)]
#[versionize(PrivDataTypeVersions)]
pub enum PrivDataType {
SigningKey,
FheKeyInfo, // Only for the threshold case
Expand Down
41 changes: 30 additions & 11 deletions core/service/src/backup/custodian.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ pub(crate) const HEADER: &str = "ZAMA TKMS SETUP TEST OPERATORS-CUSTODIAN";
pub(crate) const DSEP_BACKUP_CUSTODIAN: DomainSep = *b"BKUPCUST";

#[derive(Clone, Serialize, Deserialize, VersionsDispatch)]
pub enum InternalCustodianRecoveryOutputVersioned {
pub enum InternalCustodianRecoveryOutputVersions {
V0(InternalCustodianRecoveryOutput),
}

/// This is the message that a custodian sends to an operator after starting recovery.
///
/// The payload of the signcryption is a `BackupMaterial` that contains the decrypted backup share for an operator.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Versionize)]
#[versionize(InternalCustodianRecoveryOutputVersioned)]
#[versionize(InternalCustodianRecoveryOutputVersions)]
pub struct InternalCustodianRecoveryOutput {
pub signcryption: UnifiedSigncryption,
pub custodian_role: Role,
Expand Down Expand Up @@ -89,13 +89,13 @@ impl TryFrom<InternalCustodianRecoveryOutput> for CustodianRecoveryOutput {
}

#[derive(Clone, Serialize, Deserialize, VersionsDispatch)]
pub enum CustodianSetupMessagePayloadVersioned {
pub enum CustodianSetupMessagePayloadVersions {
V0(CustodianSetupMessagePayload),
}

/// This is payload in the setup message that the custodian sends to the operators.
#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Versionize)]
#[versionize(CustodianSetupMessagePayloadVersioned)]
#[versionize(CustodianSetupMessagePayloadVersions)]
pub struct CustodianSetupMessagePayload {
pub header: String,
pub random_value: [u8; 32],
Expand All @@ -109,7 +109,7 @@ impl Named for CustodianSetupMessagePayload {
}

#[derive(Clone, Serialize, Deserialize, VersionsDispatch)]
pub enum InternalCustodianSetupMessageVersioned {
pub enum InternalCustodianSetupMessageVersions {
V0(InternalCustodianSetupMessage),
}

Expand All @@ -120,7 +120,7 @@ pub enum InternalCustodianSetupMessageVersioned {
/// The operators need to persist this message in their storage
/// so that they can run the backup procedure when needed.
#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Versionize)]
#[versionize(InternalCustodianSetupMessageVersioned)]
#[versionize(InternalCustodianSetupMessageVersions)]
pub struct InternalCustodianSetupMessage {
pub header: String,
pub custodian_role: Role,
Expand Down Expand Up @@ -177,13 +177,13 @@ impl TryFrom<InternalCustodianSetupMessage> for CustodianSetupMessage {
}

#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, VersionsDispatch)]
pub enum InternalCustodianContextVersioned {
pub enum InternalCustodianContextVersions {
V0(InternalCustodianContext),
}

/// This is the internal representation of the custodian context.
#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Versionize)]
#[versionize(InternalCustodianContextVersioned)]
#[versionize(InternalCustodianContextVersions)]
pub struct InternalCustodianContext {
/// The custodian threshold for recovery
pub threshold: u32,
Expand Down Expand Up @@ -284,19 +284,38 @@ impl Custodian {
})
}

/// Obtain the operator ephemeral public key for reencryption,
/// unsigncrypt the signcryption encrypted under the custodian's public key
/// and then signcrypt it it under the operator's public key
// We allow the following lints because we are fine with mutating the rng even if
// we end up returning an error when signing the encrypted share.
#[allow(unknown_lints)]
#[allow(non_local_effect_before_error_return)]
/// Obtain the operator ephemeral public key for reencryption,
/// unsigncrypt the signcryption encrypted under the custodian's public key
/// and then signcrypt it it under the operator's public key
pub fn verify_reencrypt<R: Rng + CryptoRng>(
&self,
rng: &mut R,
backup: &InnerOperatorBackupOutput,
operator_verification_key: &PublicSigKey,
operator_ephem_enc_key: &UnifiedPublicEncKey,
) -> Result<InternalCustodianRecoveryOutput, BackupError> {
self.verify_reencrypt_inner(
rng,
backup,
operator_verification_key,
operator_ephem_enc_key,
)
}

// Keep the body private so the Dylint public API pass does not exhaust its
// path-search work limit on this deliberately allowed pattern.
#[allow(unknown_lints)]
#[allow(non_local_effect_before_error_return)]
fn verify_reencrypt_inner<R: Rng + CryptoRng>(
&self,
rng: &mut R,
backup: &InnerOperatorBackupOutput,
operator_verification_key: &PublicSigKey,
operator_ephem_enc_key: &UnifiedPublicEncKey,
) -> Result<InternalCustodianRecoveryOutput, BackupError> {
tracing::debug!(
"Verifying and re-encrypting backup for operator: {}",
Expand Down
4 changes: 2 additions & 2 deletions core/service/src/backup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ pub const KMS_CUSTODIAN: &str = "kms-custodian";
pub const SEED_PHRASE_DESC: &str = "The SECRET seed phrase for the custodian keys is: ";

#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, VersionsDispatch)]
pub enum BackupCiphertextVersioned {
pub enum BackupCiphertextVersions {
V0(BackupCiphertext),
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Versionize)]
#[versionize(BackupCiphertextVersioned)]
#[versionize(BackupCiphertextVersions)]
pub struct BackupCiphertext {
pub ciphertext: UnifiedCipher,
pub priv_data_type: PrivDataType,
Expand Down
Loading
Loading