Saving then reading pw (session token) client-side fails when length is close to MAX_PASSWORD_LEN

[theferrit32]

While upgrading the session token used to identify a logged-in irods session, an issue was encountered in the obf module of irods.

The base64ed sha256 hash used is 44 bytes, which is under the MAX_PASSWORD_LEN (50bytes). When this is passed to obfSavePw, the data written to .irodsA is more than 50 bytes. This should be fine because that is not the raw byte length of the password, so the 'password' value itself does not exceed MAX_PASSWORD_LEN.

However when calling obfGetPw, there is a check
(https://github.com/irods/irods/blob/master/lib/core/src/obf.cpp#L441)
which makes sure the length of the data in .irodsA is not more than MAX_PASSWORD_LEN. The data has not been 'decoded' to its original length and contents yet though, so at that stage of the process if the pw saved in obfSavePw was close to 50 bytes long, obfGetPw will always fail.

In this case a pw of length 44 (from base64(sha256)) created an irodsA of length 51. As a temporary fix I am just truncating the token down to 40 bytes, which results in an irodsA of length 47.

[theferrit32]

I am working around this currently by bypassing the obf module entirely

irods_auth_plugin_openid/src/libopenid.cpp

Lines 132 to 201 in c83d746

	std::string sess_filename()
	{
	char path[LONG_NAME_LEN + 1];
	memset( path, 0, LONG_NAME_LEN + 1 );
	char *env = getRodsEnvAuthFileName();
	if ( env != NULL && *env != '\0' ) {
	return std::string( env );
	}
	env = getenv( "HOME" );
	debug( "HOME: " + std::string( env ) );
	if ( env == NULL ) {
	throw std::runtime_error( "environment variable HOME not defined: "
	+ std::to_string( ENVIRONMENT_VAR_HOME_NOT_DEFINED ) );
	}
	strncpy( path, env, strlen( env ) );
	strncat( path, "/", MAX_NAME_LEN - strlen( path ) );
	strncat( path, AUTH_FILENAME_DEFAULT, MAX_NAME_LEN - strlen( path ) );
	return std::string( path );
	}
	int write_sess_file( std::string val )
	{
	debug( "entering write_sess_file" );
	FILE *fd = fopen( sess_filename().c_str(), "w+" );
	if ( !fd ) {
	perror( "could not open session file for writing" );
	return -1;
	}
	size_t n_char = fwrite( val.c_str(), sizeof( char ), val.size(), fd );
	if ( n_char != val.size() ) {
	printf( "Could not write value to session. Length was %lu but only wrote %lu\n", val.size(), n_char );
	return -2;
	}
	debug( "leaving write_sess_file" );
	return 0;
	}
	int read_sess_file( std::string& val_out )
	{
	debug( "entering read_sess_file" );
	FILE *fd = fopen( sess_filename().c_str(), "r" );
	if ( !fd ) {
	perror( "could not open session file for reading" );
	return -1;
	}
	size_t total_bytes = 0;
	while ( true ) {
	char buf[256];
	memset( buf, 0, 256 );
	size_t n_char = fread( buf, sizeof( char ), 255, fd );
	if ( n_char > 0 ) {
	total_bytes += n_char;
	val_out.append( buf );
	}
	if ( n_char < 255 ) {
	break;
	}
	}
	if ( feof( fd ) ) {
	debug( "leaving read_sess_file: " + val_out );
	return 0;
	}
	else if ( ferror( fd ) ) {
	printf( "error during file read\n" );
	return -2;
	}
	else {
	printf( "unknown error occurred reading session file\n" );
	return -3;
	}
	}

I may stick with this approach regardless of the resolution of the bug in irods, because it enables straightforward session injection (auto-login) to irods by an external application with rodsadmin privileges. Since session tokens used by the openid plugin are completely opaque anyways (not a password, and not reversible to an oauth2 token) the obf functions are not a major priority.