All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC
@ 2017-02-09 19:17 Todor Minchev
  2017-02-09 19:17 ` [PATCH v2 1/5] Makefile: disable silent mode in Makefiles Todor Minchev
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

This is V2 of the patchset. It addresses all feedback received in V1.

Changes from previous version:

* remove default silent mode from Makefile
* extract fingerprint with -E instead of -F
* add is_rmcdb(db_blob) for database validation
* add -o option to specify an output directory to extract the database to
* remove non-alphanumeric characters from fingerprint signatures before
  using the signatures as directory names

Todor Minchev (5):
  Makefile: disable silent mode in Makefiles
  Makefile: add debug target
  rmc: Enable reading the contents of an existing fingerprint file
  rmc: remove unnecessary return variable
  rmc: add database extraction functionality

 Makefile              |  23 ++++++----
 Makefile.efi          |  10 ++--
 inc/rmc_api.h         |   9 ++++
 inc/rmcl.h            |   7 +++
 src/lib/api.c         | 106 +++++++++++++++++++++++++++++++++++++++++-
 src/lib/common/rmcl.c |  17 ++++++-
 src/rmc.c             | 124 +++++++++++++++++++++++++++++++++-----------------
 7 files changed, 237 insertions(+), 59 deletions(-)

-- 
2.11.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/5] Makefile: disable silent mode in Makefiles
  2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
@ 2017-02-09 19:17 ` Todor Minchev
  2017-02-10 18:40   ` Jianxun Zhang
  2017-02-09 19:17 ` [PATCH v2 2/5] Makefile: add debug target Todor Minchev
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

By default make will output the commands that are executed for each
target. Silent mode can be enabled with the '-s' option.

Example:
make -s

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 Makefile     | 21 +++++++++++----------
 Makefile.efi | 10 +++++-----
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 9ade775..c58047a 100644
--- a/Makefile
+++ b/Makefile
@@ -25,23 +25,24 @@ RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
 all: rmc
 
 $(ALL_OBJS): %.o: %.c
-	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
+	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
 
 librmc: $(RMC_LIB_OBJ)
-	@$(AR) rcs src/lib/$@.a $^
+	$(AR) rcs src/lib/$@.a $^
 
 rmc: $(RMC_TOOL_OBJ) librmc
-	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
+	$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
+  src/lib/librmc.a -o src/$@
 
 clean:
-	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
+	rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
 
 .PHONY: clean rmc librmc
 
 install:
-	@mkdir -p $(RMC_INSTALL_BIN_PATH)
-	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
-	@mkdir -p $(RMC_INSTALL_LIB_PATH)
-	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
-	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
-	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
+	mkdir -p $(RMC_INSTALL_BIN_PATH)
+	install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
+	mkdir -p $(RMC_INSTALL_LIB_PATH)
+	install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
+	mkdir -p $(RMC_INSTALL_HEADER_PATH)
+	install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
diff --git a/Makefile.efi b/Makefile.efi
index 3bd417d..66081d7 100644
--- a/Makefile.efi
+++ b/Makefile.efi
@@ -18,16 +18,16 @@ RMC_CFLAGS := -DRMC_EFI -Wall -I$(TOPDIR)/inc -fpic -nostdinc -nostdlib  -fno-bu
 all: librmcefi
 
 $(RMC_LIB_OBJ): %.efi.o: %.c
-	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
+	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
 
 librmcefi: $(RMC_LIB_OBJ)
-	@$(AR) rcs src/lib/$@.a $^
+	$(AR) rcs src/lib/$@.a $^
 
 clean:
-	@rm -f $(RMC_LIB_OBJ) src/lib/librmcefi.a
+	rm -f $(RMC_LIB_OBJ) src/lib/librmcefi.a
 
 .PHONY: clean librmcefi
 
 install:
