Redis AUTH support

[reimannf]

This superseeds https://github.com/Netflix/dynomite/pull/576 as I cannot push to the original feature branch. @axelfauvel if you are not ok with this, can you please merge the current state of dynomite in your branch, so I could pick from there? @smukil I think I have all your remarks covered in the origanl PR. Hopefully we can take it from here.

This commit is based on https://github.com/orange-cloudfoundry/dynomite/commit/7aa41a49773270d069b79e8a38e45c25ee1e0e09 from @axelfauvel in https://github.com/Netflix/dynomite/pull/576 and tries to close https://github.com/Netflix/dynomite/issues/46.

Unfortunatelly the initial commit was already so old and the dynomite code base already evolved, that it was easier to not jump directly on this. Especically as there were some refactorings requested.

Redis Datastore Authentification

If Dynomite is configured to require a password via config option requirepass the following behaviour will be applied:

1. On Dynomite startup, the server authenticates with the backend itself by calling the datastore agnostic function g_datastore_auth.
2. The corresponding Redis response will be handeled in g_is_authenticated. Dynomite will exit if authentification to the datatstore was not successful.
3. Each newly created client connection will require authentification.
4. Clients can authentificate itself by issue the AUTH command against dynomite.
5. Dynomite will check the password and simulate an AUTH response.
6. If AUTH was successful, the auth_required flag on the connection is reset and the client can process further commands through this connection.

[axelfauvel]

Hi @shailesh33 @ipapapa

Do you think you can merge this PR ? Many thanks :)

[jyriok]

any news about this pr ? :)

[yedongxin]

hello,any news about this merging ?

[ArthurHlt]

@smukil could you merge this ? it actually looks like author take all your remarks from original PR and do the changes and you needed only those changes before merging on original PR.

Not supporting auth is a real show blocker for many company, thanks for the time took on merging it

[mcouillard]

I've proven this PR against my fork of dev and the basics appear fine. It correctly requires the auth password & fails when given the wrong one, or none.

I'd love to see this PR get updated & re-reviewed for inclusion.

[WenningQiu]

@smukil could you merge this ? it actually looks like author take all your remarks from original PR and do the changes and you needed only those changes before merging on original PR.

Not supporting auth is a real show blocker for many company, thanks for the time took on merging it

+1

[WenningQiu]

@reimannf

I tried applied your PR (https://github.com/Netflix/dynomite/commit/682daa32a80396f9522c390d9ffff277df3bd953) to v0.8, the code builds but has runtime error on AUTH command:

qiuwen01-pa@accdevxacpx0001> redis-cli -p 5010
127.0.0.1:5010> auth WenningQiu password
Error: Server closed the connection
127.0.0.1:5010>

[2022-11-01 09:51:17.399] server_connected:297 <CONN_SERVER 0x13c3a30 5 to '127.0.0.1:5000:1'> connected
[2022-11-01 09:51:36.102] proxy_accept:203 <CONN_PROXY 0x13cdfa0 7 listening on '0.0.0.0:5010'> accepted <CONN_CLIENT 0x13d0a20 9 from '127.0.0.1:52448'>
[2022-11-01 09:51:36.103] redis_parse_req:1590 parsed unsupported command 'COMMAND'
[2022-11-01 09:51:36.103] redis_parse_req:2394 parsed bad req 3 res 1 type 0 state 5
00000000  2a 31 0d 0a 24 37 0d 0a  43 4f 4d 4d 41 4e 44 0d   |*1..$7..COMMAND.|
00000010  0a                                                 |.|
[2022-11-01 09:51:36.103] core_close:418 close <CONN_CLIENT 0x13d0a20 9 from '127.0.0.1:52448'> on event 00FF eof 0 done 0 rb 17 sb 0: Invalid argument
[2022-11-01 09:51:36.103] client_unref_internal_try_put:97 <CONN_CLIENT 0x13d0a20 -1 from '127.0.0.1:52448'> unref owner <POOL 0x13c2bb0 'dyn_o_mite'>
[2022-11-01 09:51:47.410] core_print_peer_status:62 0)0x13ce5d0 omaha rack5000 0.0.0.0:5100 NORMAL
[2022-11-01 09:52:23.096] proxy_accept:203 <CONN_PROXY 0x13cdfa0 7 listening on '0.0.0.0:5010'> accepted <CONN_CLIENT 0x13d0a20 9 from '127.0.0.1:52460'>
[2022-11-01 09:52:23.096] redis_parse_req:2394 parsed bad req 4 res 1 type 150 state 10
00000000  2a 33 0d 0a 24 34 0d 0a  61 75 74 68 0d 0a 24 31   |*3..$4..auth..$1|
00000010  30 0d 0a 57 65 6e 6e 69  6e 67 51 69 75 0d 0a 24   |0..WenningQiu..$|
00000020  38 0d 0a 70 61 73 73 77  6f 72 64 0d 0a            |8..password..|
[2022-11-01 09:52:23.096] core_close:418 close <CONN_CLIENT 0x13d0a20 9 from '127.0.0.1:52460'> on event FFFF eof 0 done 0 rb 45 sb 0: Invalid argument
[2022-11-01 09:52:23.097] client_unref_internal_try_put:97 <CONN_CLIENT 0x13d0a20 -1 from '127.0.0.1:52460'> unref owner <POOL 0x13c2bb0 'dyn_o_mite'>

So my questions for @reimannf:

1. What branch was your PR created for? 
2. If it was for a codebase before v0.8, will you be able to bring it up-to-date against v0.8? 

[reimannf]

You can see the original code that PR was prepared here: https://github.com/reimannf/dynomite/tree/redis-auth

Nevertheless, it seems that this PR is not wanted by the repo owners and I will not dump more time into it, feel free to pick it up.

[WenningQiu]

You can see the original code that PR was prepared here: https://github.com/reimannf/dynomite/tree/redis-auth

Nevertheless, it seems that this PR is not wanted by the repo owners and I will not dump more time into it, feel free to pick it up.

@reimannf It turns out your change works with "AUTH <password>" command, even when it is applied to v0.8. The failure that I ran into was on "AUTH <username> <password>" command. I will see if I can adjust the patch to make it work with AUTH <username> <password> command to support ACLs.