[Libguestfs] [PATCH v2 2/6] New API: yara_load

[noxdafox]

On 21/11/16 18:27, Pino Toscano wrote:
> On Wednesday, 9 November 2016 22:38:53 CET Matteo Cafasso wrote:
>> The yara_load API allows to load a set of Yara rules contained within a
>> file on the host.
>>
>> Rules can be in binary format, as when compiled with yarac command, or
>> in source code format. In the latter case, the rules will be first
>> compiled and then loaded.
>>
>> Subsequent calls of the yara_load API will result in the discard of the
>> previously loaded rules.
>>
>> Signed-off-by: Matteo Cafasso <noxdafox at gmail.com>
>> ---
>> [...]
>> diff --git a/daemon/cleanups.c b/daemon/cleanups.c
>> index 092e493..a02e521 100644
>> --- a/daemon/cleanups.c
>> +++ b/daemon/cleanups.c
>> @@ -78,3 +93,16 @@ cleanup_free_stringsbuf (void *ptr)
>>   {
>>     free_stringsbuf ((struct stringsbuf *) ptr);
>>   }
>> +
>> +#ifdef HAVE_YARA
>> +
>> +void
>> +cleanup_destroy_yara_compiler (void *ptr)
>> +{
>> +  YR_COMPILER *compiler = * (YR_COMPILER **) ptr;
>> +
>> +  if (compiler != NULL)
>> +    yr_compiler_destroy (compiler);
>> +}
>> +
> This should rather be directly in daemon/yara.c, since libyara would be
> used there only.
>
>> +static int
>> +upload_rules_file (char *rules_path)
>> +{
>> +  int ret = 0;
>> +  CLEANUP_CLOSE int fd = 0;
>> +  struct write_callback_data data = { .written = 0 };
>> +
>> +  data.fd = mkstemp (rules_path);
>> +  if (data.fd == -1) {
>> +    reply_with_perror ("mkstemp");
>> +    return -1;
>> +  }
>> +
>> +  ret = receive_file (write_callback, &data);
>> +  if (ret == -1) {
>> +    /* Write error. */
>> +    cancel_receive ();
>> +    reply_with_error ("write error: %s", rules_path);
>> +    return -1;
>> +  }
>> +  if (ret == -2) {
>> +    /* Cancellation from library */
>> +    reply_with_error ("file upload cancelled");
>> +    return -1;
>> +  }
>> +
>> +  return 0;
>> +}
> This function does not always unlink the temporary file on error, and
> it is never close either -- my suggestion would be to reuse and expose
> parts of the "upload" function in daemon/upload.c:
>
>    int upload_to_fd (int fd)
>
> With the above, upload_rules_file could not be needed anymore, and the
> logic to open a temporary fd could be moved directly at the beginning
> of do_yara_load.
Just one question: shall the changes in upload.c be in a separate commit 
"expose XYZ"? What is the preferred way?
This question applies also for the notes in PATCH 5.
>
>> +/* Compile source code rules and load them.
>> + * Return ERROR_SUCCESS on success, Yara error code type on error.
>> + */
>> +static int
>> +compile_rules_file (const char *rules_path)
>> +{
>> +  int ret = 0;
>> +  CLEANUP_FCLOSE FILE *rule_file = NULL;
>> +  CLEANUP_DESTROY_YARA_COMPILER YR_COMPILER *compiler = NULL;
>> +
>> +  ret = yr_compiler_create (&compiler);
>> +  if (ret != ERROR_SUCCESS) {
>> +    reply_with_error ("yr_compiler_create");
>> +    return ret;
>> +  }
>> +
>> +  yr_compiler_set_callback (compiler, compile_error_callback, NULL);
>> +
>> +  rule_file = fopen (rules_path, "r");
>> +  if (rule_file == NULL) {
>> +    reply_with_error ("unable to open rules file");
>> +    return ret;
>> +  }
>> +
>> +  ret = yr_compiler_add_file (compiler, rule_file, NULL, rules_path);
> Considering it's only a temporary file, and thus we don't show its path
> in error message, we could avoid passing rules_path here -- it looks
> like the libyara API allows NULL for this parameter.
>
>> +  if (ret > 0)
>> +    return ret;
>> +
>> +  ret = yr_compiler_get_rules (compiler, &rules);
>> +  if (ret != ERROR_SUCCESS)
>> +    reply_with_error ("yr_compiler_get_rules");
> The API doc say the return value can be either ERROR_SUCCESS or
> ERROR_INSUFICENT_MEMORY, so I'd map the latter to ENOMEM and use
> reply_with_perror.
>
>> +/* Yara compilation error callback.
>> + * Reports back the compilation error message.
>> + * Prints compilation warnings if verbose.
>> + */
>> +static void
>> +compile_error_callback(int level, const char *name, int line,
>> +                       const char *message, void *data)
>> +{
>> +  if (level == YARA_ERROR_LEVEL_ERROR)
>> +    reply_with_error ("(%d): Yara error: %s", line, message);
> "Yara error (line %d): %s"
>
>> +  else
>> +    fprintf (stderr, "(%d): Yara warning: %s\n", line, message);
> Ditto.
> In addition, IMHO this message should be shown only when verbose is
> true... like what is written in the API doc comment at the beginning
> of the function :)
>
>> +}
>> +
>> +/* Clean up yara handle on daemon exit. */
>> +void yara_finalize (void) __attribute__((destructor));
>> +void
>> +yara_finalize (void)
>> +{
>> +  int ret = 0;
>    if (!initialized)
>      return;
>
>> +
>> +  if (rules != NULL) {
>> +    yr_rules_destroy (rules);
>> +    rules = NULL;
>> +  }
>> +
>> +  ret = yr_finalize ();
>> +  if (ret != ERROR_SUCCESS)
>> +    perror ("yr_finalize");
>> +
>> +  initialized = false;
>> +}
> Thanks,
>
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20161122/ea8df082/attachment.htm>