[OpenSRF-GIT] OpenSRF branch rel_2_1 updated. osrf_rel_2_1_0-7-gf4365c8
Evergreen Git
git at git.evergreen-ils.org
Thu Nov 8 12:52:07 EST 2012
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "OpenSRF".
The branch, rel_2_1 has been updated
via f4365c898eab6678889431b49133bb94077298d3 (commit)
via fd53b3667a2e85ab53c60f1806cbe93d5b331497 (commit)
via 451115acca840ebfef57320a3ce435db0223f2d6 (commit)
via 2121279aeda9ae27dcacadfdaa6c5c1c23099196 (commit)
from cd24bb1c94c95027310f63909b692e4dbb05507f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit f4365c898eab6678889431b49133bb94077298d3
Author: Dan Wells <dbw2 at calvin.edu>
Date: Wed Oct 31 10:21:14 2012 -0400
Protect against empty/invalid log_protect sections
Connectivity would fail if the log_protect section of opensrf_core.xml
was empty, or contained only comments, as it does in the default
example. Add a simple guard against this potential problem. This fix
will also protect against cases where the 'log_protect' section
contains text rather than separate configuration elements.
Credit to Dan Scott for finding the bug and working out how to fix it.
Signed-off-by: Dan Wells <dbw2 at calvin.edu>
Signed-off-by: Dan Scott <dan at coffeecode.net>
diff --git a/src/perl/lib/OpenSRF/Application.pm b/src/perl/lib/OpenSRF/Application.pm
index 93cb92d..9283b7f 100644
--- a/src/perl/lib/OpenSRF/Application.pm
+++ b/src/perl/lib/OpenSRF/Application.pm
@@ -135,10 +135,12 @@ sub handler {
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 (ref($shared_conf->shared->log_protect) eq 'ARRAY') {
+ foreach my $match_string (@{$shared_conf->shared->log_protect}) {
+ if ($method_name =~ /^$match_string/) {
+ $redact_params = 1;
+ last;
+ }
}
}
if ($redact_params) {
commit fd53b3667a2e85ab53c60f1806cbe93d5b331497
Author: Bill Erickson <berick at esilibrary.com>
Date: Wed Oct 17 15:57:14 2012 -0400
Consolidate duplicate osrfMethodVerifyContext handler
Log redaction was not occuring in some cases because the code was using
the OSRF_METHOD_VERIFY_CONTEXT macro instead of the similarly named
function which does the same thing. This change points the macro at the
function so that all code uses the same underlying code.
Note this change turns on CALL param logging unconditionally, whereas
users of the macro would previously have been able to avoid CALL logging
via the OSRF_LOG_PARAMS variable. In practice, little code uses the
macro and all code enables OSRF_LOG_PARAMS. If we need to add this
control back, it can be added directly to osrfMethodVerifyContext().
For now, it's one less env variable we need to define.
Signed-off-by: Bill Erickson <berick at esilibrary.com>
Signed-off-by: Dan Scott <dan at coffeecode.net>
diff --git a/include/opensrf/osrf_application.h b/include/opensrf/osrf_application.h
index d05f2cc..e50e114 100644
--- a/include/opensrf/osrf_application.h
+++ b/include/opensrf/osrf_application.h
@@ -57,42 +57,8 @@ extern "C" {
*/
/** This macro verifies that methods receive the correct parameters */
-#define _OSRF_METHOD_VERIFY_CONTEXT(d) \
- if(!d) return -1; \
- if(!d->session) { \
- osrfLogError( OSRF_LOG_MARK, "Session is NULL in app request" ); \
- return -1; \
- } \
- if(!d->method) { \
- osrfLogError( OSRF_LOG_MARK, "Method is NULL in app request" ); \
- return -1; \
- } \
- if(d->method->argc) { \
- if(!d->params) { \
- osrfLogError( OSRF_LOG_MARK, "Params is NULL in app request %s", d->method->name ); \
- return -1; \
- } \
- if( d->params->type != JSON_ARRAY ) { \
- osrfLogError( OSRF_LOG_MARK, "'params' is not a JSON array for method %s", \
- d->method->name); \
- return -1; } \
- } \
- if( !d->method->name ) { \
- osrfLogError( OSRF_LOG_MARK, "Method name is NULL"); return -1; \
- }
-
-#ifdef OSRF_LOG_PARAMS
-#define OSRF_METHOD_VERIFY_CONTEXT(d) \
- _OSRF_METHOD_VERIFY_CONTEXT(d); \
- char* __j = jsonObjectToJSON(d->params); \
- if(__j) { \
- osrfLogInfo( OSRF_LOG_MARK, "CALL:\t%s %s - %s", d->session->remote_service, \
- d->method->name, __j);\
- free(__j); \
- }
-#else
-#define OSRF_METHOD_VERIFY_CONTEXT(d) _OSRF_METHOD_VERIFY_CONTEXT(d);
-#endif
+/** TODO: deprecate me, since I no longer server a purpose */
+#define OSRF_METHOD_VERIFY_CONTEXT(d) osrfMethodVerifyContext(d)
/**
@name Method options
commit 451115acca840ebfef57320a3ce435db0223f2d6
Author: Dan Wells <dbw2 at calvin.edu>
Date: Fri Oct 12 09:45:35 2012 -0400
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 <dbw2 at calvin.edu>
Signed-off-by: Dan Scott <dan at coffeecode.net>
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:
<!-- Any methods which match any of these match_string node values will
have their params redacted from lower-level input logging.
Adjust these examples as needed. -->
- <!--
<log_protect>
- <match_string>open-ils.auth_proxy.login</match_string>
+ <!--
+ <match_string>open-ils.auth</match_string>
<match_string>open-ils.some_service.some_method</match_string>
- </log_protect>
-->
+ </log_protect>
</shared>
</config>
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(', ', at 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(', ', at 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<< <config> >> other than C<< <opensrf> >> are
-currently ignored by this module.
-
=head1 EXAMPLE
Given an OpenSRF configuration file named F<opensrf_core.xml> 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</config/opensrf/> section has a hardcoded name of
-B<bootstrap>. 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</config/opensrf>.
+For compatibility with previous versions of the OpenSRF configuration
+files, the C<load()> method by default loads the C</config/opensrf>
+section with the hardcoded name of B<bootstrap>.
+
+However, it is possible to load child elements of C<< <config> >> other
+than C<< <opensrf> >> by supplying a C<base_path> argument which specifies
+the node you wish to begin loading from (in XPath notation). Doing so
+will also replace the hardcoded C<bootstrap> 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<base_path>), doing so will break compatibility with
+existing code which expects to find a C<bootstrap> 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</config/opensrf>.
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 <config><opensrf>...
- 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);
commit 2121279aeda9ae27dcacadfdaa6c5c1c23099196
Author: Dan Wells <dbw2 at calvin.edu>
Date: Wed Oct 10 17:28:07 2012 -0400
Log redaction for sensitive input values, C side
Some service/methods deal with sensitive information (passwords,
financial, etc.). All input values (e.g. gateway method params)
are currently logged in the activity log regardless of your log
level. This commit will allow you to redact the params of any
method which matches a configurable set of left-anchored string
values.
This commit adds the initial config, and covers redaction of method
parameters sent through the gateway, the translator, and more general
OpenSRF C applications.
Signed-off-by: Dan Wells <dbw2 at calvin.edu>
Signed-off-by: Dan Scott <dan at coffeecode.net>
diff --git a/examples/opensrf_core.xml.example b/examples/opensrf_core.xml.example
index 816a20c..a4367d3 100644
--- a/examples/opensrf_core.xml.example
+++ b/examples/opensrf_core.xml.example
@@ -159,4 +159,16 @@ vim:et:ts=2:sw=2:
<!-- ======================================================================================== -->
+ <shared>
+ <!-- Any methods which match any of these match_string node values will
+ have their params redacted from lower-level input logging.
+ Adjust these examples as needed. -->
+ <!--
+ <log_protect>
+ <match_string>open-ils.auth_proxy.login</match_string>
+ <match_string>open-ils.some_service.some_method</match_string>
+ </log_protect>
+ -->
+ </shared>
+
</config>
diff --git a/include/opensrf/osrf_application.h b/include/opensrf/osrf_application.h
index d7560be..d05f2cc 100644
--- a/include/opensrf/osrf_application.h
+++ b/include/opensrf/osrf_application.h
@@ -35,6 +35,7 @@
#include <opensrf/log.h>
#include <opensrf/osrf_app_session.h>
#include <opensrf/osrf_hash.h>
+#include <opensrf/string_array.h>
#include <opensrf/osrf_json.h>
#include <stdio.h>
diff --git a/include/opensrf/osrf_system.h b/include/opensrf/osrf_system.h
index b39645d..328d149 100644
--- a/include/opensrf/osrf_system.h
+++ b/include/opensrf/osrf_system.h
@@ -37,6 +37,8 @@ void osrfSystemIgnoreTransportClient();
int osrfSystemInitCache(void);
+extern osrfStringArray* log_protect_arr;
+
#ifdef __cplusplus
}
#endif
diff --git a/src/gateway/osrf_http_translator.c b/src/gateway/osrf_http_translator.c
index 83855e9..f6d492b 100644
--- a/src/gateway/osrf_http_translator.c
+++ b/src/gateway/osrf_http_translator.c
@@ -8,6 +8,7 @@
#include <opensrf/osrfConfig.h>
#include <opensrf/osrf_json.h>
#include <opensrf/osrf_cache.h>
+#include <opensrf/string_array.h>
#define MODULE_NAME "osrf_http_translator_module"
#define OSRF_TRANSLATOR_CONFIG_FILE "OSRFTranslatorConfig"
@@ -265,20 +266,34 @@ static char* osrfHttpTranslatorParseRequest(osrfHttpTranslator* trans) {
case REQUEST: {
const jsonObject* params = msg->_params;
growing_buffer* act = buffer_init(128);
+ char* method = msg->method_name;
buffer_fadd(act, "[%s] [%s] %s %s", trans->remoteHost, "",
- trans->service, msg->method_name);
+ trans->service, method);
const jsonObject* obj = NULL;
int i = 0;
- char* str;
- while((obj = jsonObjectGetIndex(params, i++))) {
- str = jsonObjectToJSON(obj);
- if( i == 1 )
- OSRF_BUFFER_ADD(act, " ");
- else
- OSRF_BUFFER_ADD(act, ", ");
- OSRF_BUFFER_ADD(act, str);
- free(str);
+ const char* str;
+ int redactParams = 0;
+ while( (str = osrfStringArrayGetString(log_protect_arr, i++)) ) {
+ //osrfLogInternal(OSRF_LOG_MARK, "Checking for log protection [%s]", str);
+ if(!strncmp(method, str, strlen(str))) {
+ redactParams = 1;
+ break;
+ }
+ }
+ if(redactParams) {
+ OSRF_BUFFER_ADD(act, " **PARAMS REDACTED**");
+ } else {
+ i = 0;
+ while((obj = jsonObjectGetIndex(params, i++))) {
+ str = jsonObjectToJSON(obj);
+ if( i == 1 )
+ OSRF_BUFFER_ADD(act, " ");
+ else
+ OSRF_BUFFER_ADD(act, ", ");
+ OSRF_BUFFER_ADD(act, str);
+ free(str);
+ }
}
osrfLogActivity(OSRF_LOG_MARK, "%s", act->buf);
buffer_free(act);
diff --git a/src/gateway/osrf_json_gateway.c b/src/gateway/osrf_json_gateway.c
index 53094e3..ecc02a2 100644
--- a/src/gateway/osrf_json_gateway.c
+++ b/src/gateway/osrf_json_gateway.c
@@ -5,6 +5,7 @@
#include <opensrf/osrf_json.h>
#include <opensrf/osrf_json_xml.h>
#include <opensrf/osrf_legacy_json.h>
+#include <opensrf/string_array.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <unistd.h>
@@ -289,14 +290,28 @@ static int osrf_json_gateway_method_handler (request_rec *r) {
growing_buffer* act = buffer_init(128);
buffer_fadd(act, "[%s] [%s] [%s] %s %s", r->connection->remote_ip,
authtoken, osrf_locale, service, method );
+
const char* str; int i = 0;
- while( (str = osrfStringArrayGetString(mparams, i++)) ) {
- if( i == 1 ) {
- OSRF_BUFFER_ADD(act, " ");
- OSRF_BUFFER_ADD(act, str);
- } else {
- OSRF_BUFFER_ADD(act, ", ");
- OSRF_BUFFER_ADD(act, str);
+ int redact_params = 0;
+ while( (str = osrfStringArrayGetString(log_protect_arr, i++)) ) {
+ //osrfLogInternal(OSRF_LOG_MARK, "Checking for log protection [%s]", str);
+ if(!strncmp(method, str, strlen(str))) {
+ redact_params = 1;
+ break;
+ }
+ }
+ if(redact_params) {
+ OSRF_BUFFER_ADD(act, " **PARAMS REDACTED**");
+ } else {
+ i = 0;
+ while( (str = osrfStringArrayGetString(mparams, i++)) ) {
+ if( i == 1 ) {
+ OSRF_BUFFER_ADD(act, " ");
+ OSRF_BUFFER_ADD(act, str);
+ } else {
+ OSRF_BUFFER_ADD(act, ", ");
+ OSRF_BUFFER_ADD(act, str);
+ }
}
}
diff --git a/src/libopensrf/osrf_application.c b/src/libopensrf/osrf_application.c
index 57a372b..f7be6a6 100644
--- a/src/libopensrf/osrf_application.c
+++ b/src/libopensrf/osrf_application.c
@@ -1035,11 +1035,30 @@ int osrfMethodVerifyContext( osrfMethodContext* ctx )
// Log the call, with the method and parameters
char* params_str = jsonObjectToJSON( ctx->params );
if( params_str ) {
- // params_str will at minimum be "[]"
- params_str[strlen(params_str) - 1] = '\0'; // drop the trailing ']'
- osrfLogInfo( OSRF_LOG_MARK, "CALL: %s %s %s",
- ctx->session->remote_service, ctx->method->name, params_str + 1);
+ // params_str will at minimum be "[]"
+ int i = 0;
+ const char* str;
+ char* method = ctx->method->name;
+ int redact_params = 0;
+ while( (str = osrfStringArrayGetString(log_protect_arr, i++)) ) {
+ //osrfLogInternal(OSRF_LOG_MARK, "Checking for log protection [%s]", str);
+ if(!strncmp(method, str, strlen(str))) {
+ redact_params = 1;
+ break;
+ }
+ }
+
+ char* params_logged;
+ if(redact_params) {
+ params_logged = strdup("**PARAMS REDACTED**");
+ } else {
+ params_str[strlen(params_str) - 1] = '\0'; // drop the trailing ']'
+ params_logged = strdup(params_str + 1);
+ }
free( params_str );
+ osrfLogInfo( OSRF_LOG_MARK, "CALL: %s %s %s",
+ ctx->session->remote_service, ctx->method->name, params_logged);
+ free( params_logged );
}
return 0;
}
diff --git a/src/libopensrf/osrf_system.c b/src/libopensrf/osrf_system.c
index 05244a9..f6d338c 100644
--- a/src/libopensrf/osrf_system.c
+++ b/src/libopensrf/osrf_system.c
@@ -26,6 +26,7 @@
static void report_child_status( pid_t pid, int status );
struct child_node;
typedef struct child_node ChildNode;
+osrfStringArray* log_protect_arr = NULL;
/**
@brief Represents a child process.
@@ -540,6 +541,11 @@ int osrfSystemBootstrapClientResc( const char* config_file,
osrfConfigSetDefaultConfig(cfg);
else
return 0; /* Can't load configuration? Bail out */
+
+ // fetch list of configured log redaction marker strings
+ log_protect_arr = osrfNewStringArray(8);
+ osrfConfig* cfg_shared = osrfConfigInit(config_file, "shared");
+ osrfConfigGetValueList( cfg_shared, log_protect_arr, "/log_protect/match_string" );
}
char* log_file = osrfConfigGetValue( NULL, "/logfile");
-----------------------------------------------------------------------
Summary of changes:
examples/opensrf_core.xml.example | 12 +++++++
include/opensrf/osrf_application.h | 39 ++--------------------
include/opensrf/osrf_system.h | 2 +
src/gateway/osrf_http_translator.c | 35 ++++++++++++++------
src/gateway/osrf_json_gateway.c | 29 ++++++++++++----
src/libopensrf/osrf_application.c | 27 +++++++++++++--
src/libopensrf/osrf_system.c | 6 +++
src/perl/lib/OpenSRF/Application.pm | 24 +++++++++++++-
src/perl/lib/OpenSRF/System.pm | 9 +++++
src/perl/lib/OpenSRF/Utils/Config.pm | 60 +++++++++++++++++++++++++++-------
10 files changed, 173 insertions(+), 70 deletions(-)
hooks/post-receive
--
OpenSRF
More information about the opensrf-commits
mailing list