Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan

[noxdafox]

On 24/11/16 17:42, Pino Toscano wrote:

On Tuesday, 22 November 2016 19:41:10 CET noxdafox wrote:

yara_load supports loading rules already compiled, which could have a
namespace set -- I guess it should be reported here as well.

The namespace is accessible via the YR_RULE struct:
https://github.com/VirusTotal/yara/blob/master/libyara/include/yara/types.h#L242

Yet is nowere to be found in the C API documentation.
http://yara.readthedocs.io/en/v3.5.0/capi.html#c.YR_RULE

That's why I kept it out of the scope. I can obviously add it but we're
not sure whether they will expose it differently in future versions of Yara.

Drat... Maybe it would be worth asking them if it's just a documentation
issue, or it is really private. In any case, it is not a big issue at
the moment.

https://github.com/VirusTotal/yara/issues/570

Let's keep it out for this patch series. I'll make sure we'll have a 
clear answer before the next stable release of libguestfs.


I'll slowly proceed applying the suggested changes. Thanks.



That triggers another question: should the yara support allow to load
more rules one after each other (with namespaces as well), instead of
just one?

We surely can do. I'll see what can be done. Maybe an optional parameter
"namespace" in the yara_load API.

Right, that is what I was thinking about.



___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan

[Pino Toscano]

On Tuesday, 22 November 2016 19:41:10 CET noxdafox wrote:
> > yara_load supports loading rules already compiled, which could have a
> > namespace set -- I guess it should be reported here as well.
> The namespace is accessible via the YR_RULE struct:
> https://github.com/VirusTotal/yara/blob/master/libyara/include/yara/types.h#L242
> 
> Yet is nowere to be found in the C API documentation.
> http://yara.readthedocs.io/en/v3.5.0/capi.html#c.YR_RULE
> 
> That's why I kept it out of the scope. I can obviously add it but we're 
> not sure whether they will expose it differently in future versions of Yara.

Drat... Maybe it would be worth asking them if it's just a documentation
issue, or it is really private. In any case, it is not a big issue at
the moment.

> > That triggers another question: should the yara support allow to load
> > more rules one after each other (with namespaces as well), instead of
> > just one?
> We surely can do. I'll see what can be done. Maybe an optional parameter 
> "namespace" in the yara_load API.

Right, that is what I was thinking about.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan

[noxdafox]

Ok on most of the comments, only few notes on the last one.

On 22/11/16 11:04, Pino Toscano wrote:

On Wednesday, 9 November 2016 22:38:55 CET Matteo Cafasso wrote:

The internal_yara_scan runs the Yara engine with the previously loaded
rules against the given file.

For each rule matching against the scanned file, a struct containing
the file name and the rule identifier is returned.

The gathered list of yara_detection structs is serialised into XDR format
and written to a file.

Signed-off-by: Matteo Cafasso 
---
  daemon/yara.c| 87 
  generator/actions.ml | 10 
  generator/structs.ml |  9 
  gobject/Makefile.inc |  2 +
  java/Makefile.inc|  1 +
  java/com/redhat/et/libguestfs/.gitignore |  1 +
  src/MAX_PROC_NR  |  2 +-
  7 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/daemon/yara.c b/daemon/yara.c
index fe1f69a..8e7d328 100644
--- a/daemon/yara.c
+++ b/daemon/yara.c
@@ -52,6 +52,8 @@ static int upload_rules_file (char *);
  static int compile_rules_file (const char *);
  static int write_callback (void *, const void *, size_t);
  static void compile_error_callback (int, const char *, int, const char *, 
void *);
+static int yara_rules_callback (int , void *, void *);
+static int send_detection_info (const char *, YR_RULE *);
  
  /* Has one FileIn parameter. */

  int
@@ -107,6 +109,39 @@ do_yara_destroy (void)
return 0;
  }
  
+/* Has one FileOut parameter. */

+int
+do_internal_yara_scan (const char *path)
+{
+  int ret = 0;
+  CLEANUP_CLOSE int fd = 0;

This must be initialized as -1, otherwise the CLEANUP_CLOSE handler
will close the fd 0, which is stdin of the daemon.


+
+  if (rules == NULL) {
+reply_with_error ("no yara rules loaded");
+return -1;
+  }
+
+  CHROOT_IN;
+  fd = open (path, O_RDONLY|O_CLOEXEC);
+  CHROOT_OUT;
+
+  if (fd < 0) {
+reply_with_perror ("%s", path);
+yr_finalize ();

I don't think that's the right place for yr_finalize.


+return -1;
+  }
+
+  reply (NULL, NULL);  /* Reply message. */
+
+  ret = yr_rules_scan_fd (rules, fd, 0, yara_rules_callback, (void *) path, 0);
+  if (ret == ERROR_SUCCESS)
+ret = send_file_end (0);  /* File transfer end. */
+  else
+send_file_end (1);  /* Cancel file transfer. */
+
+  return 0;
+}
+
  /* Upload rules file on a temporary file.
   * Return 0 on success, -1 on error.
   */
