-
Notifications
You must be signed in to change notification settings - Fork 400
support kms and encryption #425
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?
Conversation
rustfs/Cargo.toml
Outdated
@@ -56,6 +58,8 @@ atomic_enum = { workspace = true } | |||
axum.workspace = true | |||
async-trait = { workspace = true } | |||
bytes = { workspace = true } | |||
base64 = "0.21" | |||
md5 = "0.7" |
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.
Change to apply version management from the root directory cargo.toml
,rand
,base64
,md5
@@ -0,0 +1,179 @@ | |||
use prometheus::{Counter, CounterVec, Histogram, HistogramVec, Registry}; |
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.
The absence of prometheus
in Cargo.toml
and the suggested use of opentelemetry
are as follows:
let meter = global::meter_with_scope(scope);
let counter = meter
.u64_counter("test_counter")
.with_description("a simple counter for demo purposes. ")
.with_unit("my_unit")
.build();
for _ in 0.. 10 {
counter.add(1, &[KeyValue::new("test_key", "test_value")]);
}
And one of the following quick methods:
To publish a new metric, add a key-value pair to your tracing::Event
that
contains following prefixes:
monotonic_counter.
(non-negative numbers): Used when the counter should
only ever increasecounter.
: Used when the counter can go up or downhistogram.
: Used to report arbitrary values that are likely to be statistically meaningfulgauge.
: Used to report instantaneous values that can go up or down
Examples:
# use tracing::info;
info!(monotonic_counter.foo = 1);
info!(monotonic_counter.bar = 1.1);
info!(counter.baz = 1);
info!(counter.baz = -1);
info!(counter.xyz = 1.1);
info!(histogram.qux = 1);
info!(histogram.abc = -1);
info!(histogram.def = 1.1);
info!(gauge.foo = 1);
info!(gauge.bar = 1.1);
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Replace exact floating point comparisons with approximate equality checks in test_operation_metrics to handle minor precision differences in success_rate calculations. This prevents intermittent test failures due to floating point arithmetic precision issues. Modified file: crates/kms/src/monitoring.rs Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
…S and bucket encryption usage documents Delete the old versions of kms.md, bucket-encryption.md, object_encryption.md and encryption_api.md documents Add new Chinese documents: docs/zh-cn/kms-api-usage.md and docs/zh-cn/bucket-encryption-usage.md Add new English document: docs/en/kms-api-usage.md Signed-off-by: junxiang Mu <1948535941@qq.com>
- Introduced KMS configuration endpoint with support for Vault and Local backends. - Implemented key management APIs: create, enable, disable, rotate, and list keys. - Added rewrap functionality for ciphertext and batch rewrap for objects in a bucket. - Enhanced SSE support for objects with KMS and S3-compatible encryption headers. - Updated documentation for KMS and SSE usage, including curl examples. - Improved error handling and response structures for KMS operations. Signed-off-by: junxiang Mu <1948535941@qq.com>
- Updated KMS documentation to include detailed configuration options, request body fields, and error handling. - Added support for SSE-C (Server-Side Encryption with Customer-Provided Keys) in the object storage implementation, including key validation, IV handling, and metadata persistence. - Improved encryption service integration for both SSE-KMS and SSE-C, ensuring proper decryption and encryption flows. - Introduced error handling for invalid SSE-C configurations and mismatched MD5 checks. - Enhanced multipart upload handling to support SSE-C and KMS encryption, including metadata management for encryption context. - Added new dependencies for MD5 hashing and random number generation. Signed-off-by: junxiang Mu <1948535941@qq.com>
- Updated KMS documentation to reflect changes in internal metadata fields and encryption context handling. - Modified KMS handler to prefer internal sealed context for encryption operations. - Removed unused imports and legacy code related to streaming encryption. - Enhanced SSE-C handling by integrating customer key decryption with the encryption service. - Streamlined metadata management for encryption, ensuring internal fields are not exposed in responses. - Added validation for KMS keys according to bucket encryption policies. - Improved error handling and logging for encryption operations. Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
…r and adjust error status codes Signed-off-by: junxiang Mu <1948535941@qq.com>
…ard compatibility with query parameters Signed-off-by: junxiang Mu <1948535941@qq.com>
…tion Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
Signed-off-by: junxiang Mu <1948535941@qq.com>
…onsistency Signed-off-by: junxiang Mu <1948535941@qq.com>
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.
Pull Request Overview
This PR adds comprehensive KMS (Key Management Service) and server-side encryption (SSE) support to RustFS, enabling S3-compatible object encryption with multiple backend options and administrative management capabilities.
Key changes include:
- Integration of KMS functionality with Vault Transit and Local backends for master key management
- Implementation of S3-compatible SSE-S3, SSE-KMS, and SSE-C encryption methods for object storage
- Addition of comprehensive admin APIs for KMS configuration, key management, and batch operations
- Extensive documentation covering usage patterns, API references, and operational guides
Reviewed Changes
Copilot reviewed 75 out of 86 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
rustfs/src/audit/encryption.rs | New encryption audit logging system for tracking encryption operations and compliance |
rustfs/src/admin/mod.rs | Added KMS admin API routes and handlers integration |
rustfs/src/admin/handlers/kms.rs | Complete KMS admin API implementation with key management and batch operations |
rustfs/src/admin/handlers/event.rs | Updated notification system comment for clarity |
rustfs/src/admin/handlers/bucket_meta.rs | Added bucket creation event notifications for site replication |
rustfs/src/admin/handlers.rs | Added KMS handlers module declaration |
rustfs/Cargo.toml | Added KMS dependencies and cryptographic libraries |
docs/zh-cn/kms.md | Comprehensive Chinese documentation for KMS and SSE usage |
docs/zh-cn/kms-internal.md | Internal design documentation for developers |
docs/zh-cn/kms-api-usage.md | API usage guide with examples |
docs/zh-cn/bucket-encryption-usage.md | Bucket encryption configuration and usage guide |
docs/en/kms.md | English version of KMS documentation |
docs/en/kms-internal.md | English internal design documentation |
docs/en/kms-api-usage.md | English API usage guide |
docs/en/bucket-encryption-usage.md | English bucket encryption guide |
crates/utils/src/retry.rs | Added feature gate for retry functionality |
crates/utils/src/lib.rs | Updated retry module feature gating |
crates/rio/src/lib.rs | Removed deprecated encrypt/decrypt readers |
crates/rio/src/hash_reader.rs | Removed encryption-related test code |
crates/rio/src/etag.rs | Removed encrypt reader tests and references |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.unwrap() | ||
.as_secs(); |
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.
Using unwrap()
on duration_since(UNIX_EPOCH)
can panic if the system time is set before Unix epoch. Consider using unwrap_or(0)
or handling the error gracefully.
.unwrap() | |
.as_secs(); | |
.map(|d| d.as_secs()) | |
.unwrap_or(0); |
Copilot uses AI. Check for mistakes.
let json = serde_json::to_string(self).unwrap_or_default(); | ||
event!(level, audit.encryption = %json, "Encryption audit event"); |
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.
Using unwrap_or_default()
for JSON serialization will return an empty string on failure, which may not be appropriate for audit logs. Consider logging the serialization error instead.
let json = serde_json::to_string(self).unwrap_or_default(); | |
event!(level, audit.encryption = %json, "Encryption audit event"); | |
match serde_json::to_string(self) { | |
Ok(json) => { | |
event!(level, audit.encryption = %json, "Encryption audit event"); | |
} | |
Err(e) => { | |
event!( | |
Level::ERROR, | |
error = %e, | |
"Failed to serialize EncryptionAuditEvent for audit logging" | |
); | |
} | |
} |
Copilot uses AI. Check for mistakes.
.with_user_id(user_id.unwrap_or_default()) | ||
.with_request_id(request_id.unwrap_or_default()) |
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.
[nitpick] Using unwrap_or_default()
converts None
to empty strings, which may not be semantically correct for audit logs. Consider using unwrap_or_else(|| \"unknown\".to_string())
or similar to indicate missing values explicitly.
.with_user_id(user_id.unwrap_or_default()) | |
.with_request_id(request_id.unwrap_or_default()) | |
.with_user_id(user_id.unwrap_or_else(|| "unknown".to_string())) | |
.with_request_id(request_id.unwrap_or_else(|| "unknown".to_string())) |
Copilot uses AI. Check for mistakes.
KmsErrorResponse { | ||
code: "OperationNotSupported".to_string(), | ||
message: "Disable is not supported by Vault transit engine".to_string(), | ||
description: "Vault Transit 不支持显式禁用密钥,如需阻止使用请调整策略或移除密钥".to_string(), |
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 error message is in Chinese but should be in English for consistency with the codebase. Consider using: "Vault Transit does not support explicitly disabling keys; adjust policies or remove keys to restrict usage".
description: "Vault Transit 不支持显式禁用密钥,如需阻止使用请调整策略或移除密钥".to_string(), | |
description: "Vault Transit does not support explicitly disabling keys; adjust policies or remove keys to restrict usage".to_string(), |
Copilot uses AI. Check for mistakes.
if backend.eq_ignore_ascii_case("vault") { | ||
return Ok(kms_error_response( | ||
StatusCode::NOT_IMPLEMENTED, | ||
KmsErrorResponse { | ||
code: "OperationNotSupported".to_string(), | ||
message: "Disable is not supported by Vault transit engine".to_string(), | ||
description: "Vault Transit 不支持显式禁用密钥,如需阻止使用请调整策略或移除密钥".to_string(), | ||
}, | ||
)); |
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.
[nitpick] String comparison for backend type checking is fragile. Consider using an enum or constant for backend types to make this more maintainable and type-safe.
if backend.eq_ignore_ascii_case("vault") { | |
return Ok(kms_error_response( | |
StatusCode::NOT_IMPLEMENTED, | |
KmsErrorResponse { | |
code: "OperationNotSupported".to_string(), | |
message: "Disable is not supported by Vault transit engine".to_string(), | |
description: "Vault Transit 不支持显式禁用密钥,如需阻止使用请调整策略或移除密钥".to_string(), | |
}, | |
)); | |
match KmsBackendType::from_str(&backend) { | |
KmsBackendType::Vault => { | |
return Ok(kms_error_response( | |
StatusCode::NOT_IMPLEMENTED, | |
KmsErrorResponse { | |
code: "OperationNotSupported".to_string(), | |
message: "Disable is not supported by Vault transit engine".to_string(), | |
description: "Vault Transit 不支持显式禁用密钥,如需阻止使用请调整策略或移除密钥".to_string(), | |
}, | |
)); | |
} | |
_ => {} |
Copilot uses AI. Check for mistakes.
Type of Change
Related Issues
Summary of Changes
Checklist
make pre-commit
Impact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.