iilyak commented on a change in pull request #3568:
URL: https://github.com/apache/couchdb/pull/3568#discussion_r633766290
##########
File path: src/aegis/src/aegis.erl
##########
@@ -33,50 +31,47 @@ init_db(#{} = Db, Options) ->
is_encrypted => aegis_server:init_db(Db, Options)
}.
-
open_db(#{} = Db) ->
Db#{
is_encrypted => aegis_server:open_db(Db)
}.
-
get_db_info(#{is_encrypted := IsEncrypted} = Db) ->
- KeyManagerInfo = case erlang:function_exported(?AEGIS_KEY_MANAGER,
get_db_info, 1) of
- true ->
- ?AEGIS_KEY_MANAGER:get_db_info(Db);
- false ->
- []
- end,
+ KeyManagerInfo =
+ case erlang:function_exported(?AEGIS_KEY_MANAGER, get_db_info, 1) of
Review comment:
I am neutral on this one.
0
##########
File path: src/aegis/src/aegis.erl
##########
@@ -33,50 +31,47 @@ init_db(#{} = Db, Options) ->
is_encrypted => aegis_server:init_db(Db, Options)
}.
-
open_db(#{} = Db) ->
Db#{
is_encrypted => aegis_server:open_db(Db)
}.
-
get_db_info(#{is_encrypted := IsEncrypted} = Db) ->
- KeyManagerInfo = case erlang:function_exported(?AEGIS_KEY_MANAGER,
get_db_info, 1) of
- true ->
- ?AEGIS_KEY_MANAGER:get_db_info(Db);
- false ->
- []
- end,
+ KeyManagerInfo =
+ case erlang:function_exported(?AEGIS_KEY_MANAGER, get_db_info, 1) of
+ true ->
+ ?AEGIS_KEY_MANAGER:get_db_info(Db);
+ false ->
+ []
+ end,
[{enabled, IsEncrypted}, {key_manager, {KeyManagerInfo}}].
-
encrypt(#{} = _Db, _Key, <<>>) ->
<<>>;
-
encrypt(#{is_encrypted := false}, _Key, Value) when is_binary(Value) ->
Value;
-
-encrypt(#{is_encrypted := true} = Db, Key, Value)
- when is_binary(Key), is_binary(Value) ->
+encrypt(#{is_encrypted := true} = Db, Key, Value) when
+ is_binary(Key), is_binary(Value)
+->
Review comment:
-3
##########
File path: src/aegis/src/aegis_keywrap.erl
##########
@@ -50,48 +51,66 @@ unwrap(_WrappingKey, A, R, 0) ->
<<A/binary, R/binary>>;
unwrap(WrappingKey, <<A:64>>, R, T) ->
RestSize = bit_size(R) - 64,
- <<Rest:RestSize, R2: 64>> = R,
+ <<Rest:RestSize, R2:64>> = R,
Review comment:
+1
##########
File path: src/aegis/src/aegis_keywrap.erl
##########
@@ -50,48 +51,66 @@ unwrap(_WrappingKey, A, R, 0) ->
<<A/binary, R/binary>>;
unwrap(WrappingKey, <<A:64>>, R, T) ->
RestSize = bit_size(R) - 64,
- <<Rest:RestSize, R2: 64>> = R,
+ <<Rest:RestSize, R2:64>> = R,
<<MSB_B:64, LSB_B:64>> = ?aes_ecb_decrypt(WrappingKey, <<(A bxor T):64,
R2:64>>),
unwrap(WrappingKey, <<MSB_B:64>>, <<LSB_B:64, Rest:RestSize>>, T - 1).
-
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
wrap_test_() ->
[
- %% 128 KEK / 128 DATA
- test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F:128>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#1FA68B0A8112B447AEF34BD8FB5A7B829D3E862371D2CFE5:192>>),
- %% 192 KEK / 128 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#96778B25AE6CA435F92B5B97C050AED2468AB8A17AD84E5D:192>>),
- %% 256 KEK / 128 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#64E8C3F9CE0F5BA263E9777905818A2A93C8191E7D6E8AE7:192>>),
- %% 192 KEK / 192 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
-
<<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
-
<<16#031D33264E15D33268F24EC260743EDCE1C6C7DDEE725A936BA814915C6762D2:256>>),
- %% 256 KEK / 192 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
-
<<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
-
<<16#A8F9BC1612C68B3FF6E6F4FBE30E71E4769C8B80A32CB8958CD5D17D6B254DA1:256>>),
- %% 256 KEK / 256 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
-
<<16#00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F:256>>,
-
<<16#28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD21:320>>)].
+ %% 128 KEK / 128 DATA
+ test_wrap_unwrap(
Review comment:
+2
##########
File path: src/aegis/src/aegis_keywrap.erl
##########
@@ -50,48 +51,66 @@ unwrap(_WrappingKey, A, R, 0) ->
<<A/binary, R/binary>>;
unwrap(WrappingKey, <<A:64>>, R, T) ->
RestSize = bit_size(R) - 64,
- <<Rest:RestSize, R2: 64>> = R,
+ <<Rest:RestSize, R2:64>> = R,
<<MSB_B:64, LSB_B:64>> = ?aes_ecb_decrypt(WrappingKey, <<(A bxor T):64,
R2:64>>),
unwrap(WrappingKey, <<MSB_B:64>>, <<LSB_B:64, Rest:RestSize>>, T - 1).
-
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
wrap_test_() ->
[
- %% 128 KEK / 128 DATA
- test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F:128>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#1FA68B0A8112B447AEF34BD8FB5A7B829D3E862371D2CFE5:192>>),
- %% 192 KEK / 128 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#96778B25AE6CA435F92B5B97C050AED2468AB8A17AD84E5D:192>>),
- %% 256 KEK / 128 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
- <<16#00112233445566778899AABBCCDDEEFF:128>>,
-
<<16#64E8C3F9CE0F5BA263E9777905818A2A93C8191E7D6E8AE7:192>>),
- %% 192 KEK / 192 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
-
<<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
-
<<16#031D33264E15D33268F24EC260743EDCE1C6C7DDEE725A936BA814915C6762D2:256>>),
- %% 256 KEK / 192 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
-
<<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
-
<<16#A8F9BC1612C68B3FF6E6F4FBE30E71E4769C8B80A32CB8958CD5D17D6B254DA1:256>>),
- %% 256 KEK / 256 DATA
-
test_wrap_unwrap(<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
-
<<16#00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F:256>>,
-
<<16#28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD21:320>>)].
+ %% 128 KEK / 128 DATA
+ test_wrap_unwrap(
+ <<16#000102030405060708090A0B0C0D0E0F:128>>,
+ <<16#00112233445566778899AABBCCDDEEFF:128>>,
+ <<16#1FA68B0A8112B447AEF34BD8FB5A7B829D3E862371D2CFE5:192>>
+ ),
+ %% 192 KEK / 128 DATA
+ test_wrap_unwrap(
+ <<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
+ <<16#00112233445566778899AABBCCDDEEFF:128>>,
+ <<16#96778B25AE6CA435F92B5B97C050AED2468AB8A17AD84E5D:192>>
+ ),
+ %% 256 KEK / 128 DATA
+ test_wrap_unwrap(
+
<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
+ <<16#00112233445566778899AABBCCDDEEFF:128>>,
+ <<16#64E8C3F9CE0F5BA263E9777905818A2A93C8191E7D6E8AE7:192>>
+ ),
+ %% 192 KEK / 192 DATA
+ test_wrap_unwrap(
+ <<16#000102030405060708090A0B0C0D0E0F1011121314151617:192>>,
+ <<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
+
<<16#031D33264E15D33268F24EC260743EDCE1C6C7DDEE725A936BA814915C6762D2:256>>
+ ),
+ %% 256 KEK / 192 DATA
+ test_wrap_unwrap(
+
<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
+ <<16#00112233445566778899AABBCCDDEEFF0001020304050607:192>>,
+
<<16#A8F9BC1612C68B3FF6E6F4FBE30E71E4769C8B80A32CB8958CD5D17D6B254DA1:256>>
+ ),
+ %% 256 KEK / 256 DATA
+ test_wrap_unwrap(
+
<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
+
<<16#00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F:256>>,
+ <<
+
16#28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD21:320
+ >>
+ )
+ ].
test_wrap_unwrap(WrappingKey, KeyToWrap, ExpectedWrappedKey) ->
- [?_assertEqual(ExpectedWrappedKey, key_wrap(WrappingKey, KeyToWrap)),
- ?_assertEqual(KeyToWrap, key_unwrap(WrappingKey, key_wrap(WrappingKey,
KeyToWrap)))].
+ [
+ ?_assertEqual(ExpectedWrappedKey, key_wrap(WrappingKey, KeyToWrap)),
+ ?_assertEqual(KeyToWrap, key_unwrap(WrappingKey, key_wrap(WrappingKey,
KeyToWrap)))
+ ].
fail_test() ->
KEK =
<<16#000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F:256>>,
- CipherText =
<<16#28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD20:320>>,
+ CipherText = <<
Review comment:
0
##########
File path: src/aegis/src/aegis_server.erl
##########
@@ -94,7 +85,7 @@ encrypt(#{} = Db, Key, Value) when is_binary(Key),
is_binary(Value) ->
case gen_server:call(?MODULE, {encrypt, Db, Key, Value}) of
CipherText when is_binary(CipherText) ->
CipherText;
- {error, {_Tag, {_C_FileName,_LineNumber}, _Desc} = Reason} ->
+ {error, {_Tag, {_C_FileName, _LineNumber}, _Desc} = Reason} ->
Review comment:
+2
##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -54,74 +90,91 @@
code,
headers,
chunks,
- resp=nil,
- buffer_response=false
+ resp = nil,
+ buffer_response = false
}).
start_link() ->
start_link(http).
start_link(http) ->
Port = config:get("chttpd", "port", "5984"),
start_link(?MODULE, [{port, Port}]);
-
start_link(https) ->
Port = config:get("ssl", "port", "6984"),
{ok, Ciphers} = couch_util:parse_term(config:get("ssl", "ciphers",
"undefined")),
{ok, Versions} = couch_util:parse_term(config:get("ssl", "tls_versions",
"undefined")),
- {ok, SecureRenegotiate} = couch_util:parse_term(config:get("ssl",
"secure_renegotiate", "undefined")),
+ {ok, SecureRenegotiate} = couch_util:parse_term(
+ config:get("ssl", "secure_renegotiate", "undefined")
+ ),
ServerOpts0 =
- [{cacertfile, config:get("ssl", "cacert_file", undefined)},
- {keyfile, config:get("ssl", "key_file", undefined)},
- {certfile, config:get("ssl", "cert_file", undefined)},
- {password, config:get("ssl", "password", undefined)},
- {secure_renegotiate, SecureRenegotiate},
- {versions, Versions},
- {ciphers, Ciphers}],
-
- case (couch_util:get_value(keyfile, ServerOpts0) == undefined orelse
- couch_util:get_value(certfile, ServerOpts0) == undefined) of
+ [
+ {cacertfile, config:get("ssl", "cacert_file", undefined)},
+ {keyfile, config:get("ssl", "key_file", undefined)},
+ {certfile, config:get("ssl", "cert_file", undefined)},
+ {password, config:get("ssl", "password", undefined)},
+ {secure_renegotiate, SecureRenegotiate},
+ {versions, Versions},
+ {ciphers, Ciphers}
+ ],
+
+ case
+ (couch_util:get_value(keyfile, ServerOpts0) == undefined orelse
Review comment:
I would prefer the following here
```
(couch_util:get_value(keyfile, ServerOpts0) == undefined
orelse couch_util:get_value(certfile, ServerOpts0) == undefined)
```
OR
```
(
couch_util:get_value(keyfile, ServerOpts0) == undefined
orelse
couch_util:get_value(certfile, ServerOpts0) == undefined
)
```
-1
##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -132,19 +185,21 @@ start_link(Name, Options) ->
set_auth_handlers(),
- Options1 = Options ++ [
- {loop, fun ?MODULE:handle_request/1},
- {name, Name},
- {ip, IP}
- ],
+ Options1 =
Review comment:
0
##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -132,19 +185,21 @@ start_link(Name, Options) ->
set_auth_handlers(),
- Options1 = Options ++ [
- {loop, fun ?MODULE:handle_request/1},
- {name, Name},
- {ip, IP}
- ],
+ Options1 =
+ Options ++
+ [
+ {loop, fun ?MODULE:handle_request/1},
+ {name, Name},
+ {ip, IP}
+ ],
ServerOpts = get_server_options("chttpd"),
Options2 = merge_server_options(Options1, ServerOpts),
case mochiweb_http:start(Options2) of
- {ok, Pid} ->
- {ok, Pid};
- {error, Reason} ->
- io:format("Failure to start Mochiweb: ~s~n", [Reason]),
- {error, Reason}
+ {ok, Pid} ->
Review comment:
+2 to always indent clauses in case even though CouchDB historically
doesn't do it.
##########
File path: src/chttpd/src/chttpd.erl
##########
@@ -437,32 +525,42 @@ maybe_log(#httpd{} = HttpReq, #httpd_resp{should_log =
true} = HttpResp) ->
% - client port
% - timers: connection, request, time to first byte, ...
% - response size
- %
- ?LOG_NOTICE(#{
- method => Method,
- path => RawUri,
- code => Code,
- user => User,
- % req_size => MochiReq:get(body_length),
- src => #{ip4 => Peer},
- duration => RequestTime
- }, #{domain => [chttpd_access_log]}),
- couch_log:notice("~s ~s ~s ~s ~s ~B ~p ~B", [Host, Peer, User,
- Method, RawUri, Code, Status, RequestTime]);
+ %
+ ?LOG_NOTICE(
+ #{
Review comment:
+2 to symmetric brackets.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