@@ -209,6 +244,58 @@ compile_error_callback(int level, const char *name, int 
line,
  fprintf (stderr, "(%d): Yara warning: %s\n", line, message);
  }
  
+/* Yara scan callback, called by yr_rules_scan_file.

+ * Return 0 on success, -1 on error.
+ */
+static int
+yara_rules_callback (int code, void *message, void *data)
+{
+  int ret = 0;
+
+  if (code == CALLBACK_MSG_RULE_MATCHING)
+ret = send_detection_info ((const char *)data, (YR_RULE *) message);
+
+  return (ret == 0) ? CALLBACK_CONTINUE : CALLBACK_ERROR;
+}
+
+/* Serialize file path and rule name and send it out.
+ * Return 0 on success, -1 on error.
+ */
+static int
+send_detection_info (const char *name, YR_RULE *rule)
+{
+  XDR xdr;
+  int ret = 0;
+  size_t len = 0;
+  struct guestfs_int_yara_detection detection;
+  CLEANUP_FREE char *buf = NULL, *fname = NULL;

fname is not used here -- I suggest passing --enable-werror to
autogen.sh or configure, so any compiler warning is turned to error.


+
+  detection.name = (char *) name;
+  detection.rule = (char *) rule->identifier;
+
+  /* Serialize detection struct. */
+  buf = malloc (GUESTFS_MAX_CHUNK_SIZE);
+  if (buf == NULL) {
+perror ("malloc");
+return -1;
+  }
+
+  xdrmem_create (, buf, GUESTFS_MAX_CHUNK_SIZE, XDR_ENCODE);
+
+  ret = xdr_guestfs_int_yara_detection (, );
+  if (ret == 0) {
+perror ("xdr_guestfs_int_yara_detection");
+return -1;
+  }
+
+  len = xdr_getpos ();
+
+  xdr_destroy ();
+
+  /* Send serialised tsk_detection out. */

Typo in comment.


+  return send_file_write (buf, len);
+}
+
  /* Clean up yara handle on daemon exit. */
  void yara_finalize (void) __attribute__((destructor));
  void
