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@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.

+/* 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@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs