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.

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

Move pam_password configuration to R_GRID_CONFIGURATION #7274

Closed
3 tasks done
alanking opened this issue Aug 22, 2023 · 2 comments
Closed
3 tasks done

Move pam_password configuration to R_GRID_CONFIGURATION #7274

alanking opened this issue Aug 22, 2023 · 2 comments
Assignees
Milestone

Comments

@alanking
Copy link
Contributor

alanking commented Aug 22, 2023

  • main
  • 4-3-stable
  • irods_docs

Feature

The following pam_password configurations are supported in server_config.json:

  • no_extend
  • password_max_time
  • password_min_time

These configurations should be moved out of server_config to new rows in R_GRID_CONFIGURATION. This allows for greater support of multiple providers (or HA setups) and will allow authenticating from other servers in a zone in the future.

Additionally, no_extend should be renamed to extend_password_lifetime to remove the negative language and prevent confusion. The setting should only affect the lifetime of the randomly generated password for PAM authentication. It's not meant to extend the lifetime by "less", as it does now:

if ( irods_pam_auth_no_extend ) {
snprintf( irods_pam_password_default_time,
sizeof( irods_pam_password_default_time ),
"%s", "28800" );
}

The configuration values should be pulled from server_config.json if available. If not, default values should be used as follows:

  • extend_password_lifetime: 1 - This will be interpreted as "true", which is logically equivalent to no_extend defaulting to "false".
  • password_max_time: 1209600 - Current default. Measured in seconds (14 days).
  • password_min_time: 121 - Current default. Measured in seconds (2 minutes and 1 second)

This issue is a follow-up to #7098.

@alanking alanking added this to the 4.3.1 milestone Aug 22, 2023
@alanking alanking self-assigned this Aug 22, 2023
alanking added a commit to alanking/irods that referenced this issue Sep 20, 2023
This change adds a new namespace to the R_GRID_CONFIGURATION table in
the database for pam_password. The configuration values which were set
in the server_config plugin_configuration/authentication/pam_password
stanza are now to be set here. These can be configured with the iadmin
set_pam_password_config subcommand and queried using iadmin
get_pam_password_config.
alanking added a commit to alanking/irods that referenced this issue Sep 20, 2023
Authentication configurations in the database now use grid configurations
instead of server_config. These are stored in the database and so incur
an additional query whenever they are needed. If the values are invalid,
an error is returned, which renders all pam_password and native authentication
which uses TTL to be unusable. The values are updated on each invocation
of the database operations to update the temporary passwords.
alanking added a commit that referenced this issue Sep 20, 2023
This change adds a new namespace to the R_GRID_CONFIGURATION table in
the database for pam_password. The configuration values which were set
in the server_config plugin_configuration/authentication/pam_password
stanza are now to be set here. These can be configured with the iadmin
set_pam_password_config subcommand and queried using iadmin
get_pam_password_config.
alanking added a commit that referenced this issue Sep 20, 2023
Authentication configurations in the database now use grid configurations
instead of server_config. These are stored in the database and so incur
an additional query whenever they are needed. If the values are invalid,
an error is returned, which renders all pam_password and native authentication
which uses TTL to be unusable. The values are updated on each invocation
of the database operations to update the temporary passwords.
alanking added a commit to alanking/irods that referenced this issue Sep 20, 2023
This change adds a new namespace to the R_GRID_CONFIGURATION table in
the database for pam_password. The configuration values which were set
in the server_config plugin_configuration/authentication/pam_password
stanza are now to be set here. These can be configured with the iadmin
set_pam_password_config subcommand and queried using iadmin
get_pam_password_config.
alanking added a commit to alanking/irods that referenced this issue Sep 20, 2023
Authentication configurations in the database now use grid configurations
instead of server_config. These are stored in the database and so incur
an additional query whenever they are needed. If the values are invalid,
an error is returned, which renders all pam_password and native authentication
which uses TTL to be unusable. The values are updated on each invocation
of the database operations to update the temporary passwords.
alanking added a commit that referenced this issue Sep 20, 2023
This change adds a new namespace to the R_GRID_CONFIGURATION table in
the database for pam_password. The configuration values which were set
in the server_config plugin_configuration/authentication/pam_password
stanza are now to be set here. These can be configured with the iadmin
set_pam_password_config subcommand and queried using iadmin
get_pam_password_config.
alanking added a commit that referenced this issue Sep 20, 2023
Authentication configurations in the database now use grid configurations
instead of server_config. These are stored in the database and so incur
an additional query whenever they are needed. If the values are invalid,
an error is returned, which renders all pam_password and native authentication
which uses TTL to be unusable. The values are updated on each invocation
of the database operations to update the temporary passwords.
@alanking
Copy link
Contributor Author

alanking commented Sep 21, 2023

Adding tests for various invalid configurations. Automated testing is not feasible at the moment because all the settings have to do with password lifetimes, which are currently only measured in hours.

alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
Fix erroneous attempted deletion of password from the catalog when not a
temporary password. Also, invalid configurations do not return errors now
because the system becomes unusable when we do this. Warnings are logged
and we fall back to the default values whenever an invalid configuration
is encountered.
alanking added a commit that referenced this issue Sep 28, 2023
Fix erroneous attempted deletion of password from the catalog when not a
temporary password. Also, invalid configurations do not return errors now
because the system becomes unusable when we do this. Warnings are logged
and we fall back to the default values whenever an invalid configuration
is encountered.
alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
alanking added a commit to alanking/irods that referenced this issue Sep 28, 2023
Fix erroneous attempted deletion of password from the catalog when not a
temporary password. Also, invalid configurations do not return errors now
because the system becomes unusable when we do this. Warnings are logged
and we fall back to the default values whenever an invalid configuration
is encountered.
alanking added a commit that referenced this issue Sep 28, 2023
Fix erroneous attempted deletion of password from the catalog when not a
temporary password. Also, invalid configurations do not return errors now
because the system becomes unusable when we do this. Warnings are logged
and we fall back to the default values whenever an invalid configuration
is encountered.
@alanking
Copy link
Contributor Author

Discovered a major issue with the current approach... native and pam_password authentication cannot be distinguished in the auth check database operation because pam_password literally performs native authentication with the randomly generated password.

We had some discussion and agreed that the best/least destructive path forward would be to keep the options combined as they have been historically, but make the auth configuration more generic. Instead of namespacing authentication::pam_password and authentication::native configurations in R_GRID_CONFIGURATION, we will introduce a single namespace authentication which will configure for both plugins (and any other authentication plugins which so choose to use the setting).

In the future we may introduce plugin-specific configurations which could be used to override relevant settings in the generic namespace.

alanking added a commit to alanking/irods_docs that referenced this issue Sep 29, 2023
alanking added a commit to irods/irods_docs that referenced this issue Sep 29, 2023
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
These tests are based entirely off of the pam_password equivalents
because they should behave in the same way.

Tests for password_extend_lifetime have been written but are skipped
because that configuration is not supported for native authentication.

Tests for password expiration are also being skipped because the
minimum TTL we can specify is 1 hour which is not feasible for
automated testing.
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
TTL needs to be converted to seconds before comparing against the
min/max password time configurations.

clientLogin needs to return a better error message when a failure
occurs in rcGetLimitedPassword.
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
The plugin-specific auth configurations (authentication::pam_password
and authentication::native) have been replaced by a general config
for all auth schemes. The string in R_GRID_CONFIGURATION is now just
"authentication".
alanking added a commit that referenced this issue Oct 2, 2023
These tests are based entirely off of the pam_password equivalents
because they should behave in the same way.

Tests for password_extend_lifetime have been written but are skipped
because that configuration is not supported for native authentication.

Tests for password expiration are also being skipped because the
minimum TTL we can specify is 1 hour which is not feasible for
automated testing.
alanking added a commit that referenced this issue Oct 2, 2023
TTL needs to be converted to seconds before comparing against the
min/max password time configurations.

clientLogin needs to return a better error message when a failure
occurs in rcGetLimitedPassword.
alanking added a commit that referenced this issue Oct 2, 2023
The plugin-specific auth configurations (authentication::pam_password
and authentication::native) have been replaced by a general config
for all auth schemes. The string in R_GRID_CONFIGURATION is now just
"authentication".
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
These tests are based entirely off of the pam_password equivalents
because they should behave in the same way.

Tests for password_extend_lifetime have been written but are skipped
because that configuration is not supported for native authentication.

Tests for password expiration are also being skipped because the
minimum TTL we can specify is 1 hour which is not feasible for
automated testing.
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
TTL needs to be converted to seconds before comparing against the
min/max password time configurations.

clientLogin needs to return a better error message when a failure
occurs in rcGetLimitedPassword.
alanking added a commit to alanking/irods that referenced this issue Oct 2, 2023
The plugin-specific auth configurations (authentication::pam_password
and authentication::native) have been replaced by a general config
for all auth schemes. The string in R_GRID_CONFIGURATION is now just
"authentication".
alanking added a commit that referenced this issue Oct 2, 2023
These tests are based entirely off of the pam_password equivalents
because they should behave in the same way.

Tests for password_extend_lifetime have been written but are skipped
because that configuration is not supported for native authentication.

Tests for password expiration are also being skipped because the
minimum TTL we can specify is 1 hour which is not feasible for
automated testing.
alanking added a commit that referenced this issue Oct 2, 2023
TTL needs to be converted to seconds before comparing against the
min/max password time configurations.

clientLogin needs to return a better error message when a failure
occurs in rcGetLimitedPassword.
alanking added a commit that referenced this issue Oct 2, 2023
The plugin-specific auth configurations (authentication::pam_password
and authentication::native) have been replaced by a general config
for all auth schemes. The string in R_GRID_CONFIGURATION is now just
"authentication".
@alanking alanking closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant