From: Alberto Leiva Popper Date: Thu, 28 Feb 2019 23:19:48 +0000 (-0600) Subject: Big fat review of the RSYNC module X-Git-Tag: v0.0.2~78 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3996c37a15e8feed357c1b202b97cac254ddb59a;p=thirdparty%2FFORT-validator.git Big fat review of the RSYNC module - The TOML reader now expects correct TOML syntax, according to toml99 - The RSYNC command and its arguments are now configurable - Instead of an enable-disable RSYNC switch, we now have a "synchronization strategy". (Needed to optimize RSYNC operations according to user needs.) - The RSYNC command is now executed via execvp(3) instead of system(3), to increase security. --- diff --git a/.gitignore b/.gitignore index 46ddebdf..6b2a00f6 100644 --- a/.gitignore +++ b/.gitignore @@ -92,6 +92,7 @@ test-driver # Temporal files *~ +tmp/ # Unwanted manure shat by imbecile OSs .DS_Store* diff --git a/deconf.sh b/deconf.sh index 5bfc806f..cc40fc03 100755 --- a/deconf.sh +++ b/deconf.sh @@ -3,4 +3,5 @@ git clean -dfx \ -e .project \ -e .settings \ -e .metadata \ - -e Debug/ + -e Debug/ \ + -e tmp diff --git a/src/Makefile.am b/src/Makefile.am index 299bf18a..a9bb0410 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -11,7 +11,7 @@ rpki_validator_SOURCES += address.h address.c rpki_validator_SOURCES += algorithm.h algorithm.c rpki_validator_SOURCES += array_list.h rpki_validator_SOURCES += certificate_refs.h certificate_refs.c -rpki_validator_SOURCES += common.h common.c +rpki_validator_SOURCES += common.h rpki_validator_SOURCES += config.h config.c rpki_validator_SOURCES += debug.h debug.c rpki_validator_SOURCES += extension.h extension.c @@ -24,6 +24,7 @@ rpki_validator_SOURCES += resource.h resource.c rpki_validator_SOURCES += rpp.h rpp.c rpki_validator_SOURCES += sorted_array.h sorted_array.c rpki_validator_SOURCES += state.h state.c +rpki_validator_SOURCES += str.h str.c rpki_validator_SOURCES += thread_var.h thread_var.c rpki_validator_SOURCES += uri.h uri.c rpki_validator_SOURCES += toml_handler.h toml_handler.c diff --git a/src/asn1/content_info.c b/src/asn1/content_info.c index b74cd7b8..9b1f87b2 100644 --- a/src/asn1/content_info.c +++ b/src/asn1/content_info.c @@ -53,7 +53,7 @@ content_info_load(struct rpki_uri const *uri, struct ContentInfo **result) struct file_contents fc; int error; - error = file_load(uri, &fc); + error = file_load(uri->local, &fc); if (error) return error; diff --git a/src/common.c b/src/common.c deleted file mode 100644 index 80e09460..00000000 --- a/src/common.c +++ /dev/null @@ -1,31 +0,0 @@ -#include "common.h" - -#include -#include "log.h" - -/** - * Does not assume that @string is NULL-terminated. - */ -int -string_clone(void const *string, size_t size, char **clone) -{ - char *result; - - result = malloc(size + 1); - if (result == NULL) - return pr_enomem(); - - memcpy(result, string, size); - result[size] = '\0'; - - *clone = result; - return 0; -} - -int -ia5s2string(ASN1_IA5STRING *ia5, char **result) -{ - return (ia5->flags & ASN1_STRING_FLAG_BITS_LEFT) - ? pr_err("CRL URI IA5String has unused bits.") - : string_clone(ia5->data, ia5->length, result); -} diff --git a/src/common.h b/src/common.h index e78d09fd..b56db53d 100644 --- a/src/common.h +++ b/src/common.h @@ -1,9 +1,6 @@ #ifndef SRC_RTR_COMMON_H_ #define SRC_RTR_COMMON_H_ -#include -#include - /* "I think that this is not supposed to be implemented." */ #define ENOTSUPPORTED 3172 /* "I haven't implemented this yet." */ @@ -18,7 +15,4 @@ #define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0])) -int string_clone(void const *, size_t, char **); -int ia5s2string(ASN1_IA5STRING *, char **); - #endif /* SRC_RTR_COMMON_H_ */ diff --git a/src/config.c b/src/config.c index ebad48ec..87a8cd87 100644 --- a/src/config.c +++ b/src/config.c @@ -7,22 +7,37 @@ #include #include +#include "common.h" #include "log.h" #include "toml_handler.h" +/** + * Please note that this is actually two `for`s stacked together, so don't use + * `break` nor `continue` to get out. + */ #define FOREACH_OPTION(groups, grp, opt, type) \ for (grp = groups; grp->name != NULL; grp++) \ for (opt = grp->options; opt->id != 0; opt++) \ if ((opt->availability == 0) || \ (opt->availability & type)) +/** + * To add a member to this structure, + * + * 1. Add it. + * 2. Add its metadata somewhere in @groups. + * 3. Add default value to set_default_values(). + * 4. Create the getter. + * + * Assuming you don't need to create a data type, that should be all. + */ struct rpki_config { /** TAL file name/location. */ - char const *tal; + char *tal; /** Path of our local clone of the repository */ - char const *local_repository; - /** Disable rsync downloads? */ - bool disable_rsync; + char *local_repository; + /** Synchronization (currently only RSYNC) download strategy. */ + enum sync_strategy sync_strategy; /** * Shuffle uris in tal? * (https://tools.ietf.org/html/rfc7730#section-3, last paragraphs) @@ -35,25 +50,64 @@ struct rpki_config { unsigned int maximum_certificate_depth; /** Print ANSI color codes? */ bool color_output; -}; + struct { + char *program; + struct string_array args; + } rsync; +}; -static void print_usage(FILE *stream); -void print_bool(struct group_fields const *, struct option_field const *, - void *); -void print_u_int(struct group_fields const *, struct option_field const *, - void *); -void print_string(struct group_fields const *, struct option_field const *, - void *); -static int parse_bool(struct option_field const *, char const *, void *); -static int parse_u_int(struct option_field const *, char const *, void *); -static int parse_string(struct option_field const *, char const *, void *); -static int handle_help(struct option_field const *, char *); -static int handle_usage(struct option_field const *, char *); -static int handle_version(struct option_field const *, char *); -static int handle_toml(struct option_field const *, char *); -static void free_string(void *); +static void print_usage(FILE *, bool); + +#define DECLARE_PRINT_FN(name) \ + void name( \ + struct group_fields const *, \ + struct option_field const *, \ + void * \ + ) +DECLARE_PRINT_FN(print_bool); +DECLARE_PRINT_FN(print_u_int); +DECLARE_PRINT_FN(print_string); +DECLARE_PRINT_FN(print_string_array); +DECLARE_PRINT_FN(print_sync_strategy); + +#define DECLARE_PARSE_ARGV_FN(name) \ + static int name( \ + struct option_field const *, \ + char const *, \ + void * \ + ) +DECLARE_PARSE_ARGV_FN(parse_argv_bool); +DECLARE_PARSE_ARGV_FN(parse_argv_u_int); +DECLARE_PARSE_ARGV_FN(parse_argv_string); +DECLARE_PARSE_ARGV_FN(parse_argv_sync_strategy); + +#define DECLARE_PARSE_TOML_FN(name) \ + static int name( \ + struct option_field const *, \ + struct toml_table_t *, \ + void * \ + ) +DECLARE_PARSE_TOML_FN(parse_toml_bool); +DECLARE_PARSE_TOML_FN(parse_toml_u_int); +DECLARE_PARSE_TOML_FN(parse_toml_string); +DECLARE_PARSE_TOML_FN(parse_toml_sync_strategy); +DECLARE_PARSE_TOML_FN(parse_toml_string_array); + +#define DECLARE_HANDLE_FN(name) \ + static int name( \ + struct option_field const *, \ + char * \ + ) +DECLARE_HANDLE_FN(handle_help); +DECLARE_HANDLE_FN(handle_usage); +DECLARE_HANDLE_FN(handle_version); +DECLARE_HANDLE_FN(handle_toml); + +#define DECLARE_FREE_FN(name) static void name(void *) +DECLARE_FREE_FN(free_string); +DECLARE_FREE_FN(free_string_array); static char const *program_name; static struct rpki_config rpki_config; @@ -62,7 +116,8 @@ static const struct global_type gt_bool = { .has_arg = no_argument, .size = sizeof(bool), .print = print_bool, - .parse = parse_bool, + .parse.argv = parse_argv_bool, + .parse.toml = parse_toml_bool, .arg_doc = "true|false", }; @@ -70,7 +125,8 @@ static const struct global_type gt_u_int = { .has_arg = required_argument, .size = sizeof(unsigned int), .print = print_u_int, - .parse = parse_u_int, + .parse.argv = parse_argv_u_int, + .parse.toml = parse_toml_u_int, .arg_doc = "", }; @@ -78,11 +134,30 @@ static const struct global_type gt_string = { .has_arg = required_argument, .size = sizeof(char *), .print = print_string, - .parse = parse_string, + .parse.argv = parse_argv_string, + .parse.toml = parse_toml_string, .free = free_string, .arg_doc = "", }; +static const struct global_type gt_string_array = { + .has_arg = required_argument, + .size = sizeof(char *const *), + .print = print_string_array, + .parse.toml = parse_toml_string_array, + .free = free_string_array, + .arg_doc = "", +}; + +static const struct global_type gt_sync_strategy = { + .has_arg = required_argument, + .size = sizeof(enum sync_strategy), + .print = print_sync_strategy, + .parse.argv = parse_argv_sync_strategy, + .parse.toml = parse_toml_sync_strategy, + .arg_doc = SYNC_VALUE_OFF "|" SYNC_VALUE_STRICT "|" SYNC_VALUE_ROOT, +}; + /** * An option that takes no arguments, is not correlated to any rpki_config * fields, and is entirely managed by its handler function. @@ -130,10 +205,10 @@ static const struct option_field global_fields[] = { .arg_doc = "", }, { .id = 1001, - .name = "disable-rsync", - .type = >_bool, - .offset = offsetof(struct rpki_config, disable_rsync), - .doc = "Enable or disable rsync downloads.", + .name = "sync-strategy", + .type = >_sync_strategy, + .offset = offsetof(struct rpki_config, sync_strategy), + .doc = "RSYNC download strategy.", }, { .id = 'c', .name = "color-output", @@ -176,6 +251,26 @@ static const struct option_field tal_fields[] = { { 0 }, }; +static const struct option_field rsync_fields[] = { + { + .id = 3000, + .name = "program", + .type = >_string, + .offset = offsetof(struct rpki_config, rsync.program), + .doc = "Name of the program needed to execute an RSYNC.", + .arg_doc = "", + .availability = AVAILABILITY_TOML, + }, { + .id = 3001, + .name = "arguments", + .type = >_string_array, + .offset = offsetof(struct rpki_config, rsync.args), + .doc = "Arguments to send to the RSYNC program call.", + .availability = AVAILABILITY_TOML, + }, + { 0 }, +}; + static const struct group_fields groups[] = { { .name = "root", @@ -183,6 +278,9 @@ static const struct group_fields groups[] = { }, { .name = "tal", .options = tal_fields, + }, { + .name = "rsync", + .options = rsync_fields, }, { NULL }, }; @@ -198,7 +296,7 @@ is_rpki_config_field(struct option_field const *field) return field->handler == NULL; } -static void * +void * get_rpki_config_field(struct option_field const *field) { return ((unsigned char *) &rpki_config) + field->offset; @@ -228,8 +326,48 @@ print_string(struct group_fields const *group, struct option_field const *field, pr_info("%s.%s: %s", group->name, field->name, *((char **) value)); } +void +print_string_array(struct group_fields const *group, + struct option_field const *field, void *_value) +{ + struct string_array *value = _value; + size_t i; + + pr_info("%s.%s:", group->name, field->name); + pr_indent_add(); + + if (value->length == 0) + pr_info(""); + else for (i = 0; i < value->length; i++) + pr_info("%s", value->array[i]); + + pr_indent_rm(); +} + +void +print_sync_strategy(struct group_fields const *group, + struct option_field const *field, void *value) +{ + enum sync_strategy *strategy = value; + char const *str = ""; + + switch (*strategy) { + case SYNC_OFF: + str = SYNC_VALUE_OFF; + break; + case SYNC_STRICT: + str = SYNC_VALUE_STRICT; + break; + case SYNC_ROOT: + str = SYNC_VALUE_ROOT; + break; + } + + pr_info("%s.%s: %s", group->name, field->name, str); +} + static int -parse_bool(struct option_field const *field, char const *str, void *result) +parse_argv_bool(struct option_field const *field, char const *str, void *result) { bool *value = result; @@ -252,7 +390,7 @@ parse_bool(struct option_field const *field, char const *str, void *result) } static int -parse_u_int(struct option_field const *field, char const *str, void *_result) +parse_argv_u_int(struct option_field const *field, char const *str, void *_result) { unsigned long parsed; int *result; @@ -278,10 +416,13 @@ parse_u_int(struct option_field const *field, char const *str, void *_result) } static int -parse_string(struct option_field const *field, char const *str, void *_result) +parse_argv_string(struct option_field const *field, char const *str, void *_result) { char **result = _result; + /* Remove the previous value (usually the default). */ + field->type->free(result); + if (field->type->has_arg != required_argument || str == NULL) { return pr_err("String options ('%s' in this case) require an argument.", field->name); @@ -292,17 +433,161 @@ parse_string(struct option_field const *field, char const *str, void *_result) return ((*result) != NULL) ? 0 : pr_enomem(); } +static int +parse_argv_sync_strategy(struct option_field const *field, char const *str, + void *_result) +{ + enum sync_strategy *result = _result; + + if (strcmp(str, SYNC_VALUE_OFF) == 0) + *result = SYNC_OFF; + else if (strcmp(str, SYNC_VALUE_STRICT) == 0) + *result = SYNC_STRICT; + else if (strcmp(str, SYNC_VALUE_ROOT) == 0) + *result = SYNC_ROOT; + else + return pr_err("Unknown synchronization strategy: '%s'", str); + + return 0; +} + +static int +parse_toml_bool(struct option_field const *opt, struct toml_table_t *toml, + void *_result) +{ + const char *raw; + int value; + bool *result; + + raw = toml_raw_in(toml, opt->name); + if (raw == NULL) + return pr_err("TOML boolean '%s' was not found.", opt->name); + if (toml_rtob(raw, &value) == -1) + return pr_err("Cannot parse '%s' as a boolean.", raw); + + result = _result; + *result = value; + return 0; +} + +static int +parse_toml_u_int(struct option_field const *opt, struct toml_table_t *toml, + void *_result) +{ + const char *raw; + int64_t value; + unsigned int *result; + + raw = toml_raw_in(toml, opt->name); + if (raw == NULL) + return pr_err("TOML integer '%s' was not found.", opt->name); + if (toml_rtoi(raw, &value) == -1) + return pr_err("Cannot parse '%s' as an integer.", raw); + + if (value < opt->min || opt->max < value) { + return pr_err("Integer '%s' is out of range (%u-%u).", + opt->name, opt->min, opt->max); + } + + result = _result; + *result = value; + return 0; +} + +static int +parse_toml_string(struct option_field const *opt, struct toml_table_t *toml, + void *_result) +{ + const char *raw; + char *value; + char **result; + + /* Remove the previous value (usually the default). */ + opt->type->free(_result); + + raw = toml_raw_in(toml, opt->name); + if (raw == NULL) + return pr_err("TOML string '%s' was not found.", opt->name); + if (toml_rtos(raw, &value) == -1) + return pr_err("Cannot parse '%s' as a string.", raw); + + result = _result; + *result = value; + return 0; +} + +static int +parse_toml_sync_strategy(struct option_field const *opt, + struct toml_table_t *toml, void *_result) +{ + int error; + char *string; + + error = parse_toml_string(opt, toml, &string); + if (error) + return error; + + error = parse_argv_sync_strategy(opt, string, _result); + + free(string); + return error; +} + +static int +parse_toml_string_array(struct option_field const *opt, + struct toml_table_t *toml, void *_result) +{ + toml_array_t *array; + int array_len; + int i; + const char *raw; + struct string_array *result = _result; + int error; + + /* Remove the previous value (usually the default). */ + opt->type->free(_result); + + array = toml_array_in(toml, opt->name); + if (array == NULL) + return pr_err("TOML array '%s' was not found.", opt->name); + array_len = toml_array_nelem(array); + + result->array = malloc(array_len * sizeof(char *)); + if (result->array == NULL) + return pr_enomem(); + result->length = array_len; + + for (i = 0; i < array_len; i++) { + raw = toml_raw_at(array, i); + if (raw == NULL) { + error = pr_crit("Array index %d is NULL.", i); + goto fail; + } + if (toml_rtos(raw, &result->array[i]) == -1) { + error = pr_err("Cannot parse '%s' as a string.", raw); + goto fail; + } + } + + return 0; + +fail: + free(result->array); + result->length = 0; + return error; +} + static int handle_help(struct option_field const *field, char *arg) { - print_usage(stdout); + print_usage(stdout, true); exit(0); } static int handle_usage(struct option_field const *field, char *arg) { - print_usage(stdout); + print_usage(stdout, false); exit(0); } @@ -324,6 +609,20 @@ free_string(void *_string) { char **string = _string; free(*string); + *string = NULL; +} + +static void +free_string_array(void *_array) +{ + struct string_array *array = _array; + size_t i; + + for (i = 0; i < array->length; i++) + free(array->array[i]); + + array->array = NULL; + array->length = 0; } static bool @@ -393,13 +692,6 @@ construct_getopt_options(struct option **_long_opts, char **_short_opts) return 0; } -int -parse_option(struct option_field const *field, char const *str) -{ - return field->type->parse(field, str, - get_rpki_config_field(field)); -} - static void print_config(void) { @@ -420,14 +712,60 @@ print_config(void) static int set_default_values(void) { + static char const *default_rsync_args[] = { + "--recursive", + "--delete", + "--times", + "--contimeout=20", + "$REMOTE", + "$LOCAL", + }; + + size_t i; + + /* + * Values that might need to be freed WILL be freed, so use heap + * duplicates. + */ + rpki_config.tal = NULL; + rpki_config.local_repository = strdup("repository/"); if (rpki_config.local_repository == NULL) return pr_enomem(); - rpki_config.disable_rsync = false; + + rpki_config.sync_strategy = SYNC_STRICT; rpki_config.shuffle_uris = false; rpki_config.maximum_certificate_depth = 32; + rpki_config.color_output = false; + + rpki_config.rsync.program = strdup("rsync"); + if (rpki_config.rsync.program == NULL) + goto revert_repository; + + rpki_config.rsync.args.length = ARRAY_LEN(default_rsync_args); + rpki_config.rsync.args.array = calloc(rpki_config.rsync.args.length, + sizeof(char *)); + if (rpki_config.rsync.args.array == NULL) + goto revert_rsync_program; + + for (i = 0; i < ARRAY_LEN(default_rsync_args); i++) { + rpki_config.rsync.args.array[i] = strdup(default_rsync_args[i]); + if (rpki_config.rsync.args.array[i] == NULL) + goto revert_rsync_args; + } + return 0; + +revert_rsync_args: + for (i = 0; i < ARRAY_LEN(default_rsync_args); i++) + free(rpki_config.rsync.args.array[i]); + free(rpki_config.rsync.args.array); +revert_rsync_program: + free(rpki_config.rsync.program); +revert_repository: + free(rpki_config.local_repository); + return pr_enomem(); } static int @@ -439,7 +777,7 @@ validate_config(void) } static void -print_usage(FILE *stream) +print_usage(FILE *stream, bool print_doc) { struct group_fields const *group; struct option_field const *option; @@ -463,13 +801,15 @@ print_usage(FILE *stream) break; case optional_argument: case required_argument: - if(arg_doc == NULL) - break; - fprintf(stream, "=%s", arg_doc); + if (arg_doc != NULL) + fprintf(stream, "=%s", arg_doc); break; } fprintf(stream, "]\n"); + + if (print_doc) + fprintf(stream, "\t (%s)\n", option->doc); } } @@ -483,7 +823,8 @@ handle_opt(int opt) FOREACH_OPTION(groups, group, option, AVAILABILITY_GETOPT) { if (option->id == opt) { return is_rpki_config_field(option) - ? parse_option(option, optarg) + ? option->type->parse.argv(option, optarg, + get_rpki_config_field(option)) : option->handler(option, optarg); } } @@ -561,10 +902,10 @@ config_get_local_repository(void) return rpki_config.local_repository; } -bool -config_get_enable_rsync(void) +enum sync_strategy +config_get_sync_strategy(void) { - return !rpki_config.disable_rsync; + return rpki_config.sync_strategy; } bool @@ -585,6 +926,19 @@ config_get_color_output(void) return rpki_config.color_output; } + +char * +config_get_rsync_program(void) +{ + return rpki_config.rsync.program; +} + +struct string_array const * +config_get_rsync_args(void) +{ + return &rpki_config.rsync.args; +} + void free_rpki_config(void) { diff --git a/src/config.h b/src/config.h index a0f06a7d..f127798b 100644 --- a/src/config.h +++ b/src/config.h @@ -3,17 +3,85 @@ #include #include +#include +#include +#include + +/** + * Note: The only repository synchronization protocol implemented so far is + * RSYNC. Whenever you see "sync", think "rsync." + */ +enum sync_strategy { + /** + * Synchronization is turned off. + * The validator will work on an already downloaded repository. + */ + SYNC_OFF, + /** + * Strictly correct download strategy. + * + * The validator will sync each repository publication point separately + * as requested by each caRepository contained in the CA certificates' + * SIA extensions. + * + * No risk of downloading unneeded files, but otherwise slow, as every + * different repository publication point requires a separate sync call. + */ + SYNC_STRICT, + /** + * Always download the likely root of the entire repository. + * + * For example, if we get the following caRepositories: + * + * - `rsync://a.b.c/d/e/f/g/h/i` + * - `rsync://a.b.c/d/e/f/g/h/j` + * - `rsync://a.b.c/d/e/f/k` + * + * This strategy will synchronize `rsync://a.b.c/d` while parsing the + * first caRepository, and then skip synchronization during the second + * and third ones. (Because they are already downloaded.) + * + * This strategy risks downloading unneeded files, and even failing due + * to lack of read permissions on stray subdirectories. On the flip + * side, if the repository holds no unnecessary subdirectories, then + * this strategy is the fastest one, since it generally only requires + * one sync call per domain, which often translates into one sync call + * per validation cycle. + * + * Currently, all of the official repositories are actually specifically + * structured to benefit this strategy. + */ + SYNC_ROOT, +}; + +#define SYNC_VALUE_OFF "off" +#define SYNC_VALUE_STRICT "strict" +#define SYNC_VALUE_ROOT "root" struct rpki_config; struct group_fields; struct option_field; -typedef void (*print_function)(struct group_fields const *, - struct option_field const *, void *); -typedef int (*parse_function)(struct option_field const *, char const *, - void *); -typedef int (*handler_function)(struct option_field const *, char *); +typedef void (*print_function)( + struct group_fields const *, + struct option_field const *, + void * +); +typedef int (*argv_parse_function)( + struct option_field const *, + char const *, + void * +); +typedef int (*toml_parse_function)( + struct option_field const *, + struct toml_table_t *, + void * +); +typedef int (*handler_function)( + struct option_field const *, + char * +); struct global_type { /** Same as struct option.has_arg. Mandatory. */ @@ -29,14 +97,30 @@ struct global_type { * Optional. */ print_function print; - /** - * Convers from string to this data type. - * If the option's handler is not NULL, this is optional. - */ - parse_function parse; + + /** If the option's handler is not NULL, this is optional. */ + struct { + /** + * Convers from string to this data type. + * Optional if there are no fields of this type that are read + * from argv. + */ + argv_parse_function argv; + /** + * Converts from a TOML node to this data type. + * Optional if there are no fields of this type that are read + * from TOML files. + */ + toml_parse_function toml; + } parse; + /** * Function that will release this data type. * If the option's handler is not NULL, this is optional. + * + * IMPORTANT: This function might be called twice in succession. + * Therefore, make sure that it nullifies the value, and reacts properly + * when the input is NULL. */ void (*free)(void *); @@ -103,17 +187,25 @@ struct group_fields { struct option_field const *options; }; -int parse_option(struct option_field const *, char const *); +struct string_array { + char **array; + size_t length; +}; + +void *get_rpki_config_field(struct option_field const *); int handle_flags_config(int , char **); void get_group_fields(struct group_fields const **); char const *config_get_tal(void); char const *config_get_local_repository(void); -bool config_get_enable_rsync(void); +enum sync_strategy config_get_sync_strategy(void); bool config_get_shuffle_uris(void); unsigned int config_get_max_cert_depth(void); bool config_get_color_output(void); +char *config_get_rsync_program(void); +struct string_array const *config_get_rsync_args(void); + void free_rpki_config(void); #endif /* SRC_CONFIG_H_ */ diff --git a/src/crypto/hash.c b/src/crypto/hash.c index 1dd6cefe..a61aa174 100644 --- a/src/crypto/hash.c +++ b/src/crypto/hash.c @@ -65,7 +65,7 @@ hash_file(char const *algorithm, struct rpki_uri const *uri, if (error) return error; - error = file_open(uri, &file, &stat); + error = file_open(uri->local, &file, &stat); if (error) return error; diff --git a/src/file.c b/src/file.c index e8abb962..bb85e878 100644 --- a/src/file.c +++ b/src/file.c @@ -5,21 +5,21 @@ #include "log.h" int -file_open(struct rpki_uri const *uri, FILE **result, struct stat *stat) +file_open(char const *file_name, FILE **result, struct stat *stat) { FILE *file; int error; - file = fopen(uri->local, "rb"); + file = fopen(file_name, "rb"); if (file == NULL) - return pr_errno(errno, "Could not open file '%s'", uri->local); + return pr_errno(errno, "Could not open file '%s'", file_name); if (fstat(fileno(file), stat) == -1) { - error = pr_errno(errno, "fstat(%s) failed", uri->local); + error = pr_errno(errno, "fstat(%s) failed", file_name); goto fail; } if (!S_ISREG(stat->st_mode)) { - error = pr_err("%s does not seem to be a file", uri->local); + error = pr_err("%s does not seem to be a file", file_name); goto fail; } @@ -39,14 +39,14 @@ file_close(FILE *file) } int -file_load(struct rpki_uri const *uri, struct file_contents *fc) +file_load(char const *file_name, struct file_contents *fc) { FILE *file; struct stat stat; size_t fread_result; int error; - error = file_open(uri, &file, &stat); + error = file_open(file_name, &file, &stat); if (error) return error; diff --git a/src/file.h b/src/file.h index a420c8a5..c494a65b 100644 --- a/src/file.h +++ b/src/file.h @@ -4,7 +4,6 @@ #include #include #include -#include "uri.h" /* * The entire contents of the file, loaded into a buffer. @@ -16,10 +15,10 @@ struct file_contents { size_t buffer_size; }; -int file_open(struct rpki_uri const *, FILE **, struct stat *); -void file_close(FILE *file); +int file_open(char const *, FILE **, struct stat *); +void file_close(FILE *); -int file_load(struct rpki_uri const *, struct file_contents *); +int file_load(char const *, struct file_contents *); void file_free(struct file_contents *); #endif /* SRC_FILE_H_ */ diff --git a/src/main.c b/src/main.c index 82f733ad..a6a067b9 100644 --- a/src/main.c +++ b/src/main.c @@ -92,7 +92,7 @@ main(int argc, char **argv) if (error) return error; - error = rsync_init(config_get_enable_rsync()); + error = rsync_init(); if (error) goto end1; diff --git a/src/object/certificate.c b/src/object/certificate.c index af0e7c8d..50f77ae8 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -6,12 +6,12 @@ #include #include "algorithm.h" -#include "common.h" #include "config.h" #include "extension.h" #include "log.h" #include "manifest.h" #include "nid.h" +#include "str.h" #include "thread_var.h" #include "asn1/decode.h" #include "asn1/oid.h" diff --git a/src/object/tal.c b/src/object/tal.c index bf47ee52..8e8f912a 100644 --- a/src/object/tal.c +++ b/src/object/tal.c @@ -212,7 +212,8 @@ foreach_uri(struct tal *tal, foreach_uri_cb cb) int error; for (i = 0; i < tal->uris.count; i++) { - error = uri_init_str(&uri, tal->uris.array[i]); + error = uri_init_str(&uri, tal->uris.array[i], + strlen(tal->uris.array[i])); if (error == ENOTRSYNC) { /* Log level should probably be INFO. */ pr_debug("TAL has non-RSYNC URI; ignoring."); diff --git a/src/rpp.c b/src/rpp.c index b7992f83..289fa334 100644 --- a/src/rpp.c +++ b/src/rpp.c @@ -102,7 +102,6 @@ add_crl_to_stack(struct rpp *pp, STACK_OF(X509_CRL) *crls) int error; int idx; - pr_debug("MANIFEST.CRL: %s", pp->crl.global); fnstack_push(pp->crl.global); error = crl_load(&pp->crl, &crl); diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c index 30433b63..851c01c0 100644 --- a/src/rsync/rsync.c +++ b/src/rsync/rsync.c @@ -1,17 +1,17 @@ #include "rsync.h" -#include -#include -#include #include -#include -#include -#include +#include #include +#include +#include +#include +#include +#include -#include "common.h" #include "config.h" #include "log.h" +#include "str.h" struct uri { char *string; @@ -19,25 +19,15 @@ struct uri { SLIST_ENTRY(uri) next; }; -SLIST_HEAD(uri_list, uri); +/** URIs that we have already downloaded. */ +SLIST_HEAD(uri_list, uri) visited_uris; -static struct uri_list *rsync_uris; -static char const *const RSYNC_PREFIX = "rsync://"; -static bool rsync_enabled; +/* static char const *const RSYNC_PREFIX = "rsync://"; */ int -rsync_init(bool is_rsync_enable) +rsync_init(void) { - /* Disabling rsync will forever be a useful debugging feature. */ - rsync_enabled = is_rsync_enable; - if (!rsync_enabled) - return 0; - - rsync_uris = malloc(sizeof(struct uri_list)); - if (rsync_uris == NULL) - return pr_enomem(); - - SLIST_INIT(rsync_uris); + SLIST_INIT(&visited_uris); return 0; } @@ -46,284 +36,149 @@ rsync_destroy(void) { struct uri *uri; - if (!rsync_enabled) - return; - - while (!SLIST_EMPTY(rsync_uris)) { - uri = SLIST_FIRST(rsync_uris); - SLIST_REMOVE_HEAD(rsync_uris, next); + while (!SLIST_EMPTY(&visited_uris)) { + uri = SLIST_FIRST(&visited_uris); + SLIST_REMOVE_HEAD(&visited_uris, next); free(uri->string); free(uri); } - - free(rsync_uris); -} - -/* - * Executes the rsync command. 'rsync_uri' as SRC and 'localuri' as DEST. - */ -static int -do_rsync(char const *rsync_uri, char const *localuri) -{ - int error; - char *command; - char const *rsync_command = "rsync --recursive --delete --times " - "--contimeout=20 "; /* space char at end*/ - - command = malloc(strlen(rsync_command) - + strlen(rsync_uri) - + 1 /* space char */ - + strlen(localuri) + 1); /* null char at end*/ - - if (command == NULL) - return pr_enomem(); - - strcpy(command, rsync_command); - strcat(command, rsync_uri); - strcat(command, " "); - strcat(command, localuri); - - pr_debug("(%s) command = %s", __func__, command); - - /* - * TODO (next iteration) system(3): "Do not use system() from a - * privileged program" - * I don't think there's a reason to run this program with privileges, - * but consider using exec(3) instead. - */ - error = system(command); - if (error) { - int error2 = errno; - /* - * The error message needs to be really generic because it - * seems that the Linux system() and the OpenBSD system() - * return different things. - */ - pr_err("rsync returned nonzero. result:%d errno:%d", - error, error2); - if (error2) - pr_errno(error2, "The error message for errno is"); - } - free(command); - - return error; } /* - * Returns whether new_uri's prefix is rsync_uri. + * Returns true if @ancestor an ancestor of @descendant, or @descendant itself. + * Returns false otherwise. */ static bool -rsync_uri_prefix_equals(struct uri *rsync_uri, char const *new_uri) +is_descendant(struct uri *ancestor, struct rpki_uri const *descendant) { - size_t uri_len; - - uri_len = strlen(new_uri); - if (rsync_uri->len > uri_len) - return false; - - if (rsync_uri->string[rsync_uri->len - 1] != '/' - && uri_len > rsync_uri->len && new_uri[rsync_uri->len] != '/') { - return false; - } - - return strncasecmp(rsync_uri->string, new_uri, rsync_uri->len) == 0; + struct string_tokenizer ancestor_tokenizer; + struct string_tokenizer descendant_tokenizer; + + string_tokenizer_init(&ancestor_tokenizer, ancestor->string, + ancestor->len, '/'); + string_tokenizer_init(&descendant_tokenizer, descendant->global, + descendant->global_len, '/'); + + do { + if (!string_tokenizer_next(&ancestor_tokenizer)) + return true; + if (!string_tokenizer_next(&descendant_tokenizer)) + return false; + if (!token_equals(&ancestor_tokenizer, &descendant_tokenizer)) + return false; + } while (true); } /* - * Checks if the 'rsync_uri' match equal or as a child of an existing URI in - * the list. + * Returns whether @uri has already been rsync'd during the current validation + * run. */ static bool -is_uri_in_list(char const *rsync_uri) +is_already_downloaded(struct rpki_uri const *uri) { struct uri *cursor; - bool found; - found = false; - SLIST_FOREACH(cursor, rsync_uris, next) { - if (rsync_uri_prefix_equals(cursor, rsync_uri)) { - found = true; - break; - } - } + /* TODO (next iteration) this is begging for a radix trie. */ + SLIST_FOREACH(cursor, &visited_uris, next) + if (is_descendant(cursor, uri)) + return true; - return found; + return false; } static int -add_uri_to_list(char *rsync_uri_path) +mark_as_downloaded(struct rpki_uri *uri) { - struct uri *rsync_uri; + struct uri *node; - rsync_uri = malloc(sizeof(struct uri)); - if (rsync_uri == NULL) + node = malloc(sizeof(struct uri)); + if (node == NULL) return pr_enomem(); - rsync_uri->string = rsync_uri_path; - rsync_uri->len = strlen(rsync_uri_path); + node->string = uri->global; + node->len = uri->global_len; + uri->global = NULL; /* Ownership transferred. */ - SLIST_INSERT_HEAD(rsync_uris, rsync_uri, next); + SLIST_INSERT_HEAD(&visited_uris, node, next); return 0; } -/* - * Compares two URIs to obtain the common path if exist. - * Return NULL if URIs does not match or only match in its domain name. - * E.g. uri1=proto://a/b/c/d uri2=proto://a/b/c/f, will return proto://a/b/c/ - */ static int -find_prefix_path(char const *uri1, char const *uri2, char **result) +handle_strict_strategy(struct rpki_uri const *requested_uri, + struct rpki_uri *rsync_uri) { - int i, error; - char const *tmp, *last_equal; - - *result = NULL; - last_equal = NULL; - error = 0; - - /* - * This code looks for 3 slashes to start compare path section. - */ - tmp = uri1; - for (i = 0; i < 3; i++) { - tmp = strchr(tmp, '/'); - if (tmp == NULL) { - goto end; - } - tmp++; - } - - /* Compare protocol and domain. */ - if (strncmp(uri1, uri2, tmp - uri1) != 0) - goto end; - - while((tmp = strchr(tmp, '/')) != NULL) { - if (strncmp(uri1, uri2, tmp - uri1) != 0) { - break; - } - last_equal = tmp; - tmp++; - } + return uri_clone(requested_uri, rsync_uri); +} - if (last_equal != NULL) { - /*+ 1 slash + 1 null char*/ - *result = malloc(last_equal - uri1 - + 1 /* + slash char */ - + 1); /* + null char */ - if (*result == NULL) { - error = pr_enomem(); - goto end; +static int +handle_root_strategy(struct rpki_uri const *src, struct rpki_uri *dst) +{ + unsigned int slashes; + size_t i; + + slashes = 0; + for (i = 0; i < src->global_len; i++) { + if (src->global[i] == '/') { + slashes++; + if (slashes == 4) + return uri_init_str(dst, src->global, i); } - strncpy(*result, uri1, last_equal - uri1 + 1); - (*result)[last_equal - uri1 + 1] = '\0'; } -end: - return error; + return uri_clone(src, dst); } -/* - * Compares rsync_uri against the uri_list and checks if can obtain a common - * short path. - * Returns NULL if URIs does not match any URI in the List. - * @see find_prefix_path - */ static int -compare_uris_and_short(char const *rsync_uri, char **result) +get_rsync_uri(struct rpki_uri const *requested_uri, struct rpki_uri *rsync_uri) { - struct uri *cursor; - int error; - - *result = NULL; - SLIST_FOREACH(cursor, rsync_uris, next) { - error = find_prefix_path(rsync_uri, cursor->string, result); - - if (error) - return error; - - if (*result != NULL) - break; + switch (config_get_sync_strategy()) { + case SYNC_ROOT: + return handle_root_strategy(requested_uri, rsync_uri); + case SYNC_STRICT: + return handle_strict_strategy(requested_uri, rsync_uri); + case SYNC_OFF: + return pr_crit("Supposedly unreachable code reached."); } - return 0; + return pr_crit("Unknown sync strategy: %u", config_get_sync_strategy()); } -/* - * Removes filename or last path if not end with an slash char. - */ static int -get_path_only(char const *uri, size_t uri_len, size_t rsync_prefix_len, - char **result) +dir_exists(char *path, bool *result) { - int error, i; - char tmp_uri[uri_len + 1]; - char *slash_search; - bool is_domain_only; - - slash_search = NULL; - is_domain_only = false; - error = 0; + struct stat _stat; + char *last_slash; - for (i = 0; i < uri_len + 1; i++) { - tmp_uri[i] = uri[i]; + last_slash = strrchr(path, '/'); + if (last_slash == NULL) { + /* + * Simply because create_dir_recursive() has nothing meaningful + * to do when this happens. It's a pretty strange error. + */ + *result = true; + return 0; } - if (tmp_uri[uri_len - 1] != '/') { - slash_search = strrchr(tmp_uri, '/'); - } + *last_slash = '\0'; - if (slash_search != NULL) { - if ((slash_search - tmp_uri) > rsync_prefix_len) { - tmp_uri[slash_search - tmp_uri + 1] = '\0'; - uri_len = strlen(tmp_uri); - } else { - is_domain_only = true; - slash_search = NULL; + switch (stat(path, &_stat)) { + case 0: + if (!S_ISDIR(_stat.st_mode)) { + return pr_err("Path '%s' exists and is not a directory.", + path); } - } - - if (is_domain_only) - uri_len++; /* Add slash */ - - *result = malloc(uri_len + 1); /* +1 null char */ - if (*result == NULL) { - error = pr_enomem(); - goto end; - } - - strcpy(*result, tmp_uri); - - if (is_domain_only) { - (*result)[uri_len - 1] = '/'; - (*result)[uri_len] = '\0'; - } - -end: - return error; -} - -static int -dir_exist(char *path, bool *result) -{ - struct stat _stat; - int error; - error = stat(path, &_stat); - if (error != 0) { - if (errno == ENOENT) { - *result = false; - goto end; - } else { - return pr_errno(errno, "stat() failed"); - } + *result = true; + break; + case ENOENT: + *result = false; + break; + default: + return pr_errno(errno, "stat() failed"); } - if (!S_ISDIR(_stat.st_mode)) - return pr_err("Path '%s' exist but is not a directory", path); - - *result = true; -end: + *last_slash = '/'; return 0; } @@ -341,6 +196,10 @@ create_dir(char *path) return 0; } +/** + * Apparently, RSYNC does not like to create parent directories. + * This function fixes that. + */ static int create_dir_recursive(char *localuri) { @@ -348,7 +207,7 @@ create_dir_recursive(char *localuri) int i, error; bool exist = false; - error = dir_exist(localuri, &exist); + error = dir_exists(localuri, &exist); if (error) return error; @@ -371,70 +230,146 @@ create_dir_recursive(char *localuri) return 0; } -int -download_files(struct rpki_uri const *uri) +static void +handle_child_thread(struct rpki_uri *uri) { - size_t prefix_len; - char *rsync_uri_path, *localuri, *tmp; + /* THIS FUNCTION MUST NEVER RETURN!!! */ + + struct string_array const *config_args; + char **copy_args; + unsigned int i; int error; - prefix_len = strlen(RSYNC_PREFIX); + config_args = config_get_rsync_args(); + /* + * We need to work on a copy, because the config args are immutable, + * and we need to add the program name (for some reason) and NULL + * elements, and replace $REMOTE and $LOCAL. + */ + copy_args = calloc(config_args->length + 2, sizeof(char *)); + if (copy_args == NULL) + exit(pr_enomem()); - if (!rsync_enabled) - return 0; + copy_args[0] = config_get_rsync_program(); + copy_args[config_args->length + 1] = NULL; - if (uri->global_len < prefix_len || - strncmp(RSYNC_PREFIX, uri->global, prefix_len) != 0) { - pr_err("Global URI '%s' does not begin with '%s'.", - uri->global, RSYNC_PREFIX); - return ENOTRSYNC; /* Not really an error, so not negative */ - } + memcpy(copy_args + 1, config_args->array, + config_args->length * sizeof(char *)); - if (is_uri_in_list(uri->global)){ - pr_debug("(%s) ON LIST: %s", __func__, uri->global); - error = 0; - goto end; - } else { - pr_debug("(%s) DOWNLOAD: %s", __func__, uri->global); + for (i = 1; i < config_args->length + 1; i++) { + if (strcmp(copy_args[i], "$REMOTE") == 0) + copy_args[i] = uri->global; + else if (strcmp(copy_args[i], "$LOCAL") == 0) + copy_args[i] = uri->local; } - error = get_path_only(uri->global, uri->global_len, prefix_len, - &rsync_uri_path); + pr_debug("Executing RSYNC:"); + for (i = 0; i < config_args->length + 1; i++) + pr_debug(" %s", copy_args[i]); + + execvp(copy_args[0], copy_args); + error = errno; + pr_err("Could not execute the rsync command: %s", + strerror(error)); + + /* + * https://stackoverflow.com/a/14493459/1735458 + * Might as well. Prrbrrrlllt. + */ + free(copy_args); + + exit(error); +} + +/* + * Downloads the @uri->global file into the @uri->local path. + */ +static int +do_rsync(struct rpki_uri *uri) +{ + pid_t child_pid; + int child_status; + int error; + + error = create_dir_recursive(uri->local); if (error) return error; - error = compare_uris_and_short(rsync_uri_path, &tmp); - if (error) { - goto free_uri_path; + /* We need to fork because execvp() magics the thread away. */ + child_pid = fork(); + if (child_pid == 0) + handle_child_thread(uri); /* This code is run by the child. */ + + /* This code is run by us. */ + + error = waitpid(child_pid, &child_status, 0); + if (error == -1) { + error = errno; + pr_err("The rsync sub-process returned error %d (%s)", + error, strerror(error)); + return error; } - if (tmp != NULL) { - free(rsync_uri_path); - rsync_uri_path = tmp; + if (WIFEXITED(child_status)) { + /* Happy path (but also sad path sometimes). */ + error = WEXITSTATUS(child_status); + pr_debug("Child terminated with error code %d.", error); + return error; } - error = uri_g2l(rsync_uri_path, &localuri); - if (error) - goto free_uri_path; + if (WIFSIGNALED(child_status)) { + switch (WTERMSIG(child_status)) { + case SIGINT: + pr_err("RSYNC was user-interrupted. Guess I'll interrupt myself too."); + break; + case SIGQUIT: + pr_err("RSYNC received a quit signal. Guess I'll quit as well."); + break; + case SIGKILL: + pr_err("Killed."); + break; + default: + pr_err("The RSYNC was terminated by a signal I don't have a handler for. Dunno; guess I'll just die."); + break; + } + exit(-EINTR); /* Meh? */ + } - error = create_dir_recursive(localuri); - if (error) - goto free_uri_path; + pr_err("The RSYNC command died in a way I don't have a handler for. Dunno; guess I'll die as well."); + exit(-EINVAL); +} - error = do_rsync(rsync_uri_path, localuri); - free(localuri); - if (error) - goto free_uri_path; +int +download_files(struct rpki_uri const *requested_uri) +{ + /** + * Note: + * @requested_uri is the URI we were asked to RSYNC. + * @rsync_uri is the URL we're actually going to RSYNC. + * (They can differ, depending on config_get_sync_strategy().) + */ + struct rpki_uri rsync_uri; + int error; - error = add_uri_to_list(rsync_uri_path); + if (config_get_sync_strategy() == SYNC_OFF) + return 0; + + if (is_already_downloaded(requested_uri)) { + pr_debug("No need to redownload '%s'.", requested_uri->global); + return 0; + } + + error = get_rsync_uri(requested_uri, &rsync_uri); if (error) - goto free_uri_path; + return error; - return 0; + pr_debug("Going to RSYNC '%s' ('%s').", rsync_uri.global, + rsync_uri.local); -free_uri_path: - free(rsync_uri_path); -end: - return error; + error = do_rsync(&rsync_uri); + if (!error) + error = mark_as_downloaded(&rsync_uri); + uri_cleanup(&rsync_uri); + return error; } diff --git a/src/rsync/rsync.h b/src/rsync/rsync.h index 688c1df6..50d92437 100644 --- a/src/rsync/rsync.h +++ b/src/rsync/rsync.h @@ -1,11 +1,10 @@ #ifndef SRC_RSYNC_RSYNC_H_ #define SRC_RSYNC_RSYNC_H_ -#include #include "uri.h" int download_files(struct rpki_uri const *); -int rsync_init(bool); +int rsync_init(void); void rsync_destroy(void); diff --git a/src/str.c b/src/str.c new file mode 100644 index 00000000..d8e52d53 --- /dev/null +++ b/src/str.c @@ -0,0 +1,82 @@ +#include "str.h" + +#include +#include "log.h" + +/** + * Does not assume that @string is NULL-terminated. + */ +int +string_clone(void const *string, size_t size, char **clone) +{ + char *result; + + result = malloc(size + 1); + if (result == NULL) + return pr_enomem(); + + memcpy(result, string, size); + result[size] = '\0'; + + *clone = result; + return 0; +} + +int +ia5s2string(ASN1_IA5STRING *ia5, char **result) +{ + return (ia5->flags & ASN1_STRING_FLAG_BITS_LEFT) + ? pr_err("CRL URI IA5String has unused bits.") + : string_clone(ia5->data, ia5->length, result); +} + +void +string_tokenizer_init(struct string_tokenizer *tokenizer, char const *str, + size_t str_len, unsigned char separator) +{ + tokenizer->str = str; + tokenizer->str_len = str_len; + tokenizer->separator = separator; + tokenizer->start = 0; + tokenizer->end = 0; +} + +/** + * Returns whether a new token was found. + */ +bool +string_tokenizer_next(struct string_tokenizer *tokenizer) +{ + size_t end = tokenizer->end; + + if (end == tokenizer->str_len) + return false; + + if (end != 0) { /* end is pointing to a slash. */ + end++; + if (end == tokenizer->str_len) + return false; + + tokenizer->start = end; + } /* otherwise it's pointing to the beginning of the string. */ + + for (; end < tokenizer->str_len; end++) + if (tokenizer->str[end] == tokenizer->separator) + break; + + tokenizer->end = end; + return true; +} + +/** + * Returns whether the tokens described by @t1 and @t2 are identical. + */ +bool +token_equals(struct string_tokenizer *t1, struct string_tokenizer *t2) +{ + size_t t1len = t1->end - t1->start; + size_t t2len = t2->end - t2->start; + return (t1len == t2len) + ? (memcmp(t1->str + t1->start, t2->str + t2->start, t1len) == 0) + : false; +} diff --git a/src/str.h b/src/str.h new file mode 100644 index 00000000..5e93a1d2 --- /dev/null +++ b/src/str.h @@ -0,0 +1,34 @@ +#ifndef SRC_STR_H_ +#define SRC_STR_H_ + +#include +#include +#include +#include + +int string_clone(void const *, size_t, char **); +int ia5s2string(ASN1_IA5STRING *, char **); + +/* This file is named "str.h" because "string.h" collides with . */ + +/** + * Do not modify fields directly; this should be private. + */ +struct string_tokenizer { + /** String we're tokenizing. */ + char const *str; + size_t str_len; + /** Token delimiter. */ + unsigned char separator; + /** Offset of the first character of the current token. */ + size_t start; + /** Offset of the last character of the current token + 1. */ + size_t end; +}; + +void string_tokenizer_init(struct string_tokenizer *, char const *, size_t, + unsigned char); +bool string_tokenizer_next(struct string_tokenizer *); +bool token_equals(struct string_tokenizer *, struct string_tokenizer *); + +#endif /* SRC_STR_H_ */ diff --git a/src/toml_handler.c b/src/toml_handler.c index 07f7b419..b24cb8e4 100644 --- a/src/toml_handler.c +++ b/src/toml_handler.c @@ -16,7 +16,6 @@ toml_to_config(struct toml_table_t *root) struct group_fields const *group; struct option_field const *option; struct toml_table_t *table; - const char *value; int error; get_group_fields(&groups); @@ -29,10 +28,8 @@ toml_to_config(struct toml_table_t *root) for (option = group->options; option->id != 0; option++) { if (option->availability == 0 || (option->availability & AVAILABILITY_TOML)) { - value = toml_raw_in(table, option->name); - if (value == NULL) - continue; - error = parse_option(option, value); + error = option->type->parse.toml(option, table, + get_rpki_config_field(option)); if (error) return error; } @@ -48,15 +45,10 @@ set_config_from_file(char *config_file) FILE *file; struct stat stat; struct toml_table_t *root; - struct rpki_uri uri; char errbuf[200]; int error; - uri.global = config_file; - uri.global_len = strlen(config_file); - uri.local = config_file; - - error = file_open(&uri, &file, &stat); + error = file_open(config_file, &file, &stat); if (error) return error; /* Error msg already printed. */ diff --git a/src/uri.c b/src/uri.c index 6d942a60..da645ec3 100644 --- a/src/uri.c +++ b/src/uri.c @@ -3,6 +3,46 @@ #include "common.h" #include "config.h" #include "log.h" +#include "str.h" + +/* + * @character is an integer because we sometimes receive signed chars, and other + * times we get unsigned chars. + * Casting a negative char into a unsigned char is undefined behavior. + */ +static int +validate_url_character(int character) +{ + /* + * RFCs 1738 and 3986 define a very specific range of allowed + * characters, but I don't think we're that concerned about URL + * correctness. Validating the URL properly is more involved than simply + * checking legal characters, anyway. + * + * What I really need this validation for is ensure that we won't get + * any trouble later, when we attempt to convert the global URI to a + * local file. + * + * Sample trouble: Getting UTF-8 characters. Why are they trouble? + * Because we don't have any guarantees that the system's file name + * encoding is UTF-8. URIs are not supposed to contain UTF-8 in the + * first place, so we have no reason to deal with encoding conversion. + * + * To be perfectly fair, we have no guarantees that the system's file + * name encoding is ASCII-compatible either, but I need to hang onto + * SOMETHING. + * + * (Asking users to use UTF-8 is fine, but asking users to use something + * ASCII-compatible is a little better.) + * + * So just make sure that the character is printable ASCII. + * + * TODO (next iteration) Consider exhaustive URL validation. + */ + return (0x20 <= character && character <= 0x7E) + ? 0 + : pr_err("URL has non-printable character code '%d'.", character); +} /** * Initializes @uri->global* by cloning @str. @@ -11,8 +51,21 @@ static int str2global(void const *str, size_t str_len, struct rpki_uri *uri) { + int error; + size_t i; + + error = string_clone(str, str_len, &uri->global); + if (error) + return error; uri->global_len = str_len; - return string_clone(str, str_len, &uri->global); + + for (i = 0; i < str_len; i++) { + error = validate_url_character(uri->global[i]); + if (error) + return error; + } + + return 0; } /** @@ -21,10 +74,15 @@ str2global(void const *str, size_t str_len, struct rpki_uri *uri) * * ie. if @mft is "rsync://a/b/c.mft" and @ia5 is "d/e/f.cer", @uri->global will * be "rsync://a/b/d/e/f.cer". + * + * Assumes that @mft is a "global" URL. (ie. extracted from rpki_uri.global.) */ static int ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5) { + int error; + size_t i; + char *joined; char *slash_pos; int dir_len; @@ -35,6 +93,12 @@ ia5str2global(struct rpki_uri *uri, char const *mft, IA5String_t *ia5) * `(char *) ia5->buf` is fair, but `strlen(ia5->buf)` is not. */ + for (i = 0; i < ia5->size; i++) { + error = validate_url_character(ia5->buf[i]); + if (error) + return error; + } + slash_pos = strrchr(mft, '/'); if (slash_pos == NULL) { joined = malloc(ia5->size + 1); @@ -138,12 +202,10 @@ uri_init(struct rpki_uri *uri, void const *guri, size_t guri_len) return 0; } -/** - * Do not call this function unless you're sure that @guri is NULL-terminated. - */ -int uri_init_str(struct rpki_uri *uri, char const *guri) +int +uri_init_str(struct rpki_uri *uri, char const *guri, size_t guri_len) { - return uri_init(uri, guri, strlen(guri)); + return uri_init(uri, guri, guri_len); } /* @@ -212,6 +274,23 @@ uri_init_ad(struct rpki_uri *uri, ACCESS_DESCRIPTION *ad) ASN1_STRING_length(asn1_string)); } +int +uri_clone(struct rpki_uri const *old, struct rpki_uri *copy) +{ + copy->global = strdup(old->global); + if (copy->global == NULL) + return pr_enomem(); + copy->global_len = old->global_len; + + copy->local = strdup(old->local); + if (copy->local == NULL) { + free(copy->global); + return pr_enomem(); + } + + return 0; +} + void uri_cleanup(struct rpki_uri *uri) { @@ -239,9 +318,3 @@ uri_is_certificate(struct rpki_uri const *uri) { return uri_has_extension(uri, ".cer"); } - -int -uri_g2l(char const *global, char **local) -{ - return g2l(global, strlen(global), local); -} diff --git a/src/uri.h b/src/uri.h index de2bc476..757589aa 100644 --- a/src/uri.h +++ b/src/uri.h @@ -9,12 +9,25 @@ * These are expected to live on the stack, or as part of other objects. * * All rpki_uris are guaranteed to be RSYNC URLs right now. + * + * Design notes: + * + * Because we need to generate @local from @global, @global's allowed character + * set must be a subset of @local. Because this is Unix, @local must never + * contain NULL (except as a terminating character). Therefore, even though IA5 + * allows NULL, @global won't. TODO (NOW) validate this on constructors. + * + * Because we will simply embed @global (minus "rsync://") into @local, @local's + * encoding must be IA5-compatible. In other words, UTF-16 and UTF-32 are out of + * the question. */ struct rpki_uri { /** * "Global URI". * The one that always starts with "rsync://". - * As currently implemented, it's expected to live in the heap. + * + * These things are IA5-encoded, which means you're not bound to get + * non-ASCII characters. */ char *global; /** Length of @global. */ @@ -23,7 +36,19 @@ struct rpki_uri { /** * "Local URI". * The file pointed by @global, but cached in the local filesystem. - * As currently implemented, it's expected to live in the heap. + * + * I can't find a standard that defines this, but lots of complaints on + * the Internet imply that Unix file paths are specifically meant to be + * C strings. + * + * So just to clarify: This is a string that permits all characters, + * printable or otherwise, except \0. (Because that's the terminating + * character.) + * + * Even though it might contain characters that are non-printable + * according to ASCII, we assume that we can just dump it into the + * output without trouble, because the input should have the same + * encoding as the output. */ char *local; @@ -31,14 +56,13 @@ struct rpki_uri { }; int uri_init(struct rpki_uri *, void const *, size_t); -int uri_init_str(struct rpki_uri *uri, char const *guri); +int uri_init_str(struct rpki_uri *, char const *, size_t); int uri_init_mft(struct rpki_uri *, char const *, IA5String_t *); -int uri_init_ad(struct rpki_uri *, ACCESS_DESCRIPTION *ad); +int uri_init_ad(struct rpki_uri *, ACCESS_DESCRIPTION *); +int uri_clone(struct rpki_uri const *, struct rpki_uri *); void uri_cleanup(struct rpki_uri *); bool uri_has_extension(struct rpki_uri const *, char const *); bool uri_is_certificate(struct rpki_uri const *); -int uri_g2l(char const *, char **); - #endif /* SRC_URI_H_ */ diff --git a/test/Makefile.am b/test/Makefile.am index 5bb2c4a5..75168b9e 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -8,8 +8,7 @@ AM_CFLAGS = -pedantic -Wall -std=gnu11 -I../src -DUNIT_TESTING ${CHECK_CFLAGS} MY_LDADD = ${CHECK_LIBS} -BASIC_MODULES = ../src/config.c ../src/config.h -BASIC_MODULES += ../src/log.c ../src/log.h +BASIC_MODULES = ../src/log.c ../src/log.h BASIC_MODULES += impersonator.c check_PROGRAMS = address.test @@ -35,16 +34,16 @@ line_file_test_SOURCES += line_file_test.c line_file_test_LDADD = ${MY_LDADD} rsync_test_SOURCES = ${BASIC_MODULES} -rsync_test_SOURCES += ../src/common.c ../src/common.h +rsync_test_SOURCES += ../src/str.c ../src/str.h rsync_test_SOURCES += ../src/uri.c ../src/uri.h rsync_test_SOURCES += rsync_test.c rsync_test_LDADD = ${MY_LDADD} tal_test_SOURCES = ${BASIC_MODULES} tal_test_SOURCES += ../src/file.c ../src/file.h -tal_test_SOURCES += ../src/common.c ../src/common.h tal_test_SOURCES += ../src/crypto/base64.c ../src/crypto/base64.h tal_test_SOURCES += ../src/random.c ../src/random.h +tal_test_SOURCES += ../src/str.c ../src/str.h tal_test_SOURCES += ../src/uri.c ../src/uri.h tal_test_SOURCES += ../src/line_file.c ../src/line_file.h tal_test_SOURCES += tal_test.c diff --git a/test/impersonator.c b/test/impersonator.c index 3ae99a65..c4c81d62 100644 --- a/test/impersonator.c +++ b/test/impersonator.c @@ -1,5 +1,7 @@ #include +#include "config.h" + /** * Some core functions, as linked from unit testing code. */ @@ -31,8 +33,34 @@ v6addr2str2(struct in6_addr *addr) return inet_ntop(AF_INET6, addr, addr_buffer2, sizeof(addr_buffer2)); } -int -set_config_from_file(char *config_file) +char const * +config_get_local_repository(void) +{ + return "repository/"; +} + +enum sync_strategy +config_get_sync_strategy(void) +{ + return SYNC_ROOT; +} + +bool +config_get_color_output(void) +{ + return false; +} + + +char * +config_get_rsync_program(void) +{ + return "rsync"; +} + +struct string_array const * +config_get_rsync_args(void) { - return 0; + static const struct string_array array = { 0 }; + return &array; } diff --git a/test/rsync_test.c b/test/rsync_test.c index b0f455ed..21bbd5e7 100644 --- a/test/rsync_test.c +++ b/test/rsync_test.c @@ -12,200 +12,139 @@ START_TEST(rsync_load_normal) } END_TEST -START_TEST(rsync_test_prefix_equals) +static void +assert_descendant(bool expected, char *ancestor, char *descendant) { - struct uri rsync_uri; - char *uri = "proto://a/b/c"; - - rsync_uri.len = strlen(uri); - rsync_uri.string = uri; - - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/c"), true); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/"), false); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/c/c"), true); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/cc"), false); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/cc/"), false); - - uri = "proto://a/b/c/"; - rsync_uri.len = strlen(uri); - rsync_uri.string = uri; - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/c"), false); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/"), false); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/c/c"), true); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/cc"), false); - ck_assert_int_eq(rsync_uri_prefix_equals(&rsync_uri, "proto://a/b/cc/"), false); + struct uri ancestor_uri; + struct rpki_uri descendant_uri; + ancestor_uri.string = ancestor; + ancestor_uri.len = strlen(ancestor); + descendant_uri.global = descendant; + descendant_uri.global_len = strlen(descendant); + + ck_assert_int_eq(is_descendant(&ancestor_uri, &descendant_uri), + expected); } -END_TEST -START_TEST(rsync_test_list) +START_TEST(rsync_test_prefix_equals) { - struct uri *uri; - char *string_uri, *test_string; - - rsync_init(true); - - string_uri = "rsync://example.foo/repository/"; - ck_assert_int_eq(add_uri_to_list(string_uri), 0); - string_uri = "rsync://example.foo/member_repository/"; - ck_assert_int_eq(add_uri_to_list(string_uri), 0); - string_uri = "rsync://example.foz/repository/"; - ck_assert_int_eq(add_uri_to_list(string_uri), 0); - string_uri = "rsync://example.boo/repo/"; - ck_assert_int_eq(add_uri_to_list(string_uri), 0); - string_uri = "rsync://example.potato/rpki/"; - ck_assert_int_eq(add_uri_to_list(string_uri), 0); - - test_string = "rsync://example.foo/repository/"; - ck_assert_int_eq(is_uri_in_list(test_string), true); - test_string = "rsync://example.foo/repository/abc/cdfg"; - ck_assert_int_eq(is_uri_in_list(test_string), true); - test_string = "rsync://example.foo/member_repository/bca"; - ck_assert_int_eq(is_uri_in_list(test_string), true); - test_string = "rsync://example.boo/repository/"; - ck_assert_int_eq(is_uri_in_list(test_string), false); - test_string = "rsync://example.potato/repository/"; - ck_assert_int_eq(is_uri_in_list(test_string), false); - test_string = "rsync://example.potato/rpki/abc/"; - ck_assert_int_eq(is_uri_in_list(test_string), true); - - /* rsync destroy */ - while(!SLIST_EMPTY(rsync_uris)) { - uri = SLIST_FIRST(rsync_uris); - SLIST_REMOVE_HEAD(rsync_uris, next); - free(uri); - } - - free(rsync_uris); + char *ancestor; + + ancestor = "proto://a/b/c"; + assert_descendant(true, ancestor, "proto://a/b/c"); + assert_descendant(false, ancestor, "proto://a/b/"); + assert_descendant(true, ancestor, "proto://a/b/c/c"); + assert_descendant(false, ancestor, "proto://a/b/cc"); + assert_descendant(false, ancestor, "proto://a/b/cc/"); + + ancestor = "proto://a/b/c/"; + assert_descendant(true, ancestor, "proto://a/b/c"); + assert_descendant(false, ancestor, "proto://a/b/"); + assert_descendant(true, ancestor, "proto://a/b/c/c"); + assert_descendant(false, ancestor, "proto://a/b/cc"); + assert_descendant(false, ancestor, "proto://a/b/cc/"); } END_TEST -static int -malloc_string(char *string, char **result) +static void +__mark_as_downloaded(char *uri_str) { - *result = malloc(strlen(string) + 1); - if (*result == NULL) { - return pr_enomem(); - } + struct rpki_uri uri; - strcpy(*result, string); - return 0; + uri.global = uri_str; + uri.global_len = strlen(uri_str); + + ck_assert_int_eq(mark_as_downloaded(&uri), 0); } static void -test_get_path(char *test, char *expected) +assert_downloaded(char *uri_str, bool expected) { - int error; - char *string, *result; - size_t rsync_prefix_len = strlen("rsync://"); - - error = malloc_string(test, &string); - if (error) - return; - - error = get_path_only(string, strlen(string), rsync_prefix_len, &result); - if (error) { - free(string); - return; - } + struct rpki_uri uri; - ck_assert_str_eq(expected, result); - ck_assert_str_eq(string, test); - free(string); - free(result); + uri.global = uri_str; + uri.global_len = strlen(uri_str); - return; + ck_assert_int_eq(is_already_downloaded(&uri), expected); } -START_TEST(rsync_test_get_path) +START_TEST(rsync_test_list) { - test_get_path("rsync://www.example.com/", "rsync://www.example.com/"); - test_get_path("rsync://www.example.com", "rsync://www.example.com/"); - test_get_path("rsync://www.example.com/test", "rsync://www.example.com/"); - test_get_path("rsync://www.example.com/test/", "rsync://www.example.com/test/"); - test_get_path("rsync://www.example.com/test/abc", "rsync://www.example.com/test/"); - test_get_path("rsync://www.example.com/test/abc/", "rsync://www.example.com/test/abc/"); - test_get_path("rsync://www.example.com/test/abc/abc.file", "rsync://www.example.com/test/abc/"); - test_get_path("rsync://www.example.com/test/file.txt", "rsync://www.example.com/test/"); -} -END_TEST + struct uri *uri; -static void -test_get_prefix_from_URIs(char *expected, char *stored_uri, char *new_uri) -{ - int error; - char *result; - error = find_prefix_path(new_uri, stored_uri, &result); + ck_assert_int_eq(rsync_init(), 0); - if (error) - return; + __mark_as_downloaded("rsync://example.foo/repository/"); + __mark_as_downloaded("rsync://example.foo/member_repository/"); + __mark_as_downloaded("rsync://example.foz/repository/"); + __mark_as_downloaded("rsync://example.boo/repo/"); + __mark_as_downloaded("rsync://example.potato/rpki/"); - if (expected == NULL) { - ck_assert_ptr_eq(expected, result); - } else { - ck_assert_str_eq(expected, result); + assert_downloaded("rsync://example.foo/repository/", true); + assert_downloaded("rsync://example.foo/repository/abc/cdfg", true); + assert_downloaded("rsync://example.foo/member_repository/bca", true); + assert_downloaded("rsync://example.boo/repository/", false); + assert_downloaded("rsync://example.potato/repository/", false); + assert_downloaded("rsync://example.potato/rpki/abc/", true); + + /* rsync destroy */ + while (!SLIST_EMPTY(&visited_uris)) { + uri = SLIST_FIRST(&visited_uris); + SLIST_REMOVE_HEAD(&visited_uris, next); + free(uri); } +} +END_TEST - if (result != NULL) - free(result); +static void +test_root_strategy(char *test, char *expected) +{ + struct rpki_uri src; + struct rpki_uri dst; + + src.global = strdup(test); + if (src.global == NULL) + ck_abort_msg("Out of memory."); + src.global_len = strlen(test); + src.local = strdup("foo"); + if (src.local == NULL) + ck_abort_msg("Out of memory."); + + ck_assert_int_eq(handle_root_strategy(&src, &dst), 0); + ck_assert_str_eq(dst.global, expected); + + free(src.global); + free(src.local); + free(dst.global); + free(dst.local); } START_TEST(rsync_test_get_prefix) { - char *expected, *stored_uri, *new_uri; - - new_uri = "rsync://www.example1.com/test/foo/"; - stored_uri = "rsync://www.example1.com/test/bar/"; - expected = "rsync://www.example1.com/test/"; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example2.com/test/foo/"; - stored_uri = "rsync://www.example2.co/test/bar/"; - expected = NULL; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example3.com/test/foo/"; - stored_uri = "rsync://www.example3.com/test/foo/test/"; - expected = "rsync://www.example3.com/test/foo/"; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example4.com/test/"; - stored_uri = "rsync://www.example4.com/test/foo/bar"; - expected = "rsync://www.example4.com/test/"; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example5.com/foo/"; - stored_uri = "rsync://www.example5.com/bar/"; - expected = NULL; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example6.com/bar/foo/"; - stored_uri = "rsync://www.example6.com/bar/"; - expected = "rsync://www.example6.com/bar/"; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example7.com/"; - stored_uri = "rsync://www.example7.com/bar/"; - expected = NULL; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example8.com/bar"; - stored_uri = "rsync://www.example8.com/"; - expected = NULL; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - - new_uri = "rsync://www.example9.com/bar/"; - stored_uri = "rsync://www.example9.com/bar/"; - expected = "rsync://www.example9.com/bar/"; - test_get_prefix_from_URIs(expected, stored_uri, new_uri); - + test_root_strategy("rsync://www.example1.com/test/foo/", + "rsync://www.example1.com/test"); + test_root_strategy("rsync://www.example1.com/test/foo/bar", + "rsync://www.example1.com/test"); + test_root_strategy("rsync://www.example1.com/test/", + "rsync://www.example1.com/test"); + test_root_strategy("rsync://www.example1.com/test", + "rsync://www.example1.com/test"); + test_root_strategy("rsync://www.example1.com", + "rsync://www.example1.com"); + test_root_strategy("rsync://w", "rsync://w"); + test_root_strategy("rsync://", "rsync://"); + test_root_strategy("rsync:/", "rsync:/"); + test_root_strategy("rsync:", "rsync:"); + test_root_strategy("r", "r"); + test_root_strategy("", ""); } END_TEST Suite *rsync_load_suite(void) { Suite *suite; - TCase *core, *prefix_equals, *uri_list, *test_get_path, *test_get_prefix; + TCase *core, *prefix_equals, *uri_list, *test_get_prefix; core = tcase_create("Core"); tcase_add_test(core, rsync_load_normal); @@ -216,9 +155,6 @@ Suite *rsync_load_suite(void) uri_list = tcase_create("uriList"); tcase_add_test(uri_list, rsync_test_list); - test_get_path = tcase_create("test_static_get_path"); - tcase_add_test(test_get_path, rsync_test_get_path); - test_get_prefix = tcase_create("test_get_prefix"); tcase_add_test(test_get_prefix, rsync_test_get_prefix); @@ -226,7 +162,6 @@ Suite *rsync_load_suite(void) suite_add_tcase(suite, core); suite_add_tcase(suite, prefix_equals); suite_add_tcase(suite, uri_list); - suite_add_tcase(suite, test_get_path); suite_add_tcase(suite, test_get_prefix); return suite;