diff options
author | Nick Vatamaniuc <vatamane@gmail.com> | 2022-12-03 03:01:25 -0500 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2022-12-04 12:38:25 -0500 |
commit | eff331c9f5e734d318f9491c469e9d7be08e6999 (patch) | |
tree | 119dc8305932d7583c024c48ebc101ba1455292b | |
parent | 958d0531087f198e7654a396ce192d1dc5790a36 (diff) | |
download | couchdb-eff331c9f5e734d318f9491c469e9d7be08e6999.tar.gz |
Allow = in config key names
They are allowed only for the "k = v" format and not the "k=v" format. The idea
is to split on " = " first, and if that fails to produce a valid kv pair we
split on "=" as before.
To implement it, simplify the parsing logic and remove the undocumented
multi-line config value feature. The continuation syntax is not documented
anywhere and not used by our default.ini or in documentation.
Fix: https://github.com/apache/couchdb/issues/3319
-rw-r--r-- | rel/overlay/etc/default.ini | 7 | ||||
-rw-r--r-- | src/config/src/config.erl | 244 | ||||
-rw-r--r-- | src/docs/src/api/server/authn.rst | 8 | ||||
-rw-r--r-- | src/docs/src/config/intro.rst | 8 |
4 files changed, 185 insertions, 82 deletions
diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini index 0efc4cb23..ae691bb8d 100644 --- a/rel/overlay/etc/default.ini +++ b/rel/overlay/etc/default.ini @@ -238,6 +238,13 @@ bind_address = 127.0.0.1 ; key with newlines replaced with the escape sequence \n. ; rsa:foo = -----BEGIN PUBLIC KEY-----\nMIIBIjAN...IDAQAB\n-----END PUBLIC KEY-----\n ; ec:bar = -----BEGIN PUBLIC KEY-----\nMHYwEAYHK...AzztRs\n-----END PUBLIC KEY-----\n +; Since version 3.3 it's possible for keys to contain "=" characters when the +; config setting is in the "key = value" format. In other words, there must be a space +; between the key and the equals sign, and another space between the equal sign +; and the value. For example, it should look like this: +; rsa:h213h2h1jg3hj2= = <somevalue> +; and *not* like this: +; rsa:h213h2h1jg3hj2==<somevalue> [couch_peruser] ; If enabled, couch_peruser ensures that a private per-user database diff --git a/src/config/src/config.erl b/src/config/src/config.erl index 58e3a40d9..7c38ff58f 100644 --- a/src/config/src/config.erl +++ b/src/config/src/config.erl @@ -48,6 +48,7 @@ -define(INVALID_SECTION, <<"Invalid configuration section">>). -define(INVALID_KEY, <<"Invalid configuration key">>). -define(INVALID_VALUE, <<"Invalid configuration value">>). +-define(DELETE, delete). -record(config, { notify_funs = [], @@ -414,89 +415,73 @@ is_sensitive(Section, Key) -> end. parse_ini_file(IniFile) -> + IniBin = read_ini_file(IniFile), + ParsedIniValues = parse_ini(IniBin), + {ok, lists:filter(fun delete_keys/1, ParsedIniValues)}. + +parse_ini(IniBin) when is_binary(IniBin) -> + Lines0 = re:split(IniBin, "\r\n|\n|\r|\032", [{return, list}]), + Lines1 = lists:map(fun remove_comments/1, Lines0), + Lines2 = lists:map(fun trim/1, Lines1), + Lines3 = lists:filter(fun(Line) -> Line =/= "" end, Lines2), + {_, IniValues} = lists:foldl(fun parse_fold/2, {"", []}, Lines3), + lists:reverse(IniValues). + +parse_fold("[" ++ Rest, {Section, KVs}) -> + % Check for end ] brackend, if not found or empty section skip the rest + case string:split(Rest, "]") of + ["", _] -> {Section, KVs}; + [NewSection, ""] -> {NewSection, KVs}; + _Else -> {Section, KVs} + end; +parse_fold(_Line, {"" = Section, KVs}) -> + % Empty section don't parse any lines until we're in a section + {Section, KVs}; +parse_fold(Line, {Section, KVs}) -> + case string:split(Line, " = ") of + [K, V] when V =/= "" -> + % Key may have "=" in it. Also, assert we'll never have a + % deletion case here since we're working with a stripped line + {Section, [{{Section, trim(K)}, trim(V)} | KVs]}; + [_] -> + % Failed to split on " = ", so try to split on "=". + % If the line starts with "=" or it's not a KV pair, ignore it. + % An empty value emit the `delete` atom as a marker. + case string:split(Line, "=") of + ["", _] -> {Section, KVs}; + [K, ""] -> {Section, [{{Section, trim(K)}, ?DELETE} | KVs]}; + [K, V] -> {Section, [{{Section, trim(K)}, trim(V)} | KVs]}; + [_] -> {Section, KVs} + end + end. + +read_ini_file(IniFile) -> IniFilename = config_util:abs_pathname(IniFile), - IniBin = - case file:read_file(IniFilename) of - {ok, IniBin0} -> - IniBin0; - {error, enoent} -> - Fmt = "Couldn't find server configuration file ~s.", - Msg = list_to_binary(io_lib:format(Fmt, [IniFilename])), - couch_log:error("~s~n", [Msg]), - throw({startup_error, Msg}) - end, + case file:read_file(IniFilename) of + {ok, IniBin0} -> + IniBin0; + {error, enoent} -> + Fmt = "Couldn't find server configuration file ~s.", + Msg = list_to_binary(io_lib:format(Fmt, [IniFilename])), + couch_log:error("~s~n", [Msg]), + throw({startup_error, Msg}) + end. - Lines = re:split(IniBin, "\r\n|\n|\r|\032", [{return, list}]), - {_, ParsedIniValues} = - lists:foldl( - fun(Line, {AccSectionName, AccValues}) -> - case string:strip(Line) of - "[" ++ Rest -> - case re:split(Rest, "\\]", [{return, list}]) of - [NewSectionName, ""] -> - {NewSectionName, AccValues}; - % end bracket not at end, ignore this line - _Else -> - {AccSectionName, AccValues} - end; - ";" ++ _Comment -> - {AccSectionName, AccValues}; - Line2 -> - case re:split(Line2, "\s?=\s?", [{return, list}]) of - [Value] -> - MultiLineValuePart = - case re:run(Line, "^ \\S", []) of - {match, _} -> - true; - _ -> - false - end, - case {MultiLineValuePart, AccValues} of - {true, [{{_, ValueName}, PrevValue} | AccValuesRest]} -> - % remove comment - case re:split(Value, " ;|\t;", [{return, list}]) of - [[]] -> - % empty line - {AccSectionName, AccValues}; - [LineValue | _Rest] -> - E = { - {AccSectionName, ValueName}, - PrevValue ++ " " ++ LineValue - }, - {AccSectionName, [E | AccValuesRest]} - end; - _ -> - {AccSectionName, AccValues} - end; - % line begins with "=", ignore - ["" | _LineValues] -> - {AccSectionName, AccValues}; - % yeehaw, got a line! - [ValueName | LineValues] -> - RemainingLine = config_util:implode(LineValues, "="), - % removes comments - case re:split(RemainingLine, " ;|\t;", [{return, list}]) of - [[]] -> - % empty line means delete this key - ets:delete(?MODULE, {AccSectionName, ValueName}), - {AccSectionName, AccValues}; - [LineValue | _Rest] -> - LineValueWithoutLeadTrailWS = string:trim(LineValue), - {AccSectionName, [ - { - {AccSectionName, ValueName}, - LineValueWithoutLeadTrailWS - } - | AccValues - ]} - end - end - end - end, - {"", []}, - Lines - ), - {ok, ParsedIniValues}. +remove_comments(Line) -> + {NoComments, _Comments} = string:take(Line, [$;], true), + NoComments. + +% Specially handle the ?DELETE marker +% +delete_keys({{Section, Key}, ?DELETE}) -> + ets:delete(?MODULE, {Section, Key}), + false; +delete_keys({{_, _}, _}) -> + true. + +trim(String) -> + % May look silly but we're using this quite a bit + string:trim(String). debug_config() -> case ?MODULE:get("log", "level") of @@ -626,4 +611,101 @@ validation_test() -> ), ok. +ini(List) when is_list(List) -> + parse_ini(list_to_binary(List)). + +parse_skip_test() -> + ?assertEqual([], ini("")), + ?assertEqual([], ini("k")), + ?assertEqual([], ini("\n")), + ?assertEqual([], ini("\r\n")), + ?assertEqual([], ini("[s]")), + ?assertEqual([], ini("\n[s]\n")), + ?assertEqual([], ini("[s ]")), + ?assertEqual([], ini("k1\nk2")), + ?assertEqual([], ini("=")), + ?assertEqual([], ini("==")), + ?assertEqual([], ini("===")), + ?assertEqual([], ini("= =")), + ?assertEqual([], ini(" = ")), + ?assertEqual([], ini(";")), + ?assertEqual([], ini(";;")), + ?assertEqual([], ini(" ;")), + ?assertEqual([], ini("k = v")), + ?assertEqual([], ini("[s]\n;k = v")), + ?assertEqual([], ini("[s\nk=v")), + ?assertEqual([], ini("s[\nk=v")), + ?assertEqual([], ini("s]\nk=v")), + ?assertEqual([], ini(";[s]\nk = v")), + ?assertEqual([], ini(" ; [s]\nk = v")), + ?assertEqual([], ini("[s]\n ; k = v")), + ?assertEqual([], ini("[]\nk = v")), + ?assertEqual([], ini(";[s]\n ")). + +parse_basic_test() -> + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk=v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\n\nk=v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\n\r\n\nk=v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\n;\nk=v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk = v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk= v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk =v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk= v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk= v ")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk = v")), + ?assertEqual([{{"s", "k"}, "v"}], ini("[s]\nk = v ; c")). + +parse_extra_equal_sign_test() -> + ?assertEqual([{{"s", "k"}, "=v"}], ini("[s]\nk==v")), + ?assertEqual([{{"s", "k"}, "v="}], ini("[s]\nk=v=")), + ?assertEqual([{{"s", "k"}, "=v"}], ini("[s]\nk ==v")), + ?assertEqual([{{"s", "k"}, "==v"}], ini("[s]\nk===v")), + ?assertEqual([{{"s", "k"}, "v=v"}], ini("[s]\nk=v=v")), + ?assertEqual([{{"s", "k"}, "=v"}], ini("[s]\nk = =v")), + ?assertEqual([{{"s", "k"}, "=v"}], ini("[s]\nk= =v")), + ?assertEqual([{{"s", "k="}, "v"}], ini("[s]\nk= = v")), + ?assertEqual([{{"s", "=k="}, "v"}], ini("[s]\n=k= = v")), + ?assertEqual([{{"s", "==k=="}, "v"}], ini("[s]\n==k== = v")). + +parse_delete_test() -> + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk=")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk=;")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk =")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk = ")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk= ")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk = ")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk = ;")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk =;")), + ?assertEqual([{{"s", "k"}, ?DELETE}], ini("[s]\nk=\n")). + +parse_multiple_kvs_test() -> + ?assertEqual( + [ + {{"s", "k1"}, "v1"}, + {{"s", "k2"}, "v2"} + ], + ini("[s]\nk1=v1\nk2=v2") + ), + ?assertEqual( + [ + {{"s", "k1"}, "v1"}, + {{"s", "k2"}, "v2"} + ], + ini("[s]\nk1 = v1\nk2 = v2\n") + ), + ?assertEqual( + [ + {{"s1", "k"}, "v"}, + {{"s2", "k"}, "v"} + ], + ini("[s1]\nk=v\n;\n\n[s2]\nk=v") + ), + ?assertEqual( + [ + {{"s", "k1"}, "v1"}, + {{"s", "k2"}, "v2"} + ], + ini("[s]\nk1=v1\ngarbage\n= more garbage\nk2=v2") + ). + -endif. diff --git a/src/docs/src/api/server/authn.rst b/src/docs/src/api/server/authn.rst index 4c506b664..b4a4806a2 100644 --- a/src/docs/src/api/server/authn.rst +++ b/src/docs/src/api/server/authn.rst @@ -422,9 +422,15 @@ list as long as the JWT token is valid. ; rsa:foo = -----BEGIN PUBLIC KEY-----\nMIIBIjAN...IDAQAB\n-----END PUBLIC KEY-----\n ; ec:bar = -----BEGIN PUBLIC KEY-----\nMHYwEAYHK...AzztRs\n-----END PUBLIC KEY-----\n -The ``jwt_key`` section lists all the keys that this CouchDB server trusts. You +The ``jwt_keys`` section lists all the keys that this CouchDB server trusts. You should ensure that all nodes of your cluster have the same list. +Since version 3.3 it's possible to use ``=`` in parameter names, but only when +the parameter and value are separated `` = ``, i.e. the equal sign is +surrounded by at least one space on each side. This might be useful in the +``[jwt_keys]`` section where base64 encoded keys may contain the ``=`` +character. + JWT tokens that do not include a ``kid`` claim will be validated against the ``$alg:_default`` key. diff --git a/src/docs/src/config/intro.rst b/src/docs/src/config/intro.rst index 483b1a5c3..10a4b6f79 100644 --- a/src/docs/src/config/intro.rst +++ b/src/docs/src/config/intro.rst @@ -93,6 +93,9 @@ requirement for style and naming. Setting parameters via the configuration file ============================================= +.. versionchanged:: 3.3 added ability to have ``=`` in parameter names +.. versionchanged:: 3.3 removed the undocumented ability to have multi-line values. + The common way to set some parameters is to edit the ``local.ini`` file (location explained above). @@ -117,6 +120,11 @@ The `parameter` specification contains two parts divided by the `equal` sign right one. The leading and following whitespace for ``=`` is an optional to improve configuration readability. +Since version 3.3 it's possible to use ``=`` in parameter names, but only when +the parameter and value are separated `` = ``, i.e. the equal sign is surrounded +by at least one space on each side. This might be useful in the ``[jwt_keys]`` +section, where base64 encoded keys may contain some ``=`` characters. + .. note:: In case when you'd like to remove some parameter from the `default.ini` without modifying that file, you may override in `local.ini`, but without |