Skip to content

Commit 8f5eb64

Browse files
MB-61292: Make sure we can read all deks at startup
Before this commit we ignored read key errors, because we had to support the case when log dir is removed together with log deks. Now since log deks are stored in config dir, they can't be removed when logs are removed, so it should be save to assume that deks must be always readable. There is another scenario that needs to be kept in mind: say we have a dek encrypted by aws key, and that aws key is unavailable at startup, so we can't read that dek. There are two ways to handle that: 1. Continue to start up, but retry reading deks later; 2. Fail to start up. Option #1 is hard to implement as the code that uses that dek should handle the case when dek is not available. This is another reason why this commit implements option #2. Note that this scenario was not supported before this commit. Change-Id: Ib01c009957ae7f413428b38c6f2c32bb19f193db Reviewed-on: https://review.couchbase.org/c/ns_server/+/221170 Reviewed-by: Navdeep S Boparai <[email protected]> Well-Formed: Build Bot <[email protected]> Tested-by: Timofey Barmin <[email protected]>
1 parent 6c71f7e commit 8f5eb64

File tree

1 file changed

+15
-19
lines changed

1 file changed

+15
-19
lines changed

apps/ns_server/src/cb_cluster_secrets.erl

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,25 +1642,21 @@ read_all_deks(#state{} = State) ->
16421642
fun (Kind, #{is_enabled := IsEnabled,
16431643
active_id := ActiveId,
16441644
dek_ids := DekIds}) ->
1645-
{Keys, Errors} = cb_deks:read(Kind, DekIds),
1646-
case Errors of
1647-
#{ActiveId := Reason} when IsEnabled ->
1648-
?log_warning("Failed to read active ~p DEK: ~p",
1649-
[Kind, Reason]),
1650-
%% Not creating that DEK in configuration
1651-
%% We can't start using undefined as an active key
1652-
%% because this can lead to data being decrypted
1653-
%% which is probably not what we want here. We should
1654-
%% rather show an error or crash instead of silently
1655-
%% decrypting the data.
1656-
%% It is also possible that all deks were removed
1657-
%% intentinally (with the data). In this case we should
1658-
%% not crash but rather ignore these deks at all.
1659-
false;
1660-
#{} ->
1661-
%% Ignoring other key errors and hoping that those keys
1662-
%% will not be needed for data decryption.
1663-
{true, new_dek_info(ActiveId, Keys, IsEnabled)}
1645+
Snapshot = deks_config_snapshot(Kind),
1646+
case call_dek_callback(encryption_method_callback, Kind,
1647+
[Snapshot]) of
1648+
{succ, {ok, _}} ->
1649+
{Keys, Errors} = cb_deks:read(Kind, DekIds),
1650+
case maps:size(Errors) > 0 of
1651+
true ->
1652+
?log_error("Failed to read ~p keys: ~p",
1653+
[Kind, Errors]),
1654+
exit({failed_to_read_keys, Errors});
1655+
false ->
1656+
{true, new_dek_info(ActiveId, Keys, IsEnabled)}
1657+
end;
1658+
{succ, {error, not_found}} ->
1659+
false
16641660
end
16651661
end, Term),
16661662
State#state{deks = Deks}.

0 commit comments

Comments
 (0)