Skip to content

Commit 2abfc9b

Browse files
MB-68168: Make sure DEK reencryption uses the same
chronicle snapshot as the one used in ensure_all_keks_on_disk Otherwise, the following scenario is possible: 1. chronicle data changes (triggers ensure_all_keks_on_disk and DEK reencryption) 2. ensure_all_keks_on_disk is called 3. chronicle data changes again (it again requires ensure_all_keks_on_disk and DEK reencryption) 4. DEK reencryption is called (triggered by change #1), but chronicle already contains changes from #3, and some keks are probably not on disk yet Change-Id: I5a526fa58c0a7be779ed737fa1ca29340f1abc0f Reviewed-on: https://review.couchbase.org/c/ns_server/+/233051 Reviewed-by: Navdeep S Boparai <[email protected]> Tested-by: Timofey Barmin <[email protected]> Tested-by: Build Bot <[email protected]> Well-Formed: Build Bot <[email protected]> Well-Formed: Restriction Checker
1 parent a141fbb commit 2abfc9b

File tree

1 file changed

+37
-22
lines changed

1 file changed

+37
-22
lines changed

apps/ns_server/src/cb_cluster_secrets.erl

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,18 +1420,24 @@ add_active_key(Id, #{id := KekId} = Kek,
14201420

14211421
-spec ensure_all_keks_on_disk(#state{}) ->
14221422
{ok, #state{}} | {error, #state{}, list()}.
1423-
ensure_all_keks_on_disk(#state{kek_hashes_on_disk = Vsns} = State) ->
1424-
{RV, NewVsns} = persist_keks(Vsns),
1423+
ensure_all_keks_on_disk(State) ->
1424+
ensure_all_keks_on_disk(State, direct).
1425+
1426+
-spec ensure_all_keks_on_disk(#state{}, chronicle_snapshot()) ->
1427+
{ok, #state{}} | {error, #state{}, list()}.
1428+
ensure_all_keks_on_disk(#state{kek_hashes_on_disk = Vsns} = State, Snapshot) ->
1429+
{RV, NewVsns} = persist_keks(Vsns, Snapshot),
14251430
NewState = State#state{kek_hashes_on_disk = NewVsns},
14261431
case RV of
14271432
ok -> {ok, NewState};
14281433
{error, Reason} -> {error, NewState, Reason}
14291434
end.
14301435

1431-
-spec persist_keks(Hashes) ->
1436+
-spec persist_keks(Hashes, Snapshot) ->
14321437
{ok, Hashes} |
1433-
{{error, term()}, Hashes} when Hashes :: #{secret_id() => integer()}.
1434-
persist_keks(Hashes) ->
1438+
{{error, term()}, Hashes} when Hashes :: #{secret_id() => integer()},
1439+
Snapshot :: chronicle_snapshot().
1440+
persist_keks(Hashes, Snapshot) ->
14351441
Write = fun (#{type := ?CB_MANAGED_KEY_TYPE} = SecretProps) ->
14361442
ensure_cb_managed_keks_on_disk(SecretProps, false);
14371443
(#{type := ?AWSKMS_KEY_TYPE} = SecretProps) ->
@@ -1442,7 +1448,7 @@ persist_keks(Hashes) ->
14421448
ok
14431449
end,
14441450

1445-
{ok, AllSecrets} = topologically_sorted_secrets(get_all()),
1451+
{ok, AllSecrets} = topologically_sorted_secrets(get_all(Snapshot)),
14461452

14471453
{RV, NewHashes} = lists:mapfoldl(
14481454
fun (#{id := Id, name := Name} = S, Acc) ->
@@ -2152,9 +2158,11 @@ maybe_read_deks(#state{} = State) ->
21522158
#{cb_deks:dek_kind() => deks_info()},
21532159
[term()]}.
21542160
init_deks() ->
2155-
Deks = read_all_deks(),
2161+
Snapshot = chronicle_compat:get_snapshot([fun fetch_snapshot_in_txn/1],
2162+
#{}),
2163+
Deks = read_all_deks(Snapshot),
21562164
KekPushHashes =
2157-
case persist_keks(#{}) of
2165+
case persist_keks(#{}, Snapshot) of
21582166
{ok, H} -> H;
21592167
{{error, Reason}, H} ->
21602168
%% Some Keks may have been written so we use the updated state
@@ -2167,7 +2175,7 @@ init_deks() ->
21672175
{ReencryptedDeksList, Errors} =
21682176
lists:mapfoldl(
21692177
fun ({Kind, KindDeks}, Acc) ->
2170-
case reencrypt_deks(Kind, KindDeks) of
2178+
case reencrypt_deks(Kind, KindDeks, Snapshot) of
21712179
no_change ->
21722180
{{Kind, KindDeks}, Acc};
21732181
{changed, NewKindDeks, Errors} ->
@@ -2180,8 +2188,9 @@ init_deks() ->
21802188
ReencryptedDeks = maps:from_list(ReencryptedDeksList),
21812189
{KekPushHashes, ReencryptedDeks, Errors}.
21822190

2183-
-spec read_all_deks() -> #{cb_deks:dek_kind() => deks_info()}.
2184-
read_all_deks() ->
2191+
-spec read_all_deks(chronicle_snapshot()) ->
2192+
#{cb_deks:dek_kind() => deks_info()}.
2193+
read_all_deks(Snapshot) ->
21852194
GetCfgDek = encryption_service:read_dek(configDek, _),
21862195
VerifyMac = fun encryption_service:verify_mac/2,
21872196
{ok, Term} = cb_deks_raw_utils:read_deks_file(deks_file_path(), GetCfgDek,
@@ -2191,7 +2200,6 @@ read_all_deks() ->
21912200
fun (Kind, #{is_enabled := IsEnabled,
21922201
active_id := ActiveId,
21932202
dek_ids := DekIds}) ->
2194-
Snapshot = deks_config_snapshot(Kind),
21952203
case call_dek_callback(encryption_method_callback, Kind,
21962204
[node, Snapshot]) of
21972205
{succ, {ok, _}} ->
@@ -2293,26 +2301,33 @@ generate_new_dek(Kind, CurrentDeks, EncryptionMethod, Snapshot) ->
22932301
maybe_reencrypt_deks(Kind, #state{deks_info = Deks} = State) ->
22942302
case maps:find(Kind, Deks) of
22952303
{ok, KindDeks} ->
2296-
case reencrypt_deks(Kind, KindDeks) of
2297-
no_change -> {ok, State};
2304+
Snapshot = deks_config_snapshot(Kind),
2305+
NewState = case ensure_all_keks_on_disk(State, Snapshot) of
2306+
{ok, NS} -> NS;
2307+
{error, NS, EnsureErrors} ->
2308+
?log_error("Failed to ensure all keks on "
2309+
"disk: ~p", [EnsureErrors]),
2310+
NS
2311+
end,
2312+
case reencrypt_deks(Kind, KindDeks, Snapshot) of
2313+
no_change -> {ok, NewState};
22982314
{changed, NewKindDeks, Errors} ->
2299-
NewState =
2300-
State#state{deks_info = Deks#{Kind => NewKindDeks}},
2301-
NewState2 = on_deks_update(Kind, NewState),
2315+
NewState2 =
2316+
NewState#state{deks_info = Deks#{Kind => NewKindDeks}},
2317+
NewState3 = on_deks_update(Kind, NewState2),
23022318
case Errors of
2303-
[] -> {ok, NewState2};
2304-
_ -> {error, NewState2, Errors}
2319+
[] -> {ok, NewState3};
2320+
_ -> {error, NewState3, Errors}
23052321
end;
23062322
{error, Errors} ->
2307-
{error, State, Errors}
2323+
{error, NewState, Errors}
23082324
end;
23092325
error ->
23102326
{ok, State}
23112327
end.
23122328

2313-
reencrypt_deks(Kind, #{deks := Keys} = DeksInfo) ->
2329+
reencrypt_deks(Kind, #{deks := Keys} = DeksInfo, Snapshot) ->
23142330
maybe
2315-
Snapshot = deks_config_snapshot(Kind),
23162331
{succ, {ok, EncrMethod}} ?= call_dek_callback(
23172332
encryption_method_callback,
23182333
Kind,

0 commit comments

Comments
 (0)