From bce36cb30a4ae24e2e2dcdc77a60a40e8b261d0d Mon Sep 17 00:00:00 2001 From: Dan Wells Date: Fri, 12 Oct 2012 09:45:35 -0400 Subject: [PATCH] Log redaction for sensitive input values, Perl side This commit attempts to do the same as the C log redaction fix, but now at the Perl level. The Perl configuration code was a little more crufty than the C side, so an additional feature was added to Config.pm to support the new 'shared' section. At some point we should consider a ground-up rewrite of Config.pm, as the code seems to suffer some from its INI file roots. Signed-off-by: Dan Wells Signed-off-by: Dan Scott --- examples/opensrf_core.xml.example | 6 ++-- src/perl/lib/OpenSRF/Application.pm | 22 ++++++++++++- src/perl/lib/OpenSRF/System.pm | 9 ++++++ src/perl/lib/OpenSRF/Utils/Config.pm | 60 ++++++++++++++++++++++++++++-------- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/examples/opensrf_core.xml.example b/examples/opensrf_core.xml.example index a4367d3..5e347e1 100644 --- a/examples/opensrf_core.xml.example +++ b/examples/opensrf_core.xml.example @@ -163,12 +163,12 @@ vim:et:ts=2:sw=2: - + diff --git a/src/perl/lib/OpenSRF/Application.pm b/src/perl/lib/OpenSRF/Application.pm index 6ebfe4b..93cb92d 100644 --- a/src/perl/lib/OpenSRF/Application.pm +++ b/src/perl/lib/OpenSRF/Application.pm @@ -22,6 +22,7 @@ $log = 'OpenSRF::Utils::Logger'; our $in_request = 0; our @pending_requests; +our $shared_conf; sub package { my $self = shift; @@ -127,7 +128,26 @@ sub handler { my @p = $app_msg->params; my $method_name = $app_msg->method; my $method_proto = $session->last_message_api_level; - $log->info("CALL: ".$session->service." $method_name ". (@p ? join(', ',@p) : '')); + + # By default, we log all method params at the info level + # Here we are consult our shared portion of the config file + # to look for any exceptions to this behavior + my $logdata = "CALL: ".$session->service." $method_name "; + my $redact_params = 0; + if (@p) { + foreach my $match_string (@{$shared_conf->shared->log_protect}) { + if ($method_name =~ /^$match_string/) { + $redact_params = 1; + last; + } + } + if ($redact_params) { + $logdata .= "**PARAMS REDACTED**"; + } else { + $logdata .= join(', ',@p); + } + } + $log->info($logdata); my $coderef = $app->method_lookup( $method_name, $method_proto, 1, 1 ); diff --git a/src/perl/lib/OpenSRF/System.pm b/src/perl/lib/OpenSRF/System.pm index 7fd7195..62a17a8 100644 --- a/src/perl/lib/OpenSRF/System.pm +++ b/src/perl/lib/OpenSRF/System.pm @@ -84,6 +84,15 @@ sub run_service { OpenSRF::Application->application_implementation->initialize() if (OpenSRF::Application->application_implementation->can('initialize')); + # Read in a shared portion of the config file + # for later use in log parameter redaction + $OpenSRF::Application::shared_conf = OpenSRF::Utils::Config->load( + 'config_file' => OpenSRF::Utils::Config->current->FILE, + 'nocache' => 1, + 'force' => 1, + 'base_path' => '/config/shared' + ); + # kill the temp connection OpenSRF::Transport::PeerHandle->retrieve->disconnect; diff --git a/src/perl/lib/OpenSRF/Utils/Config.pm b/src/perl/lib/OpenSRF/Utils/Config.pm index 9ecfc0e..5553dfb 100644 --- a/src/perl/lib/OpenSRF/Utils/Config.pm +++ b/src/perl/lib/OpenSRF/Utils/Config.pm @@ -28,6 +28,8 @@ sub new { $self->_sub_builder('__id'); # Hard-code this to match old bootstrap.conf section name + # This hardcoded value is later overridden if the config is loaded + # with the 'base_path' option $self->__id('bootstrap'); my $bootstrap = shift; @@ -135,9 +137,6 @@ XML elements are pushed into arrays and added as an array reference to the hash. Scalar values have whitespace trimmed from the left and right sides. -Child elements of C<< >> other than C<< >> are -currently ignored by this module. - =head1 EXAMPLE Given an OpenSRF configuration file named F with the @@ -174,12 +173,29 @@ section of C<$config_obj>; for example: =head1 NOTES -For compatibility with a previous version of OpenSRF configuration -files, the F section has a hardcoded name of -B. However, future iterations of this module may extend the -ability of the module to parse the entire OpenSRF configuration file -and provide sections named after the sibling elements of -C. +For compatibility with previous versions of the OpenSRF configuration +files, the C method by default loads the C +section with the hardcoded name of B. + +However, it is possible to load child elements of C<< >> other +than C<< >> by supplying a C argument which specifies +the node you wish to begin loading from (in XPath notation). Doing so +will also replace the hardcoded C name with the node name of +the last member of the given path. For example: + + my $config_obj = OpenSRF::Utils::Config->load( + config_file => '/config/file.cnf' + base_path => '/config/shared' + ); + + my $attrs_href = $config_obj->shared(); + +While it may be possible to load the entire file in this fashion (by +specifying an empty C), doing so will break compatibility with +existing code which expects to find a C member. Future +iterations of this module may extend its ability to parse the entire +OpenSRF configuration file in one pass while providing multiple base +sections named after the sibling elements of C. Hashrefs of sections can be returned by calling a method of the object of the same name as the section. They can be set by passing a hashref @@ -242,7 +258,11 @@ sub _load { delete $$self{config_file}; return undef unless ($self->FILE); - $self->load_config(); + my %load_args; + if (exists $$self{base_path}) { # blank != non-existent for this setting + $load_args{base_path} = $$self{base_path}; + } + $self->load_config(%load_args); $self->load_env(); $self->mangle_dirs(); $self->mangle_logs(); @@ -250,6 +270,7 @@ sub _load { $OpenSRF::Utils::ConfigCache = $self unless $self->nocache; delete $$self{nocache}; delete $$self{force}; + delete $$self{base_path}; return $self; } @@ -315,6 +336,7 @@ sub mangle_dirs { sub load_config { my $self = shift; my $parser = XML::LibXML->new(); + my %args = @_; # Hash of config values my %bootstrap; @@ -327,9 +349,16 @@ sub load_config { die "Could not open ".$self->FILE.": $!\n"; } + # For backwards compatibility, we default to /config/opensrf + my $base_path; + if (exists $args{base_path}) { # allow for empty to import entire file + $base_path = $args{base_path}; + } else { + $base_path = '/config/opensrf'; + } # Return an XML::LibXML::NodeList object matching all child elements - # of ... - my $osrf_cfg = $config->findnodes('/config/opensrf/child::*'); + # of $base_path... + my $osrf_cfg = $config->findnodes("$base_path/child::*"); # Iterate through the nodes to pull out key=>value pairs of config settings foreach my $node ($osrf_cfg->get_nodelist()) { @@ -354,6 +383,13 @@ sub load_config { } my $section = $self->section_pkg->new(\%bootstrap); + # if the Config was loaded with a 'base_path' option, overwrite the + # hardcoded 'bootstrap' name with something more reasonable + if (exists $$self{base_path}) { # blank != non-existent for this setting + # name root node to reflect last member of base_path, or default to root + my $root = (split('/', $$self{base_path}))[-1] || 'root'; + $section->__id($root); + } my $sub_name = $section->SECTION; $self->_sub_builder($sub_name); $self->$sub_name($section); -- 2.11.0