Pino Toscano
2016-Nov-22 09:04 UTC
Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan
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 <noxdafox@gmail.com> > --- > 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 (&xdr, buf, GUESTFS_MAX_CHUNK_SIZE, XDR_ENCODE); > + > + ret = xdr_guestfs_int_yara_detection (&xdr, &detection); > + if (ret == 0) { > + perror ("xdr_guestfs_int_yara_detection"); > + return -1; > + } > + > + len = xdr_getpos (&xdr); > + > + xdr_destroy (&xdr); > + > + /* 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 a/generator/structs.ml b/generator/structs.ml > index 029bc3a..3fa2ebc 100644 > --- a/generator/structs.ml > +++ b/generator/structs.ml > @@ -468,6 +468,15 @@ let structs = [ > ]; > s_camel_name = "TSKDirent" }; > > + (* Yara detection information. *) > + { defaults with > + s_name = "yara_detection"; > + s_cols = [ > + "name", FString; > + "rule", FString;yara_load supports loading rules already compiled, which could have a namespace set -- I guess it should be reported here as well. 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? Thanks, -- Pino Toscano
noxdafox
2016-Nov-22 17:41 UTC
Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan
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 <noxdafox@gmail.com> >> --- >> 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 (&xdr, buf, GUESTFS_MAX_CHUNK_SIZE, XDR_ENCODE); >> + >> + ret = xdr_guestfs_int_yara_detection (&xdr, &detection); >> + if (ret == 0) { >> + perror ("xdr_guestfs_int_yara_detection"); >> + return -1; >> + } >> + >> + len = xdr_getpos (&xdr); >> + >> + xdr_destroy (&xdr); >> + >> + /* 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 a/generator/structs.ml b/generator/structs.ml >> index 029bc3a..3fa2ebc 100644 >> --- a/generator/structs.ml >> +++ b/generator/structs.ml >> @@ -468,6 +468,15 @@ let structs = [ >> ]; >> s_camel_name = "TSKDirent" }; >> >> + (* Yara detection information. *) >> + { defaults with >> + s_name = "yara_detection"; >> + s_cols = [ >> + "name", FString; >> + "rule", FString; > 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.> > 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.> > Thanks, > > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs
Pino Toscano
2016-Nov-24 15:42 UTC
Re: [Libguestfs] [PATCH v2 4/6] New API: internal_yara_scan
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