Skip to content

Commit

Permalink
[ 7098] Fix random passwords for PAM authentication
Browse files Browse the repository at this point in the history 
The randomly generated passwords used with PAM authentication are not guaranteed to
be of a certain size, including having any contents at all whatsoever. This is due to
the fact that the function used to randomly generate bytes for the string may generate
all characters which fall outside the acceptable range. Furthermore, the acceptable
range of characters does not include all alphanumeric characters, but only the set
[1-8B-Yb-y].

This change uses a new function which properly generates a string of alphanumeric
characters of the specified length. Due to restrictions in password length for native
authentication, the password cannot exceed MAX_PASSWORD_LEN - 8 in length.
  • Loading branch information
@alanking
alanking committed Sep 16, 2023
1 parent 5ca82b7 commit cc29f4d
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 60 deletions.
3 changes: 1 addition & 2 deletions plugins/database/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ foreach(plugin IN LISTS IRODS_DATABASE_PLUGINS)
${plugin_target}
PRIVATE
irods_common
irods_server
irods_plugin_dependencies
irods_server
"${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_filesystem.so"
"${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_system.so"
"${IRODS_EXTERNALS_FULLPATH_BOOST}/lib/libboost_regex.so"
Expand All @@ -77,7 +77,6 @@ foreach(plugin IN LISTS IRODS_DATABASE_PLUGINS)
${plugin_target}
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<BUILD_INTERFACE:${CMAKE_IRODS_SOURCE_DIR}/plugins/auth/include>"
"${IRODS_EXTERNALS_FULLPATH_BOOST}/include"
"${IRODS_EXTERNALS_FULLPATH_FMT}/include"
"${IRODS_EXTERNALS_FULLPATH_NANODBC}/include"
Expand Down
122 changes: 64 additions & 58 deletions plugins/database/src/db_plugin.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,43 +1,43 @@
#include "irods/rodsDef.h"
#include "irods/authenticate.h"
#include "irods/rodsQuota.h"
#include "irods/msParam.h"
#include "irods/rcConnect.h"
#include "irods/icatStructs.hpp"
#include "irods/catalog.hpp"
#include "irods/catalog_utilities.hpp"
#include "irods/checksum.h"
#include "irods/icatHighLevelRoutines.hpp"
#include "irods/private/mid_level.hpp"
#include "irods/private/low_level.hpp"
#include "irods/irods_database_plugin.hpp"
#include "irods/icatStructs.hpp"
#include "irods/irods_auth_constants.hpp"
#include "irods/irods_auth_factory.hpp"
#include "irods/irods_auth_manager.hpp"
#include "irods/irods_auth_object.hpp"
#include "irods/irods_auth_plugin.hpp"
#include "irods/irods_children_parser.hpp"
#include "irods/irods_database_constants.hpp"
#include "irods/irods_postgres_object.hpp"
#include "irods/irods_stacktrace.hpp"
#include "irods/private/irods_catalog_properties.hpp"
#include "irods/private/irods_sql_logger.hpp"
#include "irods/irods_database_plugin.hpp"
#include "irods/irods_hierarchy_parser.hpp"
#include "irods/irods_children_parser.hpp"
#include "irods/irods_auth_object.hpp"
#include "irods/irods_lexical_cast.hpp"
#include "irods/irods_logger.hpp"
#include "irods/irods_pam_auth_object.hpp"
#include "irods/irods_auth_factory.hpp"
#include "irods/irods_auth_plugin.hpp"
#include "irods/irods_auth_manager.hpp"
#include "irods/irods_auth_constants.hpp"
#include "irods/irods_server_properties.hpp"
#include "irods/irods_postgres_object.hpp"
#include "irods/irods_random.hpp"
#include "irods/irods_resource_manager.hpp"
#include "irods/irods_virtual_path.hpp"
#include "irods/irods_rs_comm_query.hpp"
#include "irods/modAccessControl.h"
#include "irods/checksum.h"
#include "irods/irods_server_properties.hpp"
#include "irods/irods_stacktrace.hpp"
#include "irods/irods_virtual_path.hpp"
#include "irods/key_value_proxy.hpp"
#include "irods/user_validation_utilities.hpp"
#include "irods/rods.h"
#include "irods/rcMisc.h"
#include "irods/miscServerFunct.hpp"
#include "irods/modAccessControl.h"
#include "irods/msParam.h"
#include "irods/private/irods_catalog_properties.hpp"
#include "irods/private/irods_sql_logger.hpp"
#include "irods/private/low_level.hpp"
#include "irods/private/mid_level.hpp"
#include "irods/rcConnect.h"
#include "irods/rcMisc.h"
#include "irods/rods.h"
#include "irods/rodsDef.h"
#include "irods/rodsErrorTable.h"
#include "irods/irods_lexical_cast.hpp"
#include "irods/irods_logger.hpp"
#include "irods/catalog.hpp"
#include "irods/catalog_utilities.hpp"
#include "irods/plugins/auth/pam_password.hpp"
#include "irods/rodsQuota.h"
#include "irods/user_validation_utilities.hpp"

#include <fmt/chrono.h>
#include <fmt/format.h>
Expand All @@ -49,6 +49,7 @@
#include <boost/regex.hpp>

#include <algorithm>
#include <cctype>
#include <charconv>
#include <chrono>
#include <cstring>
Expand Down Expand Up @@ -7152,13 +7153,13 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx,
}

// Plus 1 for null terminator.
std::array<char, MAX_PASSWORD_LEN + 1> random_password{};
if (random_password.size() > _password_buffer_size) {
std::array<char, MAX_PASSWORD_LEN + 1> password_in_database_buffer{};
if (password_in_database_buffer.size() > _password_buffer_size) {
return ERROR(
SYS_INVALID_INPUT_PARAM,
fmt::format("{}: Buffer not large enough to hold password. Requires [{}] bytes, received [{}] bytes.",
__func__,
random_password.size(),
password_in_database_buffer.size(),
_password_buffer_size));
}

Expand Down Expand Up @@ -7256,7 +7257,7 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx,
log_sql::debug("chlUpdateIrodsPamPassword SQL 3");
}

cVal[0] = random_password.data();
cVal[0] = password_in_database_buffer.data();
iVal[0] = MAX_PASSWORD_LEN;
cVal[1] = passwordModifyTime;
iVal[1] = sizeof( passwordModifyTime );
Expand All @@ -7283,7 +7284,7 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx,
cllBindVars[cllBindVarCount++] = expTime;
cllBindVars[cllBindVarCount++] = selUserId;
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index)
cllBindVars[cllBindVarCount++] = random_password.data();
cllBindVars[cllBindVarCount++] = password_in_database_buffer.data();
status = cmlExecuteNoAnswerSql( "update R_USER_PASSWORD set modify_ts=?, pass_expiry_ts=? where user_id = ? and rcat_password = ?",
&icss );
if ( status ) {
Expand All @@ -7297,37 +7298,41 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx,
}
}

// random_password is the randomly generated password (see while loop below) in a scrambled form.
icatDescramble(random_password.data());
// password_in_database_buffer holds the randomly generated password in a scrambled form. It needs to be
// descrambled before returning to the caller.
icatDescramble(password_in_database_buffer.data());

// The descrambled password at time of generation is 50 characters or less (see below).
std::strncpy(*_password_buffer, random_password.data(), _password_buffer_size);
std::strncpy(*_password_buffer, password_in_database_buffer.data(), _password_buffer_size);

return SUCCESS();
}

// +1 for null terminator
std::array<char, MAX_PASSWORD_LEN + 1> scrambled_random_password{};
// See db_mod_user_op operation "password" for explanation about password lengths. This is apparently the enforced
// longest password length for native authentication. The random password is used with native authentication and so
// it cannot exceed this size. Also, make sure the output buffer can hold the randomly generated password.
constexpr auto random_password_len = MAX_PASSWORD_LEN - 8;
if (random_password_len + 1 > _password_buffer_size) {
return ERROR(
SYS_INVALID_INPUT_PARAM,
fmt::format("{}: Buffer not large enough to hold password. Requires [{}] bytes, received [{}] bytes.",
__func__,
random_password_len + 1,
_password_buffer_size));
}

constexpr auto random_bytes_count = 64 + 1; // +1 for null terminator
std::array<char, random_bytes_count> random_bytes_buffer{};
get64RandomBytes(random_bytes_buffer.data());
// The length of the randomly generated password must fit into the buffer for the random password in scrambled form.
std::array<char, MAX_PASSWORD_LEN + 1> scrambled_random_password{}; // +1 for null terminator
static_assert(random_password_len < scrambled_random_password.size());

// Minus 1 for null terminator.
for (std::size_t i = 0; i < random_password.size() - 1; i++) {
constexpr auto bitmask = 0x7f;
// NOLINTNEXTLINE(hicpp-signed-bitwise,bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
char c = random_bytes_buffer.at(i) & bitmask;
if (c < '0') {
c += '0';
}
if ((c > 'a' && c < 'z') || (c > 'A' && c < 'Z') || (c > '0' && c < '9')) {
random_password.at(i) = c;
}
}
random_password.back() = '\0';
// Historically, the randomly generated password had been a string no longer than MAX_PASSWORD_LEN with characters
// in the set [1-8B-Yb-y]. The string is now guaranteed to be the specified length and contain characters in the
// set of alphanumeric characters.
const auto random_password = irods::generate_random_alphanumeric_string(random_password_len);

std::strncpy(scrambled_random_password.data(), random_password.data(), MAX_PASSWORD_LEN);
// Copy the randomly generated password because icatScramble modifies the passed-in buffer. We want to retain the
// unscrambled password so that it can be returned to the user for future authentication purposes.
std::strncpy(scrambled_random_password.data(), random_password.c_str(), random_password_len + 1);
icatScramble(scrambled_random_password.data());

if ( _test_time != NULL && strlen( _test_time ) > 0 ) {
Expand All @@ -7352,7 +7357,8 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx,
return ERROR( status, "commit failure" );
}

std::strncpy(*_password_buffer, random_password.data(), _password_buffer_size);
// Return the unscrambled, randomly generated password to the user.
std::strncpy(*_password_buffer, random_password.c_str(), _password_buffer_size);

return SUCCESS();
} // db_update_pam_password_op
Expand Down

0 comments on commit cc29f4d

Please sign in to comment.