From 0e26066dfff8efff0039da13e29609ca7f00d9a2 Mon Sep 17 00:00:00 2001 From: Dusan Vuckovic Date: Mon, 1 Jul 2019 11:54:48 +0200 Subject: [PATCH] Improved value masking in tag replacement method. --- CHANGES.md | 1 + Kernel/System/TemplateGenerator.pm | 64 ++++++++-- scripts/test/TemplateGenerator/Replace.t | 148 +++++++++++++++++++++-- 3 files changed, 192 insertions(+), 21 deletions(-) --- a/Kernel/System/TemplateGenerator.pm +++ b/Kernel/System/TemplateGenerator.pm @@ -1049,14 +1049,16 @@ sub _Replace { # Replace config options. my $Tag = $Start . 'OTRS_CONFIG_'; $Param{Text} =~ s{$Tag(.+?)$End}{ - my $Replace = ''; - # Mask secret config options. - if ($1 =~ m{(Password|Pw)\d*$}smxi) { - $Replace = 'xxx'; - } - else { - $Replace = $ConfigObject->Get($1) // ''; - } + my $Key = $1; + my $Value = $ConfigObject->Get($Key) // ''; + + # Mask sensitive config options. + my $Replace = $Self->_MaskSensitiveValue( + Key => $Key, + Value => $Value, + IsConfig => 1, + ); + $Replace; }egx; @@ -1082,11 +1084,15 @@ sub _Replace { # Generate one single matching string for all keys to save performance. my $Keys = join '|', map {quotemeta} grep { defined $H{$_} } keys %H; - # Add all keys also as lowercase to be able to match case insensitive, + # Set all keys as lowercase to be able to match case insensitive, # e. g. and . - for my $Key ( sort keys %H ) { - $H{ lc $Key } = $H{$Key}; - } + # Also mask any values containing sensitive data. + %H = map { + lc $_ => $Self->_MaskSensitiveValue( + Key => $_, + Value => $H{$_}, + ) + } sort keys %H; $Param{Text} =~ s/(?:$Tag)($Keys)$End/$H{ lc $1 }/ieg; }; @@ -1580,6 +1586,40 @@ sub _RemoveUnSupportedTag { } +=head2 _MaskSensitiveValue() + +Mask sensitive value, i.e. a password, a security token, etc. + + my $MaskedValue = $Self->_MaskSensitiveValue( + Key => 'DatabasePassword', # (required) Name of the field/key. + Value => 'secretvalue', # (optional) Value to potentially mask. + IsConfig => 1, # (optional) Whether the value is a config option, default: 0. + ); + +Returns masked value, in case the key is matched: + + $MaskedValue = 'xxx'; + +=cut + +sub _MaskSensitiveValue { + my ( $Self, %Param ) = @_; + + return '' if !$Param{Key} || !defined $Param{Value}; + + # Match general key names, i.e. from the user preferences. + my $Match = qr{ config|secret|passw|userpw|auth|token }xi; + + # Match forbidden config keys. + if ( $Param{IsConfig} ) { + $Match = qr{ (?:password|pw) \d* $ }smxi; + } + + return $Param{Value} if $Param{Key} !~ $Match; + + return 'xxx'; +} + 1; =end Internal: --- a/scripts/test/TemplateGenerator/Replace.t +++ b/scripts/test/TemplateGenerator/Replace.t @@ -17,6 +17,8 @@ my $ConfigObject = $Kernel::OM->Ge my $DynamicFieldObject = $Kernel::OM->Get('Kernel::System::DynamicField'); my $BackendObject = $Kernel::OM->Get('Kernel::System::DynamicField::Backend'); my $TicketObject = $Kernel::OM->Get('Kernel::System::Ticket'); +my $UserObject = $Kernel::OM->Get('Kernel::System::User'); +my $CustomerUserObject = $Kernel::OM->Get('Kernel::System::CustomerUser'); # get helper object $Kernel::OM->ObjectParamAdd( @@ -101,6 +103,18 @@ my $TestCustomerLogin = $Helper->TestCus Language => 'en', ); +# Add a random secret for the customer user. +$CustomerUserObject->SetPreferences( + Key => 'UserGoogleAuthenticatorSecretKey', + Value => $Helper->GetRandomID(), + UserID => $TestCustomerLogin, +); + +# Generate a token for the customer user. +$CustomerUserObject->TokenGenerate( + UserID => $TestCustomerLogin, +); + my $TestUserLogin = $Helper->TestUserCreate( Language => 'en', ); @@ -109,6 +123,18 @@ my %TestUser = $Kernel::OM->Get('Kernel: User => $TestUserLogin, ); +# Add a random secret for the user. +$UserObject->SetPreferences( + Key => 'UserGoogleAuthenticatorSecretKey', + Value => $Helper->GetRandomID(), + UserID => $TestUser{UserID}, +); + +# Generate a token for the user. +$UserObject->TokenGenerate( + UserID => $TestUser{UserID}, +); + my $TestUser2Login = $Helper->TestUserCreate( Language => 'en', ); @@ -117,6 +143,18 @@ my %TestUser2 = $Kernel::OM->Get('Kernel User => $TestUserLogin, ); +# Add a random secret for the user. +$UserObject->SetPreferences( + Key => 'UserGoogleAuthenticatorSecretKey', + Value => $Helper->GetRandomID(), + UserID => $TestUser2{UserID}, +); + +# Generate a token for the user. +$UserObject->TokenGenerate( + UserID => $TestUser2{UserID}, +); + my $TestUser3Login = $Helper->TestUserCreate( Language => 'en', ); @@ -125,6 +163,18 @@ my %TestUser3 = $Kernel::OM->Get('Kernel User => $TestUserLogin, ); +# Add a random secret for the user. +$UserObject->SetPreferences( + Key => 'UserGoogleAuthenticatorSecretKey', + Value => $Helper->GetRandomID(), + UserID => $TestUser3{UserID}, +); + +# Generate a token for the user. +$UserObject->TokenGenerate( + UserID => $TestUser3{UserID}, +); + my $TestUser4Login = $Helper->TestUserCreate( Language => 'en', ); @@ -133,6 +183,18 @@ my %TestUser4 = $Kernel::OM->Get('Kernel User => $TestUserLogin, ); +# Add a random secret for the user. +$UserObject->SetPreferences( + Key => 'UserGoogleAuthenticatorSecretKey', + Value => $Helper->GetRandomID(), + UserID => $TestUser4{UserID}, +); + +# Generate a token for the user. +$UserObject->TokenGenerate( + UserID => $TestUser4{UserID}, +); + my $TicketID = $TicketObject->TicketCreate( Title => 'Some Ticket_Title', Queue => 'Raw', @@ -270,7 +332,25 @@ my @Tests = ( Result => "Test $TestUser2{UserFirstname} -", }, { - Name => 'OTRS owner firstname', # + Name => 'OTRS responsible password (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx -', + }, + { + Name => 'OTRS responsible secrets (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx xxx', + }, + { + Name => 'OTRS owner firstname', # Data => { From => 'test@home.com', }, @@ -279,7 +359,16 @@ my @Tests = ( Result => "Test $TestUser{UserFirstname} -", }, { - Name => 'OTRS_TICKET_OWNER firstname', # + Name => 'OTRS owner password (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx -', + }, + { + Name => 'OTRS_TICKET_OWNER firstname', # Data => { From => 'test@home.com', }, @@ -288,7 +377,16 @@ my @Tests = ( Result => "Test $TestUser{UserFirstname} -", }, { - Name => 'OTRS current firstname', # + Name => 'OTRS owner secrets (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx xxx', + }, + { + Name => 'OTRS current firstname', # Data => { From => 'test@home.com', }, @@ -297,7 +395,25 @@ my @Tests = ( Result => "Test $TestUser3{UserFirstname} -", }, { - Name => 'OTRS ticket ticketid', # + Name => 'OTRS current password (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx -', + }, + { + Name => 'OTRS current secrets (masked)', + Data => { + From => 'test@home.com', + }, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx xxx', + }, + { + Name => 'OTRS ticket ticketid', # Data => { From => 'test@home.com', }, @@ -306,7 +422,7 @@ my @Tests = ( Result => 'Test ' . $TicketID, }, { - Name => 'OTRS dynamic field (text)', # + Name => 'OTRS dynamic field (text)', # Data => { From => 'test@home.com', }, @@ -315,7 +431,7 @@ my @Tests = ( Result => 'Test otrs', }, { - Name => 'OTRS dynamic field value (text)', # + Name => 'OTRS dynamic field value (text)', # Data => { From => 'test@home.com', }, @@ -324,7 +440,7 @@ my @Tests = ( Result => 'Test otrs', }, { - Name => 'OTRS dynamic field (Dropdown)', # + Name => 'OTRS dynamic field (Dropdown)', # Data => { From => 'test@home.com', }, @@ -333,7 +449,7 @@ my @Tests = ( Result => 'Test 1', }, { - Name => 'OTRS dynamic field value (Dropdown)', # + Name => 'OTRS dynamic field value (Dropdown)', # Data => { From => 'test@home.com', }, @@ -342,7 +458,7 @@ my @Tests = ( Result => 'Test A', }, { - Name => 'OTRS config value', # + Name => 'OTRS config value', # Data => {}, RichText => 0, Template => 'Test ', @@ -546,6 +662,20 @@ Line7', Result => "Test $TestCustomerLogin", }, { + Name => 'OTRS CUSTOMER DATA UserPassword (masked)', + Data => {}, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx', + }, + { + Name => 'OTRS CUSTOMER DATA secret (masked)', + Data => {}, + RichText => 0, + Template => 'Test ', + Result => 'Test xxx xxx', + }, + { Name => 'OTRS ', Data => {}, RichText => 0,