From f9ac1e5138f2cf2b4e9af40d92cd55e6a17f1c7b Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Tue, 16 Jun 2015 16:12:42 -0700 Subject: [PATCH 1/2] Ticket #47669 - Retro Changelog Plugin accepts invalid value in nsslapd-changelogmaxage attribute Description: Converted parse_duration to the private api and applied the following config attributes. cn=changelog5,cn=config nsslapd-changelogmaxage nsslapd-changelogtrim-interval nsslapd-changelogcompactdb-interval cn=Retro Changelog Plugin,cn=plugins,cn=config nsslapd-changelogmaxage These config attributes take unsigned integer as well as digit[SsMmHhDdWw]. Other input are logged in the error log and the invalid value is ignored. --- ldap/schema/01core389.ldif | 4 +- ldap/servers/plugins/replication/cl4_init.c | 2 +- ldap/servers/plugins/replication/cl5_api.c | 2 +- ldap/servers/plugins/replication/cl5_config.c | 160 +++++++++++--------------- ldap/servers/plugins/replication/replutil.c | 44 ------- ldap/servers/plugins/retrocl/retrocl_trim.c | 82 ++----------- ldap/servers/slapd/slapi-private.h | 11 ++ ldap/servers/slapd/time.c | 96 +++++++++++++--- ldap/servers/slapd/value.c | 2 +- 9 files changed, 179 insertions(+), 224 deletions(-) diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif index ffd8710..d2fb6dc 100644 --- a/ldap/schema/01core389.ldif +++ b/ldap/schema/01core389.ldif @@ -304,8 +304,8 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2308 NAME 'nstombstonecsn' DESC 'Netscap attributeTypes: ( 2.16.840.1.113730.3.1.2309 NAME 'nsds5ReplicaPreciseTombstonePurging' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2310 NAME 'nsds5ReplicaFlowControlWindow' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2311 NAME 'nsds5ReplicaFlowControlPause' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) -attributeTypes: ( 2.16.840.1.113730.3.1.2313 NAME 'nsslapd-changelogtrim-interval' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) -attributeTypes: ( 2.16.840.1.113730.3.1.2314 NAME 'nsslapd-changelogcompactdb-interval' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2313 NAME 'nsslapd-changelogtrim-interval' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2314 NAME 'nsslapd-changelogcompactdb-interval' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2315 NAME 'nsDS5ReplicaWaitForAsyncResults' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) # # objectclasses diff --git a/ldap/servers/plugins/replication/cl4_init.c b/ldap/servers/plugins/replication/cl4_init.c index 87cd147..77c93d5 100644 --- a/ldap/servers/plugins/replication/cl4_init.c +++ b/ldap/servers/plugins/replication/cl4_init.c @@ -225,7 +225,7 @@ static void changelog4_init_trimming () { char *cl_maxage = changelog4_get_maxage (); unsigned long cl_maxentries = changelog4_get_maxentries (); - time_t ageval = age_str2time (cl_maxage); + time_t ageval = slapi_parse_duration(cl_maxage); slapi_ch_free ((void **)&cl_maxage); diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c index 0d9b8d9..5af57c8 100644 --- a/ldap/servers/plugins/replication/cl5_api.c +++ b/ldap/servers/plugins/replication/cl5_api.c @@ -1203,7 +1203,7 @@ cl5ConfigTrimming (int maxEntries, const char *maxAge, int compactInterval, int /* don't ignore this argument */ if (strcmp (maxAge, CL5_STR_IGNORE) != 0) { - s_cl5Desc.dbTrim.maxAge = age_str2time (maxAge); + s_cl5Desc.dbTrim.maxAge = slapi_parse_duration(maxAge); } } else diff --git a/ldap/servers/plugins/replication/cl5_config.c b/ldap/servers/plugins/replication/cl5_config.c index b81fe24..5b83a59 100644 --- a/ldap/servers/plugins/replication/cl5_config.c +++ b/ldap/servers/plugins/replication/cl5_config.c @@ -285,8 +285,8 @@ static int changelog5_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, int *returncode, char *returntext, void *arg) { - int rc= 0; - LDAPMod **mods; + int rc= 0; + LDAPMod **mods; int i; changelog5Config config; changelog5Config * originalConfig = NULL; @@ -392,68 +392,52 @@ changelog5_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr } } else if ( strcasecmp ( config_attr, CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE ) == 0 ) - { - slapi_ch_free_string(&config.maxAge); - config.maxAge = slapi_ch_strdup(config_attr_value); + { + if (slapi_is_duration_valid(config_attr_value)) { + slapi_ch_free_string(&config.maxAge); + config.maxAge = slapi_ch_strdup(config_attr_value); + } else { + if (returntext) { + PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE, + "%s: invalid value \"%s\", %s must range from 0 to %lld or digit[sSmMhHdD]", + CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE, config_attr_value?config_attr_value:"null", + CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE, + (long long int)LONG_MAX ); + } + *returncode = LDAP_UNWILLING_TO_PERFORM; + goto done; + } } else if ( strcasecmp ( config_attr, CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE ) == 0 ) - { - char *endp = NULL; - long value; - - errno = 0; - - if (config_attr_value && config_attr_value[0] != '\0') - { - value = strtol(config_attr_value, &endp, 10); - - if (*endp != '\0' || errno == ERANGE || value < 0 ) { - if (returntext) - { - PR_snprintf ( returntext, SLAPI_DSE_RETURNTEXT_SIZE, - "%s: invalid value \"%s\", %s must range from 0 to %lld", - CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, config_attr_value, - CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, - (long long int)LONG_MAX ); - *returncode = LDAP_UNWILLING_TO_PERFORM; - goto done; - } - config.compactInterval = value; + { + if (slapi_is_duration_valid(config_attr_value)) { + config.compactInterval = (long)slapi_parse_duration(config_attr_value); + } else { + if (returntext) { + PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE, + "%s: invalid value \"%s\", %s must range from 0 to %lld or digit[sSmMhHdD]", + CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, config_attr_value, + CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, + (long long int)LONG_MAX); } - } - else - { - config.compactInterval = 0; + *returncode = LDAP_UNWILLING_TO_PERFORM; + goto done; } } else if ( strcasecmp ( config_attr, CONFIG_CHANGELOG_TRIM_ATTRIBUTE ) == 0 ) { - char *endp = NULL; - long value; - - errno = 0; - - if (config_attr_value && config_attr_value[0] != '\0') - { - value = strtol(config_attr_value, &endp, 10); - - if (*endp != '\0' || errno == ERANGE || value < 0 ) { - if (returntext) - { - PR_snprintf ( returntext, SLAPI_DSE_RETURNTEXT_SIZE, - "%s: invalid value \"%s\", %s must range from 0 to %lld", - CONFIG_CHANGELOG_TRIM_ATTRIBUTE, config_attr_value, - CONFIG_CHANGELOG_TRIM_ATTRIBUTE, - (long long int)LONG_MAX ); - *returncode = LDAP_UNWILLING_TO_PERFORM; - goto done; - } - config.trimInterval = value; + if (slapi_is_duration_valid(config_attr_value)) { + config.trimInterval = (long)slapi_parse_duration(config_attr_value); + } else { + if (returntext) { + PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE, + "%s: invalid value \"%s\", %s must range from 0 to %lld or digit[sSmMhHdD]", + CONFIG_CHANGELOG_TRIM_ATTRIBUTE, config_attr_value, + CONFIG_CHANGELOG_TRIM_ATTRIBUTE, + (long long int)LONG_MAX ); } - } - else - { - config.trimInterval = CHANGELOGDB_TRIM_INTERVAL; + *returncode = LDAP_UNWILLING_TO_PERFORM; + goto done; } } else if ( strcasecmp ( config_attr, CONFIG_CHANGELOG_SYMMETRIC_KEY ) == 0 ) @@ -795,23 +779,12 @@ static void changelog5_extract_config(Slapi_Entry* entry, changelog5Config *conf arg = slapi_entry_attr_get_charptr(entry,CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE); if (arg) { - char *endp = NULL; - long value; - - errno = 0; - - if (arg) - { - value = strtol(arg, &endp, 10); - if (*endp != '\0' || errno == ERANGE || value < 0 ) { - slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, - "changelog5_extract_config: %s: invalid value \"%s\", using default value (%d)\n", - CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, arg, - CHANGELOGDB_COMPACT_INTERVAL ); - config->compactInterval = CHANGELOGDB_COMPACT_INTERVAL; - } else { - config->compactInterval = value; - } + if (slapi_is_duration_valid(arg)) { + config->compactInterval = (long)slapi_parse_duration(arg); + } else { + slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, + "changelog5_extract_config: %s: invalid value \"%s\", ignoring the change.\n", + CONFIG_CHANGELOG_COMPACTDB_ATTRIBUTE, arg); } slapi_ch_free_string(&arg); } @@ -819,26 +792,17 @@ static void changelog5_extract_config(Slapi_Entry* entry, changelog5Config *conf { config->compactInterval = CHANGELOGDB_COMPACT_INTERVAL; } + arg = slapi_entry_attr_get_charptr(entry, CONFIG_CHANGELOG_TRIM_ATTRIBUTE); if (arg) { - char *endp = NULL; - long value; - - errno = 0; - - if (arg) - { - value = strtol(arg, &endp, 10); - if (*endp != '\0' || errno == ERANGE || value < 0 ) { - slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, - "changelog5_extract_config: %s: invalid value \"%s\", using default value (%d)\n", - CONFIG_CHANGELOG_TRIM_ATTRIBUTE, arg, - CHANGELOGDB_TRIM_INTERVAL ); - config->trimInterval = CHANGELOGDB_TRIM_INTERVAL; - } else { - config->trimInterval = value; - } + if (slapi_is_duration_valid(arg)) { + config->trimInterval = (long)slapi_parse_duration(arg); + } else { + slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, + "changelog5_extract_config: %s: invalid value \"%s\", ignoring the change.\n", + CONFIG_CHANGELOG_TRIM_ATTRIBUTE, arg); + config->trimInterval = CHANGELOGDB_TRIM_INTERVAL; } slapi_ch_free_string(&arg); } @@ -846,7 +810,21 @@ static void changelog5_extract_config(Slapi_Entry* entry, changelog5Config *conf { config->trimInterval = CHANGELOGDB_TRIM_INTERVAL; } - config->maxAge = slapi_entry_attr_get_charptr(entry,CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE); + + arg = slapi_entry_attr_get_charptr(entry, CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE); + if (arg) { + if (slapi_is_duration_valid(arg)) { + config->maxAge = arg; + } else { + slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, + "changelog5_extract_config: %s: invalid value \"%s\", ignoring the change.\n", + CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE, arg); + slapi_ch_free_string(&arg); + config->maxAge = slapi_ch_strdup(CL5_STR_IGNORE); + } + } else { + config->maxAge = slapi_ch_strdup(CL5_STR_IGNORE); + } /* * Read the Changelog Internal Configuration Parameters for the Changelog DB diff --git a/ldap/servers/plugins/replication/replutil.c b/ldap/servers/plugins/replication/replutil.c index 47c08aa..e26c4c5 100644 --- a/ldap/servers/plugins/replication/replutil.c +++ b/ldap/servers/plugins/replication/replutil.c @@ -249,50 +249,6 @@ error: return return_value; } -/* convert time from string like 1h (1 hour) to corresponding time in seconds */ -time_t -age_str2time (const char *age) -{ - char *maxage; - char unit; - time_t ageval; - - if (age == NULL || age[0] == '\0' || strcmp (age, "0") == 0) - { - return 0; - } - - maxage = slapi_ch_strdup ( age ); - unit = maxage[ strlen( maxage ) - 1 ]; - maxage[ strlen( maxage ) - 1 ] = '\0'; - ageval = strntoul( maxage, strlen( maxage ), 10 ); - slapi_ch_free_string(&maxage); - switch ( unit ) - { - case 's': - break; - case 'm': - ageval *= 60; - break; - case 'h': - ageval *= ( 60 * 60 ); - break; - case 'd': - ageval *= ( 24 * 60 * 60 ); - break; - case 'w': - ageval *= ( 7 * 24 * 60 * 60 ); - break; - default: - slapi_log_error( SLAPI_LOG_FATAL, repl_plugin_name, - "age_str2time: unknown unit \"%c\" " - "for maxiumum changelog age\n", unit ); - ageval = 0; - } - - return ageval; -} - const char* changeType2Str (int type) { diff --git a/ldap/servers/plugins/retrocl/retrocl_trim.c b/ldap/servers/plugins/retrocl/retrocl_trim.c index 098fc09..777f763 100644 --- a/ldap/servers/plugins/retrocl/retrocl_trim.c +++ b/ldap/servers/plugins/retrocl/retrocl_trim.c @@ -427,69 +427,6 @@ void retrocl_housekeeping ( time_t cur_time, void *noarg ) PR_Unlock( ts.ts_s_trim_mutex ); } - -/* - * Function: age_str2time - * - * Returns: time_t - * - * Arguments: string representation of age (digits and unit s,m,h,d or w) - * - * Description: - * convert time from string like 1h (1 hour) to corresponding time in seconds - * - */ - -static time_t -age_str2time (const char *age) -{ - char *maxage; - char unit; - time_t ageval; - - if (age == NULL || age[0] == '\0' || strcmp (age, "0") == 0) { - return 0; - } - - maxage = slapi_ch_strdup ( age ); - if (!maxage) { - slapi_log_error( SLAPI_LOG_PLUGIN, "retrocl", - "age_str2time: Out of memory\n" ); - ageval = -1; - goto done; - } - - unit = maxage[ strlen( maxage ) - 1 ]; - maxage[ strlen( maxage ) - 1 ] = '\0'; - ageval = strntoul( maxage, strlen( maxage ), 10 ); - switch ( unit ) { - case 's': - break; - case 'm': - ageval *= 60; - break; - case 'h': - ageval *= ( 60 * 60 ); - break; - case 'd': - ageval *= ( 24 * 60 * 60 ); - break; - case 'w': - ageval *= ( 7 * 24 * 60 * 60 ); - break; - default: - slapi_log_error( SLAPI_LOG_FATAL, "retrocl", - "age_str2time: unknown unit \"%c\" " - "for maxiumum changelog age\n", unit ); - ageval = 0; - } -done: - if ( maxage) { - slapi_ch_free ( (void **) &maxage ); - } - return ageval; -} - /* * Function: retrocl_init_trimming * @@ -508,14 +445,19 @@ void retrocl_init_trimming (void) const char *cl_trim_interval; cl_maxage = retrocl_get_config_str(CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE); - - if (cl_maxage == NULL) { - LDAPDebug0Args(LDAP_DEBUG_TRACE,"No maxage, not trimming retro changelog.\n"); - return; + if (cl_maxage) { + if (slapi_is_duration_valid(cl_maxage)) { + ageval = slapi_parse_duration(cl_maxage); + slapi_ch_free_string((char **)&cl_maxage); + } else { + slapi_log_error(SLAPI_LOG_FATAL, RETROCL_PLUGIN_NAME, + "retrocl_init_trimming: ignoring invalid %s value %s; " + "not trimming retro changelog.\n", + CONFIG_CHANGELOG_MAXAGE_ATTRIBUTE, cl_maxage); + return; + } } - ageval = age_str2time (cl_maxage); - slapi_ch_free_string((char **)&cl_maxage); - + cl_trim_interval = retrocl_get_config_str(CONFIG_CHANGELOG_TRIM_INTERVAL); if (cl_trim_interval) { trim_interval = strtol(cl_trim_interval, (char **)NULL, 10); diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index c663267..994f697 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -1353,6 +1353,17 @@ char *ldaputil_get_saslpath(); */ char *slapi_getSSLVersion_str(PRUint16 vnum, char *buf, size_t bufsize); +/* + * time.c + * + * Return Value: + * Success: duration in seconds + * Failure: -1 + */ +time_t slapi_parse_duration(const char *value); +int slapi_is_duration_valid(const char *value); + + #ifdef __cplusplus } #endif diff --git a/ldap/servers/slapd/time.c b/ldap/servers/slapd/time.c index 621847c..f7a4d46 100644 --- a/ldap/servers/slapd/time.c +++ b/ldap/servers/slapd/time.c @@ -394,38 +394,106 @@ parse_duration(char *value) } input = slapi_ch_strdup(value); endp = input + strlen(input) - 1; - while ((' ' == *endp || '\t' == *endp) && endp >= input) { + while ((' ' == *endp || '\t' == *endp) && endp > input) { endp--; } if ((endp == input) && !isdigit(*input)) { goto bail; } - if ('d' == *endp || 'D' == *endp) { - times = 60 * 60 * 24; - *endp = '\0'; - } else if ('h' == *endp || 'H' == *endp) { - times = 60 * 60; - *endp = '\0'; - } else if ('m' == *endp || 'M' == *endp) { - times = 60; - *endp = '\0'; - } else if ('s' == *endp || 'S' == *endp) { + switch ( *endp ) { + case 'w': + case 'W': + times = 60 * 60 * 24 * 7; + *endp = '\0'; + break; + case 'd': + case 'D': + times = 60 * 60 * 24; + *endp = '\0'; + break; + case 'h': + case 'H': + times = 60 * 60; + *endp = '\0'; + break; + case 'm': + case 'M': + times = 60; + *endp = '\0'; + break; + case 's': + case 'S': + times = 1; + *endp = '\0'; + break; + default: + if (isdigit(*endp)) { times = 1; - *endp = '\0'; + break; + } else { + goto bail; + } } - duration = strtol(input, &endp, 10); if ( *endp != '\0' || errno == ERANGE ) { duration = -1; goto bail; } duration *= times; - bail: + if (duration == -1) { + LDAPDebug1Arg(LDAP_DEBUG_ANY, "slapi_parse_duration: invalid duration (%s)\n", value?value:"null"); + } slapi_ch_free_string(&input); return duration; } +time_t +slapi_parse_duration(const char *value) +{ + return (time_t)parse_duration((char *)value); +} + +static int +is_valid_duration_unit(const char value) +{ + int rc = 0; + switch (value) { + case 'w': + case 'W': + case 'd': + case 'D': + case 'h': + case 'H': + case 'm': + case 'M': + case 's': + case 'S': + rc = 1; + break; + } + return rc; +} + +int +slapi_is_duration_valid(const char *value) +{ + int rc = 1; /* valid */ + const char *p = value; + if (p && *p && isdigit(*p)) { /* 1st character must be digit */ + for (++p; p && *p; p++) { + if (!isdigit(*p) && !is_valid_duration_unit(*p)) { + rc = 0; + goto bail; + } + } + } else { + rc = 0; + } +bail: + return rc; +} + /* * caller is responsible to free the returned string */ diff --git a/ldap/servers/slapd/value.c b/ldap/servers/slapd/value.c index 7bd11be..6776064 100644 --- a/ldap/servers/slapd/value.c +++ b/ldap/servers/slapd/value.c @@ -532,7 +532,7 @@ slapi_value_get_timelong(const Slapi_Value *value) p = slapi_ch_malloc(value->bv.bv_len + 1); memcpy (p, value->bv.bv_val, value->bv.bv_len); p [value->bv.bv_len] = '\0'; - r = (long)parse_duration(p); + r = parse_duration(p); slapi_ch_free((void **)&p); } return r; -- 1.9.3