-	@mkdir -p $(RMC_INSTALL_LIB_PATH)
-	@install -m 644 $(RMC_INSTALL_LIBS) $(RMC_INSTALL_LIB_PATH)
+	mkdir -p $(RMC_INSTALL_LIB_PATH)
+	install -m 644 $(RMC_INSTALL_LIBS) $(RMC_INSTALL_LIB_PATH)
-- 
2.11.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/5] Makefile: add debug target
  2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
  2017-02-09 19:17 ` [PATCH v2 1/5] Makefile: disable silent mode in Makefiles Todor Minchev
@ 2017-02-09 19:17 ` Todor Minchev
  2017-02-10 18:58   ` Jianxun Zhang
  2017-02-09 19:17 ` [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

A debug version of the rmc binary can be built by using the debug
Makefile target. This will include debug symbols and will disable
compiler optimizations.

Example:

make debug

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index c58047a..fdd936f 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,8 @@ ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
 RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
 
 all: rmc
+debug: RMC_CFLAGS += -DDEBUG -g -O0
+debug: rmc
 
 $(ALL_OBJS): %.o: %.c
 	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
-- 
2.11.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
  2017-02-09 19:17 ` [PATCH v2 1/5] Makefile: disable silent mode in Makefiles Todor Minchev
  2017-02-09 19:17 ` [PATCH v2 2/5] Makefile: add debug target Todor Minchev
@ 2017-02-09 19:17 ` Todor Minchev
  2017-02-10 19:21   ` Jianxun Zhang
  2017-02-09 19:17 ` [PATCH v2 4/5] rmc: remove unnecessary return variable Todor Minchev
  2017-02-09 19:17 ` [PATCH v2 5/5] rmc: add database extraction functionality Todor Minchev
  4 siblings, 1 reply; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

The contents of an existing fingerprint file can be read and output on
the command line with the following options:

rmc -E -f input_fingerprint_file

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 src/rmc.c | 98 ++++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 65 insertions(+), 33 deletions(-)

diff --git a/src/rmc.c b/src/rmc.c
index 062dd36..f3a2a5e 100644
--- a/src/rmc.c
+++ b/src/rmc.c
@@ -14,43 +14,47 @@
 #include <rmc_api.h>
 
 #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
-    "NOTE: Most of usages require root permission (sudo)\n" \
+    "NOTE: Most of usages require root permission (sudo)\n\n" \
     "rmc -F [-o output_fingerprint]\n" \
     "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
     "rmc -D <rmc record file list> [-o output_database]\n" \
-	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
-	"\n" \
-	"-F: generate board rmc fingerprint of board\n" \
-	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
-    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
-    "    generate record if -f is missed.\n" \
-    "-b: files to be packed in record\n" \
-	"-G: generate rmc database file with records specified in record file list\n" \
-	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
-	"-d: database file to be queried\n" \
-	"-o: path and name of output file of a specific command\n" \
-	"\n" \
-    "Examples (Steps in an order to add board support into rmc):\n" \
-    "generate board fingerprint:\n" \
-    "rmc -F\n\n" \
-    "generate a rmc record for the board with two file blobs, output to:\n" \
-    "a specified file:\n" \
-    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
-    "generate a rmc database file with records from 3 different boards:\n" \
-    "rmc -D board1_record board2_record board3_record\n\n" \
-    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
-    "to /tmp/new_audio.conf:\n" \
-    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
+    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
+  "-F: manage fingerprint file\n" \
+    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
+  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
+    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
+    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
+    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
+    "\t-b: files to be packed in record\n\n" \
+  "-G: generate rmc database file with records specified in record file list\n\n" \
+  "-B: get a file blob with specified name associated to the board rmc is\n" \
+  "running on\n" \
+    "\t-d: database file to be queried\n" \
+    "\t-o: path and name of output file of a specific command\n\n" \
+  "-E: Extract data from fingerprint file and print it to terminal\n" \
+    "\t-f: fingerprint file to extract\n\n" \
+    "Examples (Steps in an order to add board support into rmc):\n\n" \
+    "1. Generate board fingerprint:\n" \
+    "\trmc -F\n\n" \
+    "2. Generate a rmc record for the board with two file blobs and save it\n" \
+    "to a specified file:\n" \
+    "\trmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
+    "3. Generate a rmc database file with records from 3 different boards:\n" \
+    "\trmc -D board1_record board2_record board3_record\n\n" \
+    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
+    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
+    "\trmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
 
 
 #define RMC_OPT_CAP_F   (1 << 0)
 #define RMC_OPT_CAP_R   (1 << 1)
 #define RMC_OPT_CAP_D   (1 << 2)
 #define RMC_OPT_CAP_B   (1 << 3)
-#define RMC_OPT_F       (1 << 4)
-#define RMC_OPT_O       (1 << 5)
-#define RMC_OPT_B       (1 << 6)
-#define RMC_OPT_D       (1 << 7)
+#define RMC_OPT_CAP_E   (1 << 4)
+#define RMC_OPT_F       (1 << 5)
+#define RMC_OPT_O       (1 << 6)
+#define RMC_OPT_B       (1 << 7)
+#define RMC_OPT_D       (1 << 8)
 
 static void usage () {
     fprintf(stdout, USAGE);
@@ -78,7 +82,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
 static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
     int i;
     int first = 0;
-
+    /* TODO - do we need to open/close file multiple times to write each field */
     for (i = 0; i < RMC_FINGER_NUM; i++) {
         if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
             return 1;
@@ -311,7 +315,7 @@ int main(int argc, char **argv){
     /* parse options */
     opterr = 0;
 
-    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
+    while ((c = getopt(argc, argv, "FRED:B:b:f:o:d:")) != -1)
         switch (c) {
         case 'F':
             options |= RMC_OPT_CAP_F;
@@ -319,6 +323,9 @@ int main(int argc, char **argv){
         case 'R':
             options |= RMC_OPT_CAP_R;
             break;
+        case 'E':
+            options |= RMC_OPT_CAP_E;
+            break;
         case 'D':
             /* we don't know number of arguments for this option at this point,
              * allocate array with argc which is bigger than needed. But we also
@@ -388,7 +395,8 @@ int main(int argc, char **argv){
             break;
         case '?':
             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
-                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
+                    optopt == 'E' ||  optopt == 'b' || optopt == 'f' || \
+                    optopt == 'o' || optopt == 'd')
                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
             else if (isprint(optopt))
                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
@@ -421,6 +429,13 @@ int main(int argc, char **argv){
         return 1;
     }
 
+    /* sanity check for -E */
+    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F))) {
+        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name>\n\n");
+        usage();
+        return 1;
+    }
+
     /* sanity check for -B */
     if ((options & RMC_OPT_CAP_B) && (!(options & RMC_OPT_D) || !(options & RMC_OPT_O))) {
         fprintf(stderr, "\nWRONG: -B requires -d and -o\n\n");
@@ -445,10 +460,28 @@ int main(int argc, char **argv){
             rmc_free_file(&file);
             goto main_free;
         }
-
         rmc_free_file(&file);
     }
 
+    if (options & RMC_OPT_CAP_E) {
+        /* print fingerpring file to console*/
+        if (options & RMC_OPT_F) {
+            rmc_fingerprint_t fp;
+            /* read fingerprint file*/
+            if (input_fingerprint != NULL) {
+                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
+                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
+                    input_fingerprint);
+                    goto main_free;
+                }
+                printf("Successfully read fingerprint from %s \n", input_fingerprint);
+                dump_fingerprint(&fp);
+            }else {
+                printf("Fingerprint file not provided! Exiting.\n");
+            }
+        }
+    }
+
     /* generate RMC database file */
     if (options & RMC_OPT_CAP_D) {
         int record_idx = 0;
@@ -580,7 +613,6 @@ int main(int argc, char **argv){
             rmc_free_fingerprint(&fingerprint);
             goto main_free;
         }
-
         rmc_free_fingerprint(&fingerprint);
     }
 
-- 
2.11.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/5] rmc: remove unnecessary return variable
  2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
                   ` (2 preceding siblings ...)
  2017-02-09 19:17 ` [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
@ 2017-02-09 19:17 ` Todor Minchev
  2017-02-10 19:22   ` Jianxun Zhang
  2017-02-09 19:17 ` [PATCH v2 5/5] rmc: add database extraction functionality Todor Minchev
  4 siblings, 1 reply; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 src/rmc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/rmc.c b/src/rmc.c
index f3a2a5e..b5c7847 100644
--- a/src/rmc.c
+++ b/src/rmc.c
@@ -218,7 +218,6 @@ read_fp_done:
 static rmc_file_t *read_policy_file(char *pathname, int type) {
     rmc_file_t *tmp = NULL;
     rmc_size_t policy_len = 0;
-    int ret;
     char *path_token;
 
     if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
@@ -230,8 +229,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
     tmp->next = NULL;
 
     if (type == RMC_GENERIC_FILE) {
-        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
-        if (ret) {
+        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
             fprintf(stderr, "Failed to read file %s\n\n", pathname);
             free(tmp);
             return NULL;
-- 
2.11.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/5] rmc: add database extraction functionality
  2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
                   ` (3 preceding siblings ...)
  2017-02-09 19:17 ` [PATCH v2 4/5] rmc: remove unnecessary return variable Todor Minchev
@ 2017-02-09 19:17 ` Todor Minchev
  2017-02-10 19:52   ` Jianxun Zhang
  4 siblings, 1 reply; 13+ messages in thread
From: Todor Minchev @ 2017-02-09 19:17 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

The contents of an existing database file can be extracted with the -E option.
By default the top level of the directory tree is rmc_db_dump, an alternative
path can be specified with the -o option. The file blobs corresponding
to a given record will be saved in a separate sub-directory. The sub-directory
name of each record is the signature corresponding to the fingerprint for that
record with all non-alphanumeric characters stripped.

Example:
./src/rmc -E -d rmc.db -o output/directory/

Successfully extracted rmc.db

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 inc/rmc_api.h         |   9 +++++
 inc/rmcl.h            |   7 ++++
 src/lib/api.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++-
 src/lib/common/rmcl.c |  17 +++++++-
 src/rmc.c             |  30 +++++++++-----
 5 files changed, 157 insertions(+), 12 deletions(-)

diff --git a/inc/rmc_api.h b/inc/rmc_api.h
index a484389..2f8c978 100644
--- a/inc/rmc_api.h
+++ b/inc/rmc_api.h
@@ -109,6 +109,15 @@ extern int read_file(const char *pathname, char **data, rmc_size_t* len);
  */
 int write_file(const char *pathname, void *data, rmc_size_t len, int append);
 
+/* extract the contents of a database file and store the files corresponding to
+ * each record in a separate directory. The name of each directory is the signature
+ * of the fingerpring for that record with all non-alphanumeric characters stripped
+ * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
+ * (in) output_path: A directory path to extract the database to
+ * return: 0 on success, non-zero on failure.
+ */
+int dump_db(char *db_pathname, char *output_path) ;
+
 #else
 /* 2 - API for UEFI context */
 
diff --git a/inc/rmcl.h b/inc/rmcl.h
index 5bbad42..471ebfe 100644
--- a/inc/rmcl.h
+++ b/inc/rmcl.h
@@ -164,4 +164,11 @@ extern int rmcl_generate_db(rmc_record_file_t *record_files, rmc_uint8_t **rmc_d
  */
 extern int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy);
 
+/*
+ * Check if db_blob has a valid rmc database signature
+ *
+ * return 0 if db_blob has a valid signature or non-zero otherwise
+ */
+int is_rmcdb(rmc_uint8_t *db_blob);
+
 #endif /* INC_RMCL_H_ */
diff --git a/src/lib/api.c b/src/lib/api.c
index 0adb390..56746a4 100644
--- a/src/lib/api.c
+++ b/src/lib/api.c
@@ -3,6 +3,7 @@
  * RMC API implementation for Linux user space
  */
 
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <unistd.h>
 #include <errno.h>
@@ -10,12 +11,15 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <sys/mman.h>
+#include <dirent.h>
+#include <ctype.h>
 
 #include <rmcl.h>
 #include <rsmp.h>
 
-#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
-#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
+#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
+#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
+#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
 
 int read_file(const char *pathname, char **data, rmc_size_t* len) {
     int fd = -1;
@@ -325,3 +329,101 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
 
     return ret;
 }
+
+void remove_non_alphanum(char *in) {
+    char c;
+    unsigned long i = 0, j = 0;
+
+    while ((c = in[i++]) != '\0'){
+        if (isalnum(c))
+            in[j++] = c;
+    }
+    in[j] = '\0';
+}
+
+int dump_db(char *db_pathname, char *output_path) {
+    rmc_meta_header_t meta_header;
+    rmc_db_header_t *db_header = NULL;
+    rmc_record_header_t record_header;
+    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
+    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
+    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
+    rmc_file_t file;
+    char *out_dir = NULL, *out_name = NULL, *cmd = NULL, *tmp_dir_name = NULL;
+    rmc_size_t db_len = 0;
+    rmc_uint8_t *rmc_db = NULL;
+    DIR *tmp_dir = NULL;
+
+    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
+        fprintf(stderr, "Failed to read database file\n\n");
+        return 1;
+    }
+
+    db_header = (rmc_db_header_t *)rmc_db;
+
+    /* sanity check of db */
+    if (is_rmcdb(rmc_db))
+        return 1;
+
+    /* query the meta. idx: start of record */
+    record_idx = sizeof(rmc_db_header_t);
+    while (record_idx < db_header->length) {
+        memcpy(&record_header, rmc_db + record_idx,
+            sizeof(rmc_record_header_t));
+
+        /* directory name is fingerprint signature with stripped special chars*/
+        asprintf(&tmp_dir_name, "%s", record_header.signature.raw);
+        remove_non_alphanum(tmp_dir_name);
+        if(output_path)
+            asprintf(&out_dir, "%s/%s/", output_path, tmp_dir_name);
+        else
+            asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, tmp_dir_name);
+        if ((tmp_dir = opendir(out_dir))) {
+            /* Directory exists */
+            closedir(tmp_dir);
+            free(tmp_dir_name);
+        } else if (ENOENT == errno) {
+            /* Directory does not exist, try to create it. */
+            asprintf(&cmd, "mkdir -p %s", out_dir);
+            if(system(cmd) == -1) {
+                fprintf(stderr, "Failed to create %s directory\n\n", out_dir);
+                free(out_dir);
+                free(tmp_dir_name);
+                free(cmd);
+                return 1;
+            }
+            free(cmd);
+        } else {
+            /* Some other error occured */
+            free(out_dir);
+            free(tmp_dir_name);
+            free(cmd);
+            return 1;
+        }
+
+        /* find meta */
+        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
+            meta_idx < record_idx + record_header.length;) {
+            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
+            file_idx = meta_idx + sizeof(rmc_meta_header_t);
+            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
+            file.blob = &rmc_db[file_idx + name_len];
+            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
+                name_len;
+            file.next = NULL;
+            file.type = RMC_GENERIC_FILE;
+            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
+            /* write file to dump directory */
+            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
+                return 1;
+
+            /* next meta */
+            meta_idx += meta_header.length;
+            free(out_name);
+        }
+        /* next record */
+        record_idx += record_header.length;
+        free(out_dir);
+    }
+    return 0;
+}
diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
index 67622a0..58a4a52 100644
--- a/src/lib/common/rmcl.c
+++ b/src/lib/common/rmcl.c
@@ -193,6 +193,21 @@ static int match_record(rmc_record_header_t *r, rmc_signature_t* sig) {
     return strncmp((const char *)r->signature.raw, (const char *)sig->raw, sizeof(r->signature.raw));
 }
 
+int is_rmcdb(rmc_uint8_t *db_blob) {
+    rmc_db_header_t *db_header = NULL;
+
+    if (db_blob == NULL)
+        return 1;
+
+    db_header = (rmc_db_header_t *)db_blob;
+
+    /* sanity check of db */
+    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
+        return 1;
+    else
+        return 0;
+}
+
 int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy) {
     rmc_meta_header_t meta_header;
     rmc_db_header_t *db_header = NULL;
@@ -211,7 +226,7 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
     db_header = (rmc_db_header_t *)rmc_db;
 
     /* sanity check of db */
-    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
+    if (is_rmcdb(rmc_db))
         return 1;
 
     /* calculate signature of fingerprint */
diff --git a/src/rmc.c b/src/rmc.c
index b5c7847..0740223 100644
--- a/src/rmc.c
+++ b/src/rmc.c
@@ -31,8 +31,10 @@
   "running on\n" \
     "\t-d: database file to be queried\n" \
     "\t-o: path and name of output file of a specific command\n\n" \
-  "-E: Extract data from fingerprint file and print it to terminal\n" \
-    "\t-f: fingerprint file to extract\n\n" \
+  "-E: Extract data from fingerprint file or database\n" \
+    "\t-f: fingerprint file to extract\n" \
+    "\t-d: database file to extract\n" \
+    "\t-o: directory to extract the database to\n\n" \
     "Examples (Steps in an order to add board support into rmc):\n\n" \
     "1. Generate board fingerprint:\n" \
     "\trmc -F\n\n" \
@@ -408,12 +410,14 @@ int main(int argc, char **argv){
 
     /* sanity check for -o */
     if (options & RMC_OPT_O) {
-        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R | RMC_OPT_CAP_F | RMC_OPT_CAP_B);
+        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R |
+            RMC_OPT_CAP_F | RMC_OPT_CAP_B | RMC_OPT_CAP_E);
         if (!(opt_o)) {
-            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -R or -F\n\n");
+            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -E, -R or -F\n\n");
             usage();
             return 1;
-        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R && opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B) {
+        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R &&
+            opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B  && opt_o != RMC_OPT_CAP_E) {
             fprintf(stderr, "\nWRONG: Option -o can be applied with only one of -B, -D, -R and -F\n\n");
             usage();
             return 1;
@@ -428,8 +432,8 @@ int main(int argc, char **argv){
     }
 
     /* sanity check for -E */
-    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F))) {
-        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name>\n\n");
+    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F) && !(options & RMC_OPT_D))) {
+        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name> or -d <database file name>\n\n");
         usage();
         return 1;
     }
@@ -446,7 +450,8 @@ int main(int argc, char **argv){
         rmc_file_t file;
 
         if (!output_path) {
-            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
+            fprintf(stderr, "-B internal error, with -o but no output \
+                pathname specified\n\n");
             goto main_free;
         }
 
@@ -454,7 +459,8 @@ int main(int argc, char **argv){
             goto main_free;
 
         if (write_file(output_path, file.blob, file.blob_len, 0)) {
-            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
+            fprintf(stderr, "-B failed to write file %s to %s\n\n",
+                input_blob_name, output_path);
             rmc_free_file(&file);
             goto main_free;
         }
@@ -477,6 +483,12 @@ int main(int argc, char **argv){
             }else {
                 printf("Fingerprint file not provided! Exiting.\n");
             }
+        } else if (options & RMC_OPT_D) {
+            if(dump_db(input_db_path_d, output_path)) {
+               fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
+               goto main_free;
+            } else
+               printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
         }
     }
 
-- 
2.11.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] Makefile: disable silent mode in Makefiles
  2017-02-09 19:17 ` [PATCH v2 1/5] Makefile: disable silent mode in Makefiles Todor Minchev
@ 2017-02-10 18:40   ` Jianxun Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: Jianxun Zhang @ 2017-02-10 18:40 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Review +1,
Thanks to address my comment

> On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> By default make will output the commands that are executed for each
> target. Silent mode can be enabled with the '-s' option.
> 
> Example:
> make -s
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> Makefile     | 21 +++++++++++----------
> Makefile.efi | 10 +++++-----
> 2 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9ade775..c58047a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -25,23 +25,24 @@ RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> all: rmc
> 
> $(ALL_OBJS): %.o: %.c
> -	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> +	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> 
> librmc: $(RMC_LIB_OBJ)
> -	@$(AR) rcs src/lib/$@.a $^
> +	$(AR) rcs src/lib/$@.a $^
> 
> rmc: $(RMC_TOOL_OBJ) librmc
> -	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
> +	$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
> +  src/lib/librmc.a -o src/$@
> 
> clean:
> -	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> +	rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> 
> .PHONY: clean rmc librmc
> 
> install:
> -	@mkdir -p $(RMC_INSTALL_BIN_PATH)
> -	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> -	@mkdir -p $(RMC_INSTALL_LIB_PATH)
> -	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> -	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
> -	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> +	mkdir -p $(RMC_INSTALL_BIN_PATH)
> +	install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> +	mkdir -p $(RMC_INSTALL_LIB_PATH)
> +	install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> +	mkdir -p $(RMC_INSTALL_HEADER_PATH)
> +	install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> diff --git a/Makefile.efi b/Makefile.efi
> index 3bd417d..66081d7 100644
> --- a/Makefile.efi
> +++ b/Makefile.efi
> @@ -18,16 +18,16 @@ RMC_CFLAGS := -DRMC_EFI -Wall -I$(TOPDIR)/inc -fpic -nostdinc -nostdlib  -fno-bu
> all: librmcefi
> 
> $(RMC_LIB_OBJ): %.efi.o: %.c
> -	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> +	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> 
> librmcefi: $(RMC_LIB_OBJ)
> -	@$(AR) rcs src/lib/$@.a $^
> +	$(AR) rcs src/lib/$@.a $^
> 
> clean:
> -	@rm -f $(RMC_LIB_OBJ) src/lib/librmcefi.a
> +	rm -f $(RMC_LIB_OBJ) src/lib/librmcefi.a
> 
> .PHONY: clean librmcefi
> 
> install:
> -	@mkdir -p $(RMC_INSTALL_LIB_PATH)
> -	@install -m 644 $(RMC_INSTALL_LIBS) $(RMC_INSTALL_LIB_PATH)
> +	mkdir -p $(RMC_INSTALL_LIB_PATH)
> +	install -m 644 $(RMC_INSTALL_LIBS) $(RMC_INSTALL_LIB_PATH)
> -- 
> 2.11.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] Makefile: add debug target
  2017-02-09 19:17 ` [PATCH v2 2/5] Makefile: add debug target Todor Minchev
@ 2017-02-10 18:58   ` Jianxun Zhang
  2017-02-14 17:33     ` Todor Minchev
  0 siblings, 1 reply; 13+ messages in thread
From: Jianxun Zhang @ 2017-02-10 18:58 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto


> On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> A debug version of the rmc binary can be built by using the debug
> Makefile target. This will include debug symbols and will disable
> compiler optimizations.
> 
> Example:
> 
> make debug
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> Makefile | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index c58047a..fdd936f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -23,6 +23,8 @@ ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
> RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> 
> all: rmc
> +debug: RMC_CFLAGS += -DDEBUG -g -O0
> +debug: rmc
> 
I missed your feedback of V1, just recap it here also with my comment:

- "Yup, the above will have the same effect as 'make debug'.
I thought that 'make debug' might be a convenient way to build a debug
binary? Do you think it makes sense to keep this extra target for
convenience since it doesn't affect the other usages of make?"

You could have to come back to CFLAGS when the a hard coded debug flags is not enough.

And, why pass -DDEBUG in RMC_CFLAGS when invoking gcc to compile rmc? I don’t see this macro DEBUG in the current rmc code.

Thanks
> $(ALL_OBJS): %.o: %.c
> 	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> -- 
> 2.11.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-09 19:17 ` [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
@ 2017-02-10 19:21   ` Jianxun Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: Jianxun Zhang @ 2017-02-10 19:21 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto


> On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> The contents of an existing fingerprint file can be read and output on
> the command line with the following options:
> 
> rmc -E -f input_fingerprint_file
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> src/rmc.c | 98 ++++++++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 65 insertions(+), 33 deletions(-)
> 
> diff --git a/src/rmc.c b/src/rmc.c
> index 062dd36..f3a2a5e 100644
> --- a/src/rmc.c
> +++ b/src/rmc.c
> @@ -14,43 +14,47 @@
> #include <rmc_api.h>
> 
> #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
> -    "NOTE: Most of usages require root permission (sudo)\n" \
> +    "NOTE: Most of usages require root permission (sudo)\n\n" \
>     "rmc -F [-o output_fingerprint]\n" \
>     "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
>     "rmc -D <rmc record file list> [-o output_database]\n" \
> -	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
> -	"\n" \
> -	"-F: generate board rmc fingerprint of board\n" \
> -	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> -    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
> -    "    generate record if -f is missed.\n" \
> -    "-b: files to be packed in record\n" \
> -	"-G: generate rmc database file with records specified in record file list\n" \
> -	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
> -	"-d: database file to be queried\n" \
> -	"-o: path and name of output file of a specific command\n" \
> -	"\n" \
> -    "Examples (Steps in an order to add board support into rmc):\n" \
> -    "generate board fingerprint:\n" \
> -    "rmc -F\n\n" \
> -    "generate a rmc record for the board with two file blobs, output to:\n" \
> -    "a specified file:\n" \
> -    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> -    "generate a rmc database file with records from 3 different boards:\n" \
> -    "rmc -D board1_record board2_record board3_record\n\n" \
> -    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
> -    "to /tmp/new_audio.conf:\n" \
> -    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> +    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
> +  "-F: manage fingerprint file\n" \
> +    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
> +  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> +    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
> +    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
> +    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
> +    "\t-b: files to be packed in record\n\n" \
> +  "-G: generate rmc database file with records specified in record file list\n\n" \
> +  "-B: get a file blob with specified name associated to the board rmc is\n" \
> +  "running on\n" \
> +    "\t-d: database file to be queried\n" \
> +    "\t-o: path and name of output file of a specific command\n\n" \
> +  "-E: Extract data from fingerprint file and print it to terminal\n" \
> +    "\t-f: fingerprint file to extract\n\n" \
> +    "Examples (Steps in an order to add board support into rmc):\n\n" \
> +    "1. Generate board fingerprint:\n" \
> +    "\trmc -F\n\n" \
> +    "2. Generate a rmc record for the board with two file blobs and save it\n" \
> +    "to a specified file:\n" \
> +    "\trmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> +    "3. Generate a rmc database file with records from 3 different boards:\n" \
> +    "\trmc -D board1_record board2_record board3_record\n\n" \
> +    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
> +    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
> +    "\trmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> 
> 
> #define RMC_OPT_CAP_F   (1 << 0)
> #define RMC_OPT_CAP_R   (1 << 1)
> #define RMC_OPT_CAP_D   (1 << 2)
> #define RMC_OPT_CAP_B   (1 << 3)
> -#define RMC_OPT_F       (1 << 4)
> -#define RMC_OPT_O       (1 << 5)
> -#define RMC_OPT_B       (1 << 6)
> -#define RMC_OPT_D       (1 << 7)
> +#define RMC_OPT_CAP_E   (1 << 4)
> +#define RMC_OPT_F       (1 << 5)
> +#define RMC_OPT_O       (1 << 6)
> +#define RMC_OPT_B       (1 << 7)
> +#define RMC_OPT_D       (1 << 8)
> 
> static void usage () {
>     fprintf(stdout, USAGE);
> @@ -78,7 +82,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
> static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
>     int i;
>     int first = 0;
> -
> +    /* TODO - do we need to open/close file multiple times to write each field */
>     for (i = 0; i < RMC_FINGER_NUM; i++) {
>         if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
>             return 1;
> @@ -311,7 +315,7 @@ int main(int argc, char **argv){
>     /* parse options */
>     opterr = 0;
> 
> -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
> +    while ((c = getopt(argc, argv, "FRED:B:b:f:o:d:")) != -1)
>         switch (c) {
>         case 'F':
>             options |= RMC_OPT_CAP_F;
> @@ -319,6 +323,9 @@ int main(int argc, char **argv){
>         case 'R':
>             options |= RMC_OPT_CAP_R;
>             break;
> +        case 'E':
> +            options |= RMC_OPT_CAP_E;
> +            break;
>         case 'D':
>             /* we don't know number of arguments for this option at this point,
>              * allocate array with argc which is bigger than needed. But we also
> @@ -388,7 +395,8 @@ int main(int argc, char **argv){
>             break;
>         case '?':
>             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
> -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
> +                    optopt == 'E' ||  optopt == 'b' || optopt == 'f' || \
> +                    optopt == 'o' || optopt == 'd')
>                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
>             else if (isprint(optopt))
>                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
> @@ -421,6 +429,13 @@ int main(int argc, char **argv){
>         return 1;
>     }
> 
> +    /* sanity check for -E */
> +    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F))) {
> +        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name>\n\n");
> +        usage();
> +        return 1;
> +    }
> +
>     /* sanity check for -B */
>     if ((options & RMC_OPT_CAP_B) && (!(options & RMC_OPT_D) || !(options & RMC_OPT_O))) {
>         fprintf(stderr, "\nWRONG: -B requires -d and -o\n\n");
> @@ -445,10 +460,28 @@ int main(int argc, char **argv){
>             rmc_free_file(&file);
>             goto main_free;
>         }
> -
>         rmc_free_file(&file);
>     }
> 
> +    if (options & RMC_OPT_CAP_E) {
> +        /* print fingerpring file to console*/
> +        if (options & RMC_OPT_F) {
> +            rmc_fingerprint_t fp;
> +            /* read fingerprint file*/
> +            if (input_fingerprint != NULL) {
> +                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
> +                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
> +                    input_fingerprint);
> +                    goto main_free;
> +                }
> +                printf("Successfully read fingerprint from %s \n", input_fingerprint);
I don’t think this is a blocking issue or worthy a V2. You may want to remove this line before merging the patch.

The message will be in the same console of the following actual dump. Anyone/scripts wants to redirect and then manipulate the dump into a file could have to discard this first line first.

A trivial point anyway...

> +                dump_fingerprint(&fp);
> +            }else {
> +                printf("Fingerprint file not provided! Exiting.\n");
> +            }
> +        }
> +    }
> +
>     /* generate RMC database file */
>     if (options & RMC_OPT_CAP_D) {
>         int record_idx = 0;
> @@ -580,7 +613,6 @@ int main(int argc, char **argv){
>             rmc_free_fingerprint(&fingerprint);
>             goto main_free;
>         }
> -
>         rmc_free_fingerprint(&fingerprint);
>     }
> 
> -- 
> 2.11.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 4/5] rmc: remove unnecessary return variable
  2017-02-09 19:17 ` [PATCH v2 4/5] rmc: remove unnecessary return variable Todor Minchev
@ 2017-02-10 19:22   ` Jianxun Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: Jianxun Zhang @ 2017-02-10 19:22 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Nice. Review +1

