spetz commented on code in PR #4615:
URL: https://github.com/apache/iggy/pull/4615#discussion_r4340263840
##########
core/server/config.toml:
##########
@@ -421,15 +421,37 @@ sysinfo_print_interval = "10 s"
# Encryption configuration
[encryption]
# Encrypt message payloads and user headers. Metadata and structural headers
remain unencrypted.
-# `true` enables encryption for stored data using AES-256-GCM.
+# `true` enables encryption for stored payloads and user headers using
AES-256-GCM-SIV.
# `false` means data is stored without encryption.
enabled = false
-# The encryption key used when encryption is enabled (string).
-# Should be a 32 bytes length key, provided as a base64 encoded string.
-# This key is required and used only if encryption is enabled.
+# The server encryption key (string), 32 bytes as standard base64.
+# Every encrypted stream has its own data key, stored wrapped under a key
derived from this one.
+# Required when encryption is enabled. With encryption disabled a key may
still be set:
+# streams created with their own key are then encrypted, keyless streams stay
plaintext.
+# A stream created without encryption stays plaintext for life.
key = ""
+# Earlier server keys (list of strings) a key rotation still needs, at most
two.
+# A node unwraps stream keys with them and wraps every new stream key under
them too.
+# Rotation runs in two phases:
+# 1. Set `key` to the new key and `previous_keys` to the old one on every
node. The metadata
+# primary adds a wrap under the new key to every encrypted stream and keeps
the old wraps,
+# so nodes still on the old key keep serving and booting, new streams
included.
+# 2. Once every node runs the phase 1 config and every encrypted stream
reports the new
+# key's fingerprint in `encryption_kek_fingerprint`, empty `previous_keys`
and put the
+# old key's fingerprint in `retired_kek_fingerprints`. The primary drops
the wraps under it.
+# Messages are never re-encrypted. Losing a key that still wraps a stream
makes it unreadable.
+# As an environment variable: IGGY_ENCRYPTION_PREVIOUS_KEYS='["<old base64
key>"]', a single key may also be given bare.
+previous_keys = []
+
+# Fingerprints of retired server keys (list of strings), 16 lowercase hex
characters each, as
Review Comment:
Added the step to clear `retired_kek_fingerprints` once every stream dropped
the old wrap, in `core/server/config.toml:444`, the server README rotation
steps and the helm README `server.encryption` entry.
##########
core/server/src/partition_reconciler.rs:
##########
@@ -748,11 +768,54 @@ async fn reconcile_additions(
// Resolved only for namespaces actually built, not once per committed
// partition every pass. A stream, topic or partition that vanished
// between the target snapshot and this read defers to the next pass.
- let Some((partition_stats, topic_runtime, partition_metadata)) =
- fetch_partition_build_inputs(ctx, ns)
+ let Some(PartitionBuildInputs {
+ stats: partition_stats,
+ runtime: topic_runtime,
+ metadata: partition_metadata,
+ stream_key,
+ encryption,
+ }) = fetch_partition_build_inputs(ctx, ns)
else {
continue;
};
+ let topic_runtime = match topic_runtime {
+ Ok(topic_runtime) => topic_runtime,
+ Err(error) => {
+ ctx.record_failure(ns, FailureCause::Add, now);
+ ctx.shard.metrics().record_partition_reconcile_failure();
+ error!(
+ shard = shard_id,
+ stream_id = ns.stream_id(),
+ topic_id = ns.topic_id(),
+ partition_id = ns.partition_id(),
+ %error,
+ "Partition of topic {} cannot be built, its options hold
critical state this node cannot read",
+ ns.topic_id()
+ );
+ continue;
+ }
+ };
+ // An encrypted partition is built only with its own stream's key. A
+ // key that does not open here leaves the partition unbuilt, never
+ // served as plaintext or under another key.
+ let encryptor = match partitions.resolve_stream_encryptor(stream_key,
&encryption) {
+ Ok(encryptor) => encryptor,
+ Err(error) => {
+ ctx.record_failure(ns, FailureCause::Add, now);
Review Comment:
The fast-skip now arms when no failure record is due for retry, and a pass
that only skipped backed-off namespaces counts as converged.
`given_only_backed_off_add_when_reconciling_should_skip_until_retry_is_due` at
`core/server/src/partition_reconciler.rs:2869` pins it.
##########
foreign/node/src/wire/stream/create-stream.command.ts:
##########
@@ -28,28 +34,48 @@ export type CreateStream = {
streamId?: number,
/** Stream name (1-255 bytes) */
name: string
-};
+} & StreamOptions;
/**
* Create stream command definition.
* Creates a new stream with the specified name.
+ * Layout: `[name_len:u8][name]` followed by the options block running to the
+ * end of the payload.
*/
export const CREATE_STREAM = {
code: COMMAND_CODE.CreateStream,
- serialize: ({name}: CreateStream) => {
+ serialize: ({ name, encryptionKey, options: extraOptions = [] }:
CreateStream) => {
// Stream ID is now auto-assigned by the server, not sent in the protocol
const bName = Buffer.from(name);
if (bName.length < 1 || bName.length > 255)
throw new Error('Stream name should be between 1 and 255 bytes');
+ // A plain entry prints its value wherever the caller logs the arguments.
+ if (extraOptions.some(({ key }) => key === StreamOptionKey.EncryptionKey))
Review Comment:
A raw `encryption_key` entry is now refused as `InvalidCommand` (3) before
the network, so a valid key is no longer reported as invalid
(`foreign/node/src/wire/stream/create-stream.command.ts:59`). The test checks
the code and that no key spelling reaches the message or stack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