diff --git a/generator/actions.ml b/generator/actions.ml
index 152c651..d9006f2 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -13280,6 +13280,16 @@ Previously loaded rules will be destroyed." };
  shortdesc = "destroy previously loaded yara rules";
  longdesc = "\
  Destroy previously loaded Yara rules in order to free libguestfs resources." 
};
+
+  { defaults with
+name = "internal_yara_scan"; added = (1, 35, 15);
+style = RErr, [Pathname "path"; FileOut "filename";], [];
+proc_nr = Some 473;
+visibility = VInternal;
+optional = Some "libyara";
+shortdesc = "scan a file with the loaded yara rules";
+longdesc = "Internal function for yara_scan." };
+
  ]
  
  (* Non-API meta-commands available only in guestfish.

diff --git 

Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan

[Pino Toscano]

On Wednesday, 9 November 2016 22:38:55 CET Matteo Cafasso wrote:
> The internal_yara_scan runs the Yara engine with the previously loaded
> rules against the given file.
> 
> For each rule matching against the scanned file, a struct containing
> the file name and the rule identifier is returned.
> 
> The gathered list of yara_detection structs is serialised into XDR format
> and written to a file.
> 
> Signed-off-by: Matteo Cafasso 
> ---
>  daemon/yara.c| 87 
> 
>  generator/actions.ml | 10 
>  generator/structs.ml |  9 
>  gobject/Makefile.inc |  2 +
>  java/Makefile.inc|  1 +
>  java/com/redhat/et/libguestfs/.gitignore |  1 +
>  src/MAX_PROC_NR  |  2 +-
>  7 files changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/yara.c b/daemon/yara.c
> index fe1f69a..8e7d328 100644
> --- a/daemon/yara.c
> +++ b/daemon/yara.c
> @@ -52,6 +52,8 @@ static int upload_rules_file (char *);
>  static int compile_rules_file (const char *);
>  static int write_callback (void *, const void *, size_t);
>  static void compile_error_callback (int, const char *, int, const char *, 
> void *);
> +static int yara_rules_callback (int , void *, void *);
> +static int send_detection_info (const char *, YR_RULE *);
>  
>  /* Has one FileIn parameter. */
>  int
> @@ -107,6 +109,39 @@ do_yara_destroy (void)
>return 0;
>  }
>  
> +/* Has one FileOut parameter. */
> +int
> +do_internal_yara_scan (const char *path)
> +{
> +  int ret = 0;
> +  CLEANUP_CLOSE int fd = 0;

This must be initialized as -1, otherwise the CLEANUP_CLOSE handler
will close the fd 0, which is stdin of the daemon.

> +
> +  if (rules == NULL) {
> +reply_with_error ("no yara rules loaded");
> +return -1;
> +  }
> +
> +  CHROOT_IN;
> +  fd = open (path, O_RDONLY|O_CLOEXEC);
> +  CHROOT_OUT;
> +
> +  if (fd < 0) {
> +reply_with_perror ("%s", path);
> +yr_finalize ();

I don't think that's the right place for yr_finalize.

> +return -1;
> +  }
> +
> +  reply (NULL, NULL);  /* Reply message. */
> +
> +  ret = yr_rules_scan_fd (rules, fd, 0, yara_rules_callback, (void *) path, 
> 0);
> +  if (ret == ERROR_SUCCESS)
> +ret = send_file_end (0);  /* File transfer end. */
> +  else
> +send_file_end (1);  /* Cancel file transfer. */
> +
> +  return 0;
> +}
> +
>  /* Upload rules file on a temporary file.
>   * Return 0 on success, -1 on error.
>   */
> @@ -209,6 +244,58 @@ compile_error_callback(int level, const char *name, int 
> line,
>  fprintf (stderr, "(%d): Yara warning: %s\n", line, message);
>  }
>  
> +/* Yara scan callback, called by yr_rules_scan_file.
> + * Return 0 on success, -1 on error.
> + */
> +static int
> +yara_rules_callback (int code, void *message, void *data)
> +{
> +  int ret = 0;
> +
> +  if (code == CALLBACK_MSG_RULE_MATCHING)
> +ret = send_detection_info ((const char *)data, (YR_RULE *) message);
> +
> +  return (ret == 0) ? CALLBACK_CONTINUE : CALLBACK_ERROR;
> +}
> +
> +/* Serialize file path and rule name and send it out.
> + * Return 0 on success, -1 on error.
> + */
> +static int
> +send_detection_info (const char *name, YR_RULE *rule)
> +{
> +  XDR xdr;
> +  int ret = 0;
> +  size_t len = 0;
> +  struct guestfs_int_yara_detection detection;
> +  CLEANUP_FREE char *buf = NULL, *fname = NULL;

fname is not used here -- I suggest passing --enable-werror to
autogen.sh or configure, so any compiler warning is turned to error.

> +
> +  detection.name = (char *) name;
> +  detection.rule = (char *) rule->identifier;
> +
> +  /* Serialize detection struct. */
> +  buf = malloc (GUESTFS_MAX_CHUNK_SIZE);
> +  if (buf == NULL) {
> +perror ("malloc");
> +return -1;
> +  }
> +
> +  xdrmem_create (, buf, GUESTFS_MAX_CHUNK_SIZE, XDR_ENCODE);
> +
> +  ret = xdr_guestfs_int_yara_detection (, );
> +  if (ret == 0) {
> +perror ("xdr_guestfs_int_yara_detection");
> +return -1;
> +  }
> +
> +  len = xdr_getpos ();
> +
> +  xdr_destroy ();
> +
> +  /* Send serialised tsk_detection out. */

Typo in comment.

> +  return send_file_write (buf, len);
> +}
> +
>  /* Clean up yara handle on daemon exit. */
>  void yara_finalize (void) __attribute__((destructor));
>  void
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 152c651..d9006f2 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -13280,6 +13280,16 @@ Previously loaded rules will be destroyed." };
>  shortdesc = "destroy previously loaded yara rules";
>  longdesc = "\
>  Destroy previously loaded Yara rules in order to free libguestfs resources." 
> };
> +
> +  { defaults with
> +name = "internal_yara_scan"; added = (1, 35, 15);
> +style = RErr, [Pathname "path"; FileOut "filename";], [];
> +proc_nr = Some 473;
> +visibility = VInternal;
> +optional = Some "libyara";
> +shortdesc = "scan