> On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> src/rmc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/rmc.c b/src/rmc.c
> index f3a2a5e..b5c7847 100644
> --- a/src/rmc.c
> +++ b/src/rmc.c
> @@ -218,7 +218,6 @@ read_fp_done:
> static rmc_file_t *read_policy_file(char *pathname, int type) {
>     rmc_file_t *tmp = NULL;
>     rmc_size_t policy_len = 0;
> -    int ret;
>     char *path_token;
> 
>     if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
> @@ -230,8 +229,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
>     tmp->next = NULL;
> 
>     if (type == RMC_GENERIC_FILE) {
> -        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
> -        if (ret) {
> +        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
>             fprintf(stderr, "Failed to read file %s\n\n", pathname);
>             free(tmp);
>             return NULL;
> -- 
> 2.11.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 5/5] rmc: add database extraction functionality
  2017-02-09 19:17 ` [PATCH v2 5/5] rmc: add database extraction functionality Todor Minchev
@ 2017-02-10 19:52   ` Jianxun Zhang
  2017-02-14 17:50     ` Todor Minchev
  0 siblings, 1 reply; 13+ messages in thread
From: Jianxun Zhang @ 2017-02-10 19:52 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Todor,
Appreciate the V2 series. I have only one real concern on using system() in this patch and the whole series. The other comments are more for corner cases which have less impact.

I could miss some of your feedbacks in V1 threads. Sorry for not to point out things earlier before the V2.

Thanks

> On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> The contents of an existing database file can be extracted with the -E option.
> By default the top level of the directory tree is rmc_db_dump, an alternative
> path can be specified with the -o option. The file blobs corresponding
> to a given record will be saved in a separate sub-directory. The sub-directory
> name of each record is the signature corresponding to the fingerprint for that
> record with all non-alphanumeric characters stripped.
> 
> Example:
> ./src/rmc -E -d rmc.db -o output/directory/
> 
> Successfully extracted rmc.db
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> inc/rmc_api.h         |   9 +++++
> inc/rmcl.h            |   7 ++++
> src/lib/api.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++-
> src/lib/common/rmcl.c |  17 +++++++-
> src/rmc.c             |  30 +++++++++-----
> 5 files changed, 157 insertions(+), 12 deletions(-)
> 
> diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> index a484389..2f8c978 100644
> --- a/inc/rmc_api.h
> +++ b/inc/rmc_api.h
> @@ -109,6 +109,15 @@ extern int read_file(const char *pathname, char **data, rmc_size_t* len);
>  */
> int write_file(const char *pathname, void *data, rmc_size_t len, int append);
> 
> +/* extract the contents of a database file and store the files corresponding to
> + * each record in a separate directory. The name of each directory is the signature
> + * of the fingerpring for that record with all non-alphanumeric characters stripped
> + * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
> + * (in) output_path: A directory path to extract the database to
> + * return: 0 on success, non-zero on failure.
> + */
> +int dump_db(char *db_pathname, char *output_path) ;
> +
> #else
> /* 2 - API for UEFI context */
> 
> diff --git a/inc/rmcl.h b/inc/rmcl.h
> index 5bbad42..471ebfe 100644
> --- a/inc/rmcl.h
> +++ b/inc/rmcl.h
> @@ -164,4 +164,11 @@ extern int rmcl_generate_db(rmc_record_file_t *record_files, rmc_uint8_t **rmc_d
>  */
> extern int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy);
> 
> +/*
> + * Check if db_blob has a valid rmc database signature
> + *
> + * return 0 if db_blob has a valid signature or non-zero otherwise
> + */
> +int is_rmcdb(rmc_uint8_t *db_blob);
> +
> #endif /* INC_RMCL_H_ */
> diff --git a/src/lib/api.c b/src/lib/api.c
> index 0adb390..56746a4 100644
> --- a/src/lib/api.c
> +++ b/src/lib/api.c
> @@ -3,6 +3,7 @@
>  * RMC API implementation for Linux user space
>  */
> 
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <unistd.h>
> #include <errno.h>
> @@ -10,12 +11,15 @@
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <sys/mman.h>
> +#include <dirent.h>
> +#include <ctype.h>
> 
> #include <rmcl.h>
> #include <rsmp.h>
> 
> -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> -#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
> +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> +#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
> +#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
> 
> int read_file(const char *pathname, char **data, rmc_size_t* len) {
>     int fd = -1;
> @@ -325,3 +329,101 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
> 
>     return ret;
> }
> +
> +void remove_non_alphanum(char *in) {
> +    char c;
> +    unsigned long i = 0, j = 0;
> +
> +    while ((c = in[i++]) != '\0'){
> +        if (isalnum(c))
> +            in[j++] = c;
I think this will work in the almost all the real cases. A corner case, though very rare, is two fingerprints differ each other _only_ by non alphnum characters. The dumps of two different fingerprints will be wrongly in the same dir as a result. (We can’t dictate what FW engineers put into bios)

Maybe in the future we could use ASCII code of hex values of a signature which is a field with a fixed length as dir name?

> +    }
> +    in[j] = '\0';
> +}
> +
> +int dump_db(char *db_pathname, char *output_path) {
> +    rmc_meta_header_t meta_header;
> +    rmc_db_header_t *db_header = NULL;
> +    rmc_record_header_t record_header;
> +    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> +    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
> +    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
> +    rmc_file_t file;
> +    char *out_dir = NULL, *out_name = NULL, *cmd = NULL, *tmp_dir_name = NULL;
> +    rmc_size_t db_len = 0;
> +    rmc_uint8_t *rmc_db = NULL;
> +    DIR *tmp_dir = NULL;
> +
> +    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
> +        fprintf(stderr, "Failed to read database file\n\n");
> +        return 1;
> +    }
> +
> +    db_header = (rmc_db_header_t *)rmc_db;
> +
> +    /* sanity check of db */
> +    if (is_rmcdb(rmc_db))
> +        return 1;
> +
> +    /* query the meta. idx: start of record */
> +    record_idx = sizeof(rmc_db_header_t);
> +    while (record_idx < db_header->length) {
> +        memcpy(&record_header, rmc_db + record_idx,
> +            sizeof(rmc_record_header_t));
> +
> +        /* directory name is fingerprint signature with stripped special chars*/
> +        asprintf(&tmp_dir_name, "%s", record_header.signature.raw);
> +        remove_non_alphanum(tmp_dir_name);
The stripped tmp_dir_name could be a null string when all of its values are non alphanum. The following logic could behave unexpected in such case.

But resolving this issue requires more logic, refer to my first comment too.
> +        if(output_path)
> +            asprintf(&out_dir, "%s/%s/", output_path, tmp_dir_name);
> +        else
> +            asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, tmp_dir_name);
> +        if ((tmp_dir = opendir(out_dir))) {
> +            /* Directory exists */
> +            closedir(tmp_dir);
> +            free(tmp_dir_name);
> +        } else if (ENOENT == errno) {
> +            /* Directory does not exist, try to create it. */
> +            asprintf(&cmd, "mkdir -p %s", out_dir);
> +            if(system(cmd) == -1) {
I think this is a blocker. Generally the system() is overkilling here and it causes security concerns. Why do you change it from mkdir() in V1?

> +                fprintf(stderr, "Failed to create %s directory\n\n", out_dir);
> +                free(out_dir);
> +                free(tmp_dir_name);
> +                free(cmd);
> +                return 1;
> +            }
> +            free(cmd);
> +        } else {
> +            /* Some other error occured */
> +            free(out_dir);
> +            free(tmp_dir_name);
> +            free(cmd);
> +            return 1;
> +        }
> +
> +        /* find meta */
> +        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
> +            meta_idx < record_idx + record_header.length;) {
> +            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
> +            file_idx = meta_idx + sizeof(rmc_meta_header_t);
> +            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
> +            file.blob = &rmc_db[file_idx + name_len];
> +            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
> +                name_len;
> +            file.next = NULL;
> +            file.type = RMC_GENERIC_FILE;
> +            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
> +            /* write file to dump directory */
> +            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
> +                return 1;
> +
> +            /* next meta */
> +            meta_idx += meta_header.length;
> +            free(out_name);
> +        }
> +        /* next record */
> +        record_idx += record_header.length;
> +        free(out_dir);
> +    }
> +    return 0;
> +}
> diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
> index 67622a0..58a4a52 100644
> --- a/src/lib/common/rmcl.c
> +++ b/src/lib/common/rmcl.c
> @@ -193,6 +193,21 @@ static int match_record(rmc_record_header_t *r, rmc_signature_t* sig) {
>     return strncmp((const char *)r->signature.raw, (const char *)sig->raw, sizeof(r->signature.raw));
> }
> 
> +int is_rmcdb(rmc_uint8_t *db_blob) {
> +    rmc_db_header_t *db_header = NULL;
> +
> +    if (db_blob == NULL)
> +        return 1;
> +
> +    db_header = (rmc_db_header_t *)db_blob;
> +
> +    /* sanity check of db */
> +    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> +        return 1;
> +    else
> +        return 0;
> +}
> +
> int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy) {
>     rmc_meta_header_t meta_header;
>     rmc_db_header_t *db_header = NULL;
> @@ -211,7 +226,7 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
>     db_header = (rmc_db_header_t *)rmc_db;
> 
>     /* sanity check of db */
> -    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> +    if (is_rmcdb(rmc_db))
>         return 1;
> 
>     /* calculate signature of fingerprint */
> diff --git a/src/rmc.c b/src/rmc.c
> index b5c7847..0740223 100644
> --- a/src/rmc.c
> +++ b/src/rmc.c
> @@ -31,8 +31,10 @@
>   "running on\n" \
>     "\t-d: database file to be queried\n" \
>     "\t-o: path and name of output file of a specific command\n\n" \
> -  "-E: Extract data from fingerprint file and print it to terminal\n" \
> -    "\t-f: fingerprint file to extract\n\n" \
> +  "-E: Extract data from fingerprint file or database\n" \
> +    "\t-f: fingerprint file to extract\n" \
> +    "\t-d: database file to extract\n" \
> +    "\t-o: directory to extract the database to\n\n" \
>     "Examples (Steps in an order to add board support into rmc):\n\n" \
>     "1. Generate board fingerprint:\n" \
>     "\trmc -F\n\n" \
> @@ -408,12 +410,14 @@ int main(int argc, char **argv){
> 
>     /* sanity check for -o */
>     if (options & RMC_OPT_O) {
> -        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R | RMC_OPT_CAP_F | RMC_OPT_CAP_B);
> +        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R |
> +            RMC_OPT_CAP_F | RMC_OPT_CAP_B | RMC_OPT_CAP_E);
>         if (!(opt_o)) {
> -            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -R or -F\n\n");
> +            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -E, -R or -F\n\n");
>             usage();
>             return 1;
> -        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R && opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B) {
> +        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R &&
> +            opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B  && opt_o != RMC_OPT_CAP_E) {
>             fprintf(stderr, "\nWRONG: Option -o can be applied with only one of -B, -D, -R and -F\n\n");
>             usage();
>             return 1;
> @@ -428,8 +432,8 @@ int main(int argc, char **argv){
>     }
> 
>     /* sanity check for -E */
> -    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F))) {
> -        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name>\n\n");
> +    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F) && !(options & RMC_OPT_D))) {
> +        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name> or -d <database file name>\n\n");
>         usage();
>         return 1;
>     }
> @@ -446,7 +450,8 @@ int main(int argc, char **argv){
>         rmc_file_t file;
> 
>         if (!output_path) {
> -            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
> +            fprintf(stderr, "-B internal error, with -o but no output \
> +                pathname specified\n\n");
>             goto main_free;
>         }
> 
> @@ -454,7 +459,8 @@ int main(int argc, char **argv){
>             goto main_free;
> 
>         if (write_file(output_path, file.blob, file.blob_len, 0)) {
> -            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
> +            fprintf(stderr, "-B failed to write file %s to %s\n\n",
> +                input_blob_name, output_path);
>             rmc_free_file(&file);
>             goto main_free;
>         }
> @@ -477,6 +483,12 @@ int main(int argc, char **argv){
>             }else {
>                 printf("Fingerprint file not provided! Exiting.\n");
>             }
> +        } else if (options & RMC_OPT_D) {
> +            if(dump_db(input_db_path_d, output_path)) {
> +               fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
> +               goto main_free;
> +            } else
> +               printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
>         }
>     }
> 
> -- 
> 2.11.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] Makefile: add debug target
  2017-02-10 18:58   ` Jianxun Zhang
@ 2017-02-14 17:33     ` Todor Minchev
  0 siblings, 0 replies; 13+ messages in thread
From: Todor Minchev @ 2017-02-14 17:33 UTC (permalink / raw)
  To: Jianxun Zhang; +Cc: meta-intel, yocto

On Fri, 2017-02-10 at 10:58 -0800, Jianxun Zhang wrote:
> > On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > A debug version of the rmc binary can be built by using the debug
> > Makefile target. This will include debug symbols and will disable
> > compiler optimizations.
> > 
> > Example:
> > 
> > make debug
> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > Makefile | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index c58047a..fdd936f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -23,6 +23,8 @@ ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
> > RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> > 
> > all: rmc
> > +debug: RMC_CFLAGS += -DDEBUG -g -O0
> > +debug: rmc
> > 
> I missed your feedback of V1, just recap it here also with my comment:
> 
> - "Yup, the above will have the same effect as 'make debug'.
> I thought that 'make debug' might be a convenient way to build a debug
> binary? Do you think it makes sense to keep this extra target for
> convenience since it doesn't affect the other usages of make?"
> 
> You could have to come back to CFLAGS when the a hard coded debug flags is not enough.
> 
> And, why pass -DDEBUG in RMC_CFLAGS when invoking gcc to compile rmc? I don’t see this macro DEBUG in the current rmc code.

I am planning to use the DEBUG macro in the future to add some more
verbosity to binaries built with 'make debug'

> 
> Thanks
> > $(ALL_OBJS): %.o: %.c
> > 	$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> > -- 
> > 2.11.1
> > 
> 




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 5/5] rmc: add database extraction functionality
  2017-02-10 19:52   ` Jianxun Zhang
@ 2017-02-14 17:50     ` Todor Minchev
  0 siblings, 0 replies; 13+ messages in thread
From: Todor Minchev @ 2017-02-14 17:50 UTC (permalink / raw)
  To: Jianxun Zhang; +Cc: meta-intel, yocto

On Fri, 2017-02-10 at 11:52 -0800, Jianxun Zhang wrote:
> Todor,
> Appreciate the V2 series. I have only one real concern on using system() in this patch and the whole series. The other comments are more for corner cases which have less impact.
> 
> I could miss some of your feedbacks in V1 threads. Sorry for not to point out things earlier before the V2.
> 
> Thanks
> 
> > On Feb 9, 2017, at 11:17 AM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > The contents of an existing database file can be extracted with the -E option.
> > By default the top level of the directory tree is rmc_db_dump, an alternative
> > path can be specified with the -o option. The file blobs corresponding
> > to a given record will be saved in a separate sub-directory. The sub-directory
> > name of each record is the signature corresponding to the fingerprint for that
> > record with all non-alphanumeric characters stripped.
> > 
> > Example:
> > ./src/rmc -E -d rmc.db -o output/directory/
> > 
> > Successfully extracted rmc.db
> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > inc/rmc_api.h         |   9 +++++
> > inc/rmcl.h            |   7 ++++
> > src/lib/api.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > src/lib/common/rmcl.c |  17 +++++++-
> > src/rmc.c             |  30 +++++++++-----
> > 5 files changed, 157 insertions(+), 12 deletions(-)
> > 
> > diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> > index a484389..2f8c978 100644
> > --- a/inc/rmc_api.h
> > +++ b/inc/rmc_api.h
> > @@ -109,6 +109,15 @@ extern int read_file(const char *pathname, char **data, rmc_size_t* len);
> >  */
> > int write_file(const char *pathname, void *data, rmc_size_t len, int append);
> > 
> > +/* extract the contents of a database file and store the files corresponding to
> > + * each record in a separate directory. The name of each directory is the signature
> > + * of the fingerpring for that record with all non-alphanumeric characters stripped
> > + * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
> > + * (in) output_path: A directory path to extract the database to
> > + * return: 0 on success, non-zero on failure.
> > + */
> > +int dump_db(char *db_pathname, char *output_path) ;
> > +
> > #else
> > /* 2 - API for UEFI context */
> > 
> > diff --git a/inc/rmcl.h b/inc/rmcl.h
> > index 5bbad42..471ebfe 100644
> > --- a/inc/rmcl.h
> > +++ b/inc/rmcl.h
> > @@ -164,4 +164,11 @@ extern int rmcl_generate_db(rmc_record_file_t *record_files, rmc_uint8_t **rmc_d
> >  */
> > extern int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy);
> > 
> > +/*
> > + * Check if db_blob has a valid rmc database signature
> > + *
> > + * return 0 if db_blob has a valid signature or non-zero otherwise
> > + */
> > +int is_rmcdb(rmc_uint8_t *db_blob);
> > +
> > #endif /* INC_RMCL_H_ */
> > diff --git a/src/lib/api.c b/src/lib/api.c
> > index 0adb390..56746a4 100644
> > --- a/src/lib/api.c
> > +++ b/src/lib/api.c
> > @@ -3,6 +3,7 @@
> >  * RMC API implementation for Linux user space
> >  */
> > 
> > +#define _GNU_SOURCE
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <errno.h>
> > @@ -10,12 +11,15 @@
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <sys/mman.h>
> > +#include <dirent.h>
> > +#include <ctype.h>
> > 
> > #include <rmcl.h>
> > #include <rsmp.h>
> > 
> > -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> > -#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
> > +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> > +#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
> > +#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
> > 
> > int read_file(const char *pathname, char **data, rmc_size_t* len) {
> >     int fd = -1;
> > @@ -325,3 +329,101 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
> > 
> >     return ret;
> > }
> > +
> > +void remove_non_alphanum(char *in) {
> > +    char c;
> > +    unsigned long i = 0, j = 0;
> > +
> > +    while ((c = in[i++]) != '\0'){
> > +        if (isalnum(c))
> > +            in[j++] = c;
> I think this will work in the almost all the real cases. A corner case, though very rare, is two fingerprints differ each other _only_ by non alphnum characters. The dumps of two different fingerprints will be wrongly in the same dir as a result. (We can’t dictate what FW engineers put into bios)
> 
> Maybe in the future we could use ASCII code of hex values of a signature which is a field with a fixed length as dir name?

I agree, this is highly unlikely but theoretically possible. The
directory names can be either an MD5 hash of the fingerprint signature
or as you suggested just the hex representation of the signature. Your
suggestion sounds better (and simpler) so I will go with it.

> 
> > +    }
> > +    in[j] = '\0';
> > +}
> > +
> > +int dump_db(char *db_pathname, char *output_path) {
> > +    rmc_meta_header_t meta_header;
> > +    rmc_db_header_t *db_header = NULL;
> > +    rmc_record_header_t record_header;
> > +    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> > +    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
> > +    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
> > +    rmc_file_t file;
> > +    char *out_dir = NULL, *out_name = NULL, *cmd = NULL, *tmp_dir_name = NULL;
> > +    rmc_size_t db_len = 0;
> > +    rmc_uint8_t *rmc_db = NULL;
> > +    DIR *tmp_dir = NULL;
> > +
> > +    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
> > +        fprintf(stderr, "Failed to read database file\n\n");
> > +        return 1;
> > +    }
> > +
> > +    db_header = (rmc_db_header_t *)rmc_db;
> > +
> > +    /* sanity check of db */
> > +    if (is_rmcdb(rmc_db))
> > +        return 1;
> > +
> > +    /* query the meta. idx: start of record */
> > +    record_idx = sizeof(rmc_db_header_t);
> > +    while (record_idx < db_header->length) {
> > +        memcpy(&record_header, rmc_db + record_idx,
> > +            sizeof(rmc_record_header_t));
> > +
> > +        /* directory name is fingerprint signature with stripped special chars*/
> > +        asprintf(&tmp_dir_name, "%s", record_header.signature.raw);
> > +        remove_non_alphanum(tmp_dir_name);
> The stripped tmp_dir_name could be a null string when all of its values are non alphanum. The following logic could behave unexpected in such case.
> 
> But resolving this issue requires more logic, refer to my first comment too.

+1

> > +        if(output_path)
> > +            asprintf(&out_dir, "%s/%s/", output_path, tmp_dir_name);
> > +        else
> > +            asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, tmp_dir_name);
> > +        if ((tmp_dir = opendir(out_dir))) {
> > +            /* Directory exists */
> > +            closedir(tmp_dir);
> > +            free(tmp_dir_name);
> > +        } else if (ENOENT == errno) {
> > +            /* Directory does not exist, try to create it. */
> > +            asprintf(&cmd, "mkdir -p %s", out_dir);
> > +            if(system(cmd) == -1) {
> I think this is a blocker. Generally the system() is overkilling here and it causes security concerns. Why do you change it from mkdir() in V1?

I got rid of mkdir, because it can only create a single directory at a
time and with the requirement to be able to dump the user output to a
user defined directory this will require implementing a directory path
parser. This said, using system() on unvalidated user input is certainly
dangerous, I will have to switch this back to mkdir.

> 
> > +                fprintf(stderr, "Failed to create %s directory\n\n", out_dir);
> > +                free(out_dir);
> > +                free(tmp_dir_name);
> > +                free(cmd);
> > +                return 1;
> > +            }
> > +            free(cmd);
> > +        } else {
> > +            /* Some other error occured */
> > +            free(out_dir);
> > +            free(tmp_dir_name);
> > +            free(cmd);
> > +            return 1;
> > +        }
> > +
> > +        /* find meta */
> > +        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
> > +            meta_idx < record_idx + record_header.length;) {
> > +            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
> > +            file_idx = meta_idx + sizeof(rmc_meta_header_t);
> > +            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
> > +            file.blob = &rmc_db[file_idx + name_len];
> > +            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
> > +                name_len;
> > +            file.next = NULL;
> > +            file.type = RMC_GENERIC_FILE;
> > +            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
> > +            /* write file to dump directory */
> > +            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
> > +                return 1;
> > +
> > +            /* next meta */
> > +            meta_idx += meta_header.length;
> > +            free(out_name);
> > +        }
> > +        /* next record */
> > +        record_idx += record_header.length;
> > +        free(out_dir);
> > +    }
> > +    return 0;
> > +}
> > diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
> > index 67622a0..58a4a52 100644
> > --- a/src/lib/common/rmcl.c
> > +++ b/src/lib/common/rmcl.c
> > @@ -193,6 +193,21 @@ static int match_record(rmc_record_header_t *r, rmc_signature_t* sig) {
> >     return strncmp((const char *)r->signature.raw, (const char *)sig->raw, sizeof(r->signature.raw));
> > }
> > 
> > +int is_rmcdb(rmc_uint8_t *db_blob) {
> > +    rmc_db_header_t *db_header = NULL;
> > +
> > +    if (db_blob == NULL)
> > +        return 1;
> > +
> > +    db_header = (rmc_db_header_t *)db_blob;
> > +
> > +    /* sanity check of db */
> > +    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> > +        return 1;
> > +    else
> > +        return 0;
> > +}
> > +
> > int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rmc_uint8_t type, char *blob_name, rmc_file_t *policy) {
> >     rmc_meta_header_t meta_header;
> >     rmc_db_header_t *db_header = NULL;
> > @@ -211,7 +226,7 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
> >     db_header = (rmc_db_header_t *)rmc_db;
> > 
> >     /* sanity check of db */
> > -    if (strncmp((const char *)db_header->signature, (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> > +    if (is_rmcdb(rmc_db))
> >         return 1;
> > 
> >     /* calculate signature of fingerprint */
> > diff --git a/src/rmc.c b/src/rmc.c
> > index b5c7847..0740223 100644
> > --- a/src/rmc.c
> > +++ b/src/rmc.c
> > @@ -31,8 +31,10 @@
> >   "running on\n" \
> >     "\t-d: database file to be queried\n" \
> >     "\t-o: path and name of output file of a specific command\n\n" \
> > -  "-E: Extract data from fingerprint file and print it to terminal\n" \
> > -    "\t-f: fingerprint file to extract\n\n" \
> > +  "-E: Extract data from fingerprint file or database\n" \
> > +    "\t-f: fingerprint file to extract\n" \
> > +    "\t-d: database file to extract\n" \
> > +    "\t-o: directory to extract the database to\n\n" \
> >     "Examples (Steps in an order to add board support into rmc):\n\n" \
> >     "1. Generate board fingerprint:\n" \
> >     "\trmc -F\n\n" \
> > @@ -408,12 +410,14 @@ int main(int argc, char **argv){
> > 
> >     /* sanity check for -o */
> >     if (options & RMC_OPT_O) {
> > -        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R | RMC_OPT_CAP_F | RMC_OPT_CAP_B);
> > +        rmc_uint16_t opt_o = options & (RMC_OPT_CAP_D | RMC_OPT_CAP_R |
> > +            RMC_OPT_CAP_F | RMC_OPT_CAP_B | RMC_OPT_CAP_E);
> >         if (!(opt_o)) {
> > -            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -R or -F\n\n");
> > +            fprintf(stderr, "\nWRONG: Option -o cannot be applied without -B, -D, -E, -R or -F\n\n");
> >             usage();
> >             return 1;
> > -        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R && opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B) {
> > +        } else if (opt_o != RMC_OPT_CAP_D && opt_o != RMC_OPT_CAP_R &&
> > +            opt_o != RMC_OPT_CAP_F && opt_o != RMC_OPT_CAP_B  && opt_o != RMC_OPT_CAP_E) {
> >             fprintf(stderr, "\nWRONG: Option -o can be applied with only one of -B, -D, -R and -F\n\n");
> >             usage();
> >             return 1;
> > @@ -428,8 +432,8 @@ int main(int argc, char **argv){
> >     }
> > 
> >     /* sanity check for -E */
> > -    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F))) {
> > -        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name>\n\n");
> > +    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_F) && !(options & RMC_OPT_D))) {
> > +        fprintf(stderr, "\nERROR: -E requires -f <fingerprint file name> or -d <database file name>\n\n");
> >         usage();
> >         return 1;
> >     }
> > @@ -446,7 +450,8 @@ int main(int argc, char **argv){
> >         rmc_file_t file;
> > 
> >         if (!output_path) {
> > -            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
> > +            fprintf(stderr, "-B internal error, with -o but no output \
> > +                pathname specified\n\n");
> >             goto main_free;
> >         }
> > 
> > @@ -454,7 +459,8 @@ int main(int argc, char **argv){
> >             goto main_free;
> > 
> >         if (write_file(output_path, file.blob, file.blob_len, 0)) {
> > -            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
> > +            fprintf(stderr, "-B failed to write file %s to %s\n\n",
> > +                input_blob_name, output_path);
> >             rmc_free_file(&file);
> >             goto main_free;
> >         }
> > @@ -477,6 +483,12 @@ int main(int argc, char **argv){
> >             }else {
> >                 printf("Fingerprint file not provided! Exiting.\n");
> >             }
> > +        } else if (options & RMC_OPT_D) {
> > +            if(dump_db(input_db_path_d, output_path)) {
> > +               fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
> > +               goto main_free;
> > +            } else
> > +               printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
> >         }
> >     }
> > 
> > -- 
> > 2.11.1
> > 
> 




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-02-14 17:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-09 19:17 [PATCH v2 0/5] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
2017-02-09 19:17 ` [PATCH v2 1/5] Makefile: disable silent mode in Makefiles Todor Minchev
2017-02-10 18:40   ` Jianxun Zhang
2017-02-09 19:17 ` [PATCH v2 2/5] Makefile: add debug target Todor Minchev
2017-02-10 18:58   ` Jianxun Zhang
2017-02-14 17:33     ` Todor Minchev
2017-02-09 19:17 ` [PATCH v2 3/5] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
2017-02-10 19:21   ` Jianxun Zhang
2017-02-09 19:17 ` [PATCH v2 4/5] rmc: remove unnecessary return variable Todor Minchev
2017-02-10 19:22   ` Jianxun Zhang
2017-02-09 19:17 ` [PATCH v2 5/5] rmc: add database extraction functionality Todor Minchev
2017-02-10 19:52   ` Jianxun Zhang
2017-02-14 17:50     ` Todor Minchev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.