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

Regex change for domainName field #113

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Regex change for domainName field #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7f3eab
Regex change for domainName field
Jan 23, 2024
e6af7d2
test
Jan 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 73 additions & 6 deletions api/src/gmsa_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#define LEASE_ID_LENGTH 10
#define UNIX_SOCKET_NAME "credentials_fetcher.sock"
#define INPUT_CREDENTIALS_LENGTH 104
//https://devblogs.microsoft.com/oldnewthing/20120412-00/?p=7873
#define DOMAIN_LENGTH 253

// invalid character in username/account name
//https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/bb726984
Expand Down Expand Up @@ -59,9 +61,9 @@ void secureClearString(std::string& str) {
*/
bool isValidDomain(const std::string& value)
{

// Regex to check valid domain name.
std::regex pattern("^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9](?:\\.[a-zA-Z]{2,})+$");
// referenced from https://www.rfc-editor.org/rfc/rfc1123
std::regex pattern("^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\\-]{0,61}[a-zA-Z0-9])(\\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\\-]{0,61}[a-zA-Z0-9]))*$");

// If the domain name
// is empty return false
Expand Down Expand Up @@ -475,10 +477,26 @@ class CredentialsFetcherImpl final
krb_ticket_arns->credential_spec_arn = results[0];
int parse_result = parse_cred_spec_domainless(
response, krb_ticket_info, krb_ticket_arns );
if(parse_result != 0)
{
err_msg = "ERROR: invalid credentialspec fields";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}

// only add the ticket info if the parsing is successful
if ( parse_result == 0 )
{
std::string secretsArn =
krb_ticket_arns->credential_domainless_user_arn;
if(secretsArn.empty())
{
err_msg = "ERROR: invalid secrets manager arn";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}
// retrieve domainless user credentials
std::tuple<std::string, std::string, std::string> userCreds =
retrieve_credspec_from_secrets_manager(
Expand All @@ -495,7 +513,7 @@ class CredentialsFetcherImpl final
if ( !username.empty() && !password.empty() &&
!domain.empty() &&
username.length() < INPUT_CREDENTIALS_LENGTH &&
password.length() < INPUT_CREDENTIALS_LENGTH )
password.length() < INPUT_CREDENTIALS_LENGTH && domain.length() < DOMAIN_LENGTH)
{

std::string krb_files_path =
Expand Down Expand Up @@ -866,9 +884,27 @@ class CredentialsFetcherImpl final
int parse_result = parse_cred_spec_domainless(
response, krb_ticket_info, krb_ticket_arns );

if(parse_result != 0)
{
err_msg = "ERROR: invalid credentialspec fields";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}

// only add the ticket info if the parsing is successful
if ( parse_result == 0 )
{
std::string secretsArn =
krb_ticket_arns->credential_domainless_user_arn;
if(secretsArn.empty())
{
err_msg = "ERROR: invalid secrets manager arn";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}

// retrieve domainless user credentials
std::tuple<std::string, std::string, std::string> userCreds =
retrieve_credspec_from_secrets_manager(
Expand All @@ -886,7 +922,7 @@ class CredentialsFetcherImpl final
if ( !username.empty() && !password.empty() &&
!domain.empty() &&
username.length() < INPUT_CREDENTIALS_LENGTH &&
password.length() < INPUT_CREDENTIALS_LENGTH )
password.length() < INPUT_CREDENTIALS_LENGTH && domain.length() < DOMAIN_LENGTH)
{
std::string renewal_path = renew_gmsa_ticket(
krb_ticket, domain, username, password, cf_logger );
Expand All @@ -898,13 +934,15 @@ class CredentialsFetcherImpl final
"credentials should not be more than 256 charaters";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}
}
else
{
err_msg = "ERROR: invalid domainName/username";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}
}
}
Expand Down Expand Up @@ -1103,6 +1141,14 @@ class CredentialsFetcherImpl final
int parse_result = parse_cred_spec( create_krb_request_.credspec_contents( i ),
krb_ticket_info );

if(parse_result != 0)
{
err_msg = "ERROR: invalid credentialspec fields";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}

// only add the ticket info if the parsing is successful
if ( parse_result == 0 )
{
Expand Down Expand Up @@ -1375,18 +1421,38 @@ class CredentialsFetcherImpl final
!contains_invalid_characters_in_ad_account_name(username))
{
if ( !username.empty() && !password.empty() && !domain.empty() && username.length() < INPUT_CREDENTIALS_LENGTH && password.length() <
INPUT_CREDENTIALS_LENGTH )
INPUT_CREDENTIALS_LENGTH
&& domain.length() < DOMAIN_LENGTH && create_domainless_krb_request_
.credspec_contents_size() > 0)
{
create_domainless_krb_reply_.set_lease_id( lease_id );
for ( int i = 0;
i < create_domainless_krb_request_.credspec_contents_size(); i++ )
{
std::string credspecContent = create_domainless_krb_request_
.credspec_contents( i );
if(credspecContent.empty())
{
err_msg = "Error: credentialspec content shouldn't be empty "
"formatted";
std::cout << getCurrentTime() << '\t' << err_msg << std::endl;
break;
}
creds_fetcher::krb_ticket_info* krb_ticket_info =
new creds_fetcher::krb_ticket_info;

int parse_result = parse_cred_spec(
create_domainless_krb_request_.credspec_contents( i ),
krb_ticket_info );

if(parse_result != 0)
{
err_msg = "ERROR: invalid credentialspec fields";
std::cout << getCurrentTime() << '\t' << err_msg
<< std::endl;
break;
}

// only add the ticket info if the parsing is successful
if ( parse_result == 0 )
{
Expand Down Expand Up @@ -1678,7 +1744,8 @@ class CredentialsFetcherImpl final
!contains_invalid_characters_in_ad_account_name(username))
{
if ( !username.empty() && !password.empty() && !domain.empty() && username.length() < INPUT_CREDENTIALS_LENGTH && password.length() <
INPUT_CREDENTIALS_LENGTH )
INPUT_CREDENTIALS_LENGTH
&& domain.length() < DOMAIN_LENGTH)
{
std::list<std::string> renewed_krb_file_paths =
renew_kerberos_tickets_domainless( krb_files_dir, domain, username,
Expand Down
29 changes: 24 additions & 5 deletions api/tests/gmsa_test_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ int run_stress_test( CredentialsFetcherClient& client, int num_of_leases,
}

// unit tests
int parse_credspec_domainless_test(std::string credspec)
bool parse_credspec_domainless_test(std::string credspec)
{
creds_fetcher::krb_ticket_info* krb_ticket_info =
new creds_fetcher::krb_ticket_info;
Expand All @@ -581,7 +581,19 @@ int parse_credspec_domainless_test(std::string credspec)
int response = parse_cred_spec_domainless(credspec, krb_ticket_info, krb_ticket_arn_mapping );
std::cout << krb_ticket_arn_mapping->credential_spec_arn;
std::cout << krb_ticket_arn_mapping->krb_file_path;
return response;
if(response == 0)
{
return true;
}
return false;
}

int validate_domain()
{
return (isValidDomain("a.com") && isValidDomain("ab.toto-abc.com") &&
!isValidDomain("p/") && isValidDomain("test4.gmsa-pentest.com") &&
!isValidDomain ("-testdomain.org") && isValidDomain("contoso.com") &&
!isValidDomain(".org"));
saikiranakula-amzn marked this conversation as resolved.
Show resolved Hide resolved
}

#if AMAZON_LINUX_DISTRO
Expand Down Expand Up @@ -678,9 +690,16 @@ int main( int argc, char** argv )
}
else if (arg == "--unit_test")
{
parse_credspec_domainless_test(credspec_contents_domainless_str);
//retrieve_credspec_from_s3_test();
return 0;

bool testStatus = (parse_credspec_domainless_test(credspec_contents_domainless_str) && validate_domain());
if(!testStatus){
std::cout << "client tests failed" << std::endl;
return EXIT_FAILURE;
}
else{
std::cout << "client tests successful" << std::endl;
return EXIT_SUCCESS;
}
}
else if ( arg == "--check" )
{
Expand Down
9 changes: 8 additions & 1 deletion auth/kerberos/src/krb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,13 +1126,20 @@ std::string retrieve_secret_from_ecs_config(std::string ecs_variable_name)

while ( std::getline( config_file, line ) )
{
// TBD: Error handling for incorrectly formatted /etc/ecs/ecs.config
results = split_string(line, '=');
std::string key = results[0];
std::string value = results[1];
if ( ecs_variable_name.compare( key ) == 0 )
{
value.erase( std::remove( value.begin(), value.end(), '"' ), value.end() );

if( contains_invalid_characters_in_ad_account_name(value))
{
std::cout << getCurrentTime() << '\t' << "invalid domain controller name" <<
std::endl;
saikiranakula-amzn marked this conversation as resolved.
Show resolved Hide resolved
return "";
}

return value;
}
}
Expand Down
11 changes: 10 additions & 1 deletion common/daemon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,17 @@ namespace creds_fetcher
{
if ( level >= log_level )
{
sd_journal_print( level, fmt, logs... );
std::string logFmt = fmt;
for (int i = 0; logFmt[i] != '\0'; ++i) {
if (logFmt[i] == '\n') {
logFmt[i] = ' '; // Replace '\n' with space
}
}
sd_journal_print( level, logFmt.c_str(), logs... );
}
}


void init_file_logger ()
{
std::string log_file_path = LOG_FILE_PATH;
Expand Down Expand Up @@ -190,6 +197,7 @@ int renewal_failure_krb_dir_not_found_test();
* Methods in config module
*/
int parse_options( int argc, const char* argv[], creds_fetcher::Daemon& cf_daemon );
bool isValidDomain(const std::string& value);
int HealthCheck(std::string serviceName);

int parse_config_file( creds_fetcher::Daemon& cf_daemon );
Expand All @@ -203,6 +211,7 @@ bool contains_invalid_characters_in_credentials( const std::string& value );
int RunGrpcServer( std::string unix_socket_dir, std::string krb_file_path,
creds_fetcher::CF_logger& cf_logger, volatile sig_atomic_t* shutdown_signal,
std::string aws_sm_secret_name );
bool contains_invalid_characters_in_ad_account_name( const std::string& value );

int parse_cred_spec( std::string credspec_data, creds_fetcher::krb_ticket_info* krb_ticket_info );

Expand Down
Loading