Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor auth code to enable for different auth mechanisms #7986

Closed
locker opened this issue Nov 30, 2022 · 0 comments · Fixed by #8028
Closed

Refactor auth code to enable for different auth mechanisms #7986

locker opened this issue Nov 30, 2022 · 0 comments · Fixed by #8028
Assignees
@locker
Labels
2.11 Target is 2.11 and all newer release/master branches refactoring Code refactoring

Comments

@locker
Copy link
Member

locker commented Nov 30, 2022

Authentication in Tarantool was designed with extensibility in mind. Although currently the only auth method supported is 'chap-sha1', both the database schema and IPROTO allow to specify a different auth method. However, the auth implementation code (password hasing, scramble check, etc) lacks any abstraction that would actually allow to implement a new auth method. We need to refactor it to make implementation of a new auth method easy.
@locker locker added the refactoring Code refactoring label Nov 30, 2022
@locker locker self-assigned this Nov 30, 2022
@locker locker mentioned this issue Nov 30, 2022
Closed
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: convert authentication.cc to C
4c56eb2 
C++ features aren't really needed there. Let's drop exceptions and
convert to C to simplify further development.

While we are at it, shorten the License text, replace ifdef guards
with pragma, and clean up the include list.

Needed for tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: add user_def_new and user_def_delete helpers
df7af0c 
We will need to store some extra data in the user_def struct to support
different authentication mechanisms. Let's introduce convenient helpers
for allocating and freeing this struct so that we don't have to patch
all the places in the code where it's allocated or freed when we extend
the struct.

While we are at it, switch to grp_alloc, shorted the license text, and
replace include guards with pragma.

Needed for tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: make auth subsystem pluggable
0a0f920 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
@locker locker mentioned this issue Dec 8, 2022
Merged
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: make auth subsystem pluggable
1c9e1a3 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: make auth subsystem pluggable
05e5600 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 8, 2022
@locker
box: make auth subsystem pluggable
27e06e7 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 9, 2022
@locker
box: convert authentication.cc to C
f5278e7 
C++ features aren't really needed there. Let's drop exceptions and
convert to C to simplify further development.

While we are at it, shorten the License text, replace ifdef guards
with pragma, and clean up the include list.

Needed for tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 9, 2022
@locker
box: add user_def_new and user_def_delete helpers
49caa65 
We will need to store some extra data in the user_def struct to support
different authentication mechanisms. Let's introduce convenient helpers
for allocating and freeing this struct so that we don't have to patch
all the places in the code where it's allocated or freed when we extend
the struct.

While we are at it, switch to grp_alloc, shorted the license text, and
replace include guards with pragma.

Needed for tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit to locker/tarantool that referenced this issue Dec 9, 2022
@locker
box: make auth subsystem pluggable
9776c50 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
@locker locker added the 2.11 Target is 2.11 and all newer release/master branches label Dec 9, 2022
locker added a commit to locker/tarantool that referenced this issue Dec 9, 2022
@locker
box: make auth subsystem pluggable
39ebedc 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of tarantool#7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes tarantool#7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit that referenced this issue Dec 9, 2022
@locker
box: convert authentication.cc to C
f1879a1 
C++ features aren't really needed there. Let's drop exceptions and
convert to C to simplify further development.

While we are at it, shorten the License text, replace ifdef guards
with pragma, and clean up the include list.

Needed for #7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit that referenced this issue Dec 9, 2022
@locker
box: add user_def_new and user_def_delete helpers
e3f170d 
We will need to store some extra data in the user_def struct to support
different authentication mechanisms. Let's introduce convenient helpers
for allocating and freeing this struct so that we don't have to patch
all the places in the code where it's allocated or freed when we extend
the struct.

While we are at it, switch to grp_alloc, shorted the license text, and
replace include guards with pragma.

Needed for #7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
locker added a commit that referenced this issue Dec 9, 2022
@locker
box: make auth subsystem pluggable
b5754d3 
This commit introduces an abstraction for the authentication code so
that one can easily add new methods. To add a new method, one just needs
to define a set of authentication callbacks in a struct auth_method and
register it with auth_method_register.

The IPROTO_AUTH and _user.auth formats were initially designed with
extensibility in mind: both take the authentication method name
(currently, only 'chap-sha1' is supported) so no changes to the schema
are required.

Note that although 'chap-sha1' is now implemented in its own file
src/box/auth_chap_sha1.c, we don't merge src/scramble.c into it.
This will be done later, in the scope of #7987.

Since we call authentication plug-ins "methods" (not "mechanisms"),
let's rename BOX_USER_FIELD_AUTH_MECH_LIST to BOX_USER_FIELD_AUTH while
we are at it. Anyway, the corresponding field of the _user system space
is called 'auth' (not 'auth_mech_list').

Closes #7986

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
@locker locker mentioned this issue Dec 9, 2022
Merged
locker added a commit to locker/tarantool that referenced this issue Dec 13, 2022
@locker
box: ensure requested auth method matches user's
4df2f98 
After checking that the received authentication method and type are
compatible with auth_request_check(), authenticate() tries to
authenticate it using the user's authentication method with
authenticate_request(). The problem is the user may use a different
authenticate method from the one received in the request while
authenticate_request() expects the request to be valid. As a result,
it may crash in this case. Fix this by ensuring that the user's
authentication method matches the one received in the request.

Follow-up commit b5754d3 ("box: make auth subsystem pluggable")
Follow-up tarantool#7986

NO_DOC=bug fix
NO_CHANGELOG=unreleased
NO_TEST=will be added to EE, because CE supports just one auth method
@locker locker mentioned this issue Dec 13, 2022
Merged
locker added a commit that referenced this issue Dec 13, 2022
@locker
box: ensure requested auth method matches user's
9dcc824 
After checking that the received authentication method and type are
compatible with auth_request_check(), authenticate() tries to
authenticate it using the user's authentication method with
authenticate_request(). The problem is the user may use a different
authenticate method from the one received in the request while
authenticate_request() expects the request to be valid. As a result,
it may crash in this case. Fix this by ensuring that the user's
authentication method matches the one received in the request.

Follow-up commit b5754d3 ("box: make auth subsystem pluggable")
Follow-up #7986

NO_DOC=bug fix
NO_CHANGELOG=unreleased
NO_TEST=will be added to EE, because CE supports just one auth method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@locker locker

Labels
2.11 Target is 2.11 and all newer release/master branches refactoring Code refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor auth code to enable for different auth mechanisms locker/tarantool
1 participant
@locker