On Thu, 2007-04-26 at 22:47 -0700, Albert Chu wrote:
> Hey Levi,
> 
> Looks pretty good.  Some minor nit-picks.

Fixed those, and I prevented printing out a key starting with a literal
'0x' in ascii.

        --Levi
Index: common/src/ipmi-common.c
===================================================================
RCS file: /sources/freeipmi/freeipmi/common/src/ipmi-common.c,v
retrieving revision 1.13
diff -t -u -5 -r1.13 ipmi-common.c
--- common/src/ipmi-common.c	13 Sep 2006 21:23:56 -0000	1.13
+++ common/src/ipmi-common.c	27 Apr 2007 17:19:47 -0000
@@ -34,12 +34,14 @@
 #include <errno.h>
 #ifdef HAVE_ERROR_H
 #include <error.h>
 #endif
 #include <argp.h>
+#include <assert.h>
 
 #include "ipmi-common.h"
+#include "freeipmi/ipmi-messaging-support-cmds.h"
 
 #define IPMI_DPRINTF_MAX_BUF_LEN 65536
 
 int
 ipmi_is_root ()
@@ -101,5 +103,115 @@
   while (n--)
     *p++=c;
 
   return s;
 }
+
+/* a k_g key is interpreted as ascii text unless it is prefixed with
+   "0x", in which case is it interpreted as hexadecimal */
+int
+parse_kg(unsigned char *outbuf, int outsz, char *instr)
+{
+  char *p, *q;
+  int i, j;
+  char buf[3] = {0, 0, 0};
+
+  assert(outbuf != NULL);
+  assert(instr != NULL);
+  assert(outsz == IPMI_MAX_K_G_LENGTH);
+
+  if (strlen(instr) == 0)
+    return 0;
+
+  if (strncmp(instr, "0x", 2) == 0) 
+    {
+      if (strlen(instr) > IPMI_MAX_K_G_LENGTH*2+2)
+        return -1;
+      p = instr + 2;
+      guaranteed_memset(outbuf, 0, IPMI_MAX_K_G_LENGTH);
+      for (i = j = 0; i < strlen(p); i+=2, j++)
+        {
+          if (p[i+1] == '\0')
+            return -1;
+          buf[0] = p[i]; buf[1] = p[i+1]; buf[2] = 0;
+          errno = 0;
+          outbuf[j] = strtoul(buf, &q, 16);
+          if (errno || (q != buf + 2))
+            return -1;
+        }
+    }
+  else
+    {
+      if (strlen(instr) > IPMI_MAX_K_G_LENGTH)
+        return -1;
+      guaranteed_memset(outbuf, 0, IPMI_MAX_K_G_LENGTH);
+      memcpy(outbuf, instr, strlen(instr));
+    }
+
+  return 1;
+}
+
+char *
+format_kg(char *outstr, int outsz, unsigned char *k_g)
+{
+  int i;
+  int printable = 1;
+  int foundnull = 0;
+  char *p;
+
+  assert(outstr != NULL);
+  assert(k_g != NULL);
+
+  /* Are there any characters that would prevent printing this as a
+     string on a single line? */
+  for (i = 0; i < IPMI_MAX_K_G_LENGTH; i++)
+    {
+      if (k_g[i] == '\0')
+        {
+          ++foundnull;
+          continue;
+        }
+      if (!(isgraph(k_g[i]) || k_g[i] == ' ') || foundnull)
+        {
+          printable = 0;
+          break;
+        }
+    }
+
+  /* print out an entirely null key in hex rather than an empty
+     string */
+  if (foundnull == IPMI_MAX_K_G_LENGTH)
+    printable = 0;
+
+  /* don't print out a key starting with a literal '0x' as a string,
+     since parse_kg will try to interpret such strings as hex */
+  if (k_g[0] == '0' && k_g[1] == 'x')
+    printable = 0;
+
+  if (printable)
+    {
+      if (outsz < IPMI_MAX_K_G_LENGTH+1)
+        return NULL;
+      p = outstr;
+      for (i = 0; i < IPMI_MAX_K_G_LENGTH; i++)
+        {
+          if (k_g[i] == '\0')
+            break;
+          p[i] = k_g[i];
+        }
+      p[i] = 0;
+    }
+  else
+    {
+      if (outsz < IPMI_MAX_K_G_LENGTH*2+1)
+        return NULL;
+      p = outstr;
+      p[0] = '0'; p[1] = 'x';
+      p+=2;
+      for (i = 0; i < IPMI_MAX_K_G_LENGTH; i++, p+=2)
+        {
+          sprintf(p, "%02x", k_g[i]);
+        }
+    }
+
+  return outstr;
+}
Index: common/src/ipmi-common.h
===================================================================
RCS file: /sources/freeipmi/freeipmi/common/src/ipmi-common.h,v
retrieving revision 1.9
diff -t -u -5 -r1.9 ipmi-common.h
--- common/src/ipmi-common.h	21 Jul 2006 18:09:05 -0000	1.9
+++ common/src/ipmi-common.h	27 Apr 2007 17:19:47 -0000
@@ -68,6 +68,12 @@
 int ipmi_dprintf(int fd, char *fmt, ...);
 
 /* From David Wheeler's Secure Programming Guide */
 void *guaranteed_memset(void *s, int c, size_t n);
 
+/* Turn an input string into a 20-byte binary k_g key */
+int parse_kg(unsigned char *outbuf, int outsz, char *instr);
+
+/* Turn a 20-byte binary k_g key into an output string */
+char *format_kg(char *outstr, int outsz, unsigned char *k_g);
+
 #endif
Index: ipmipower/ipmipower.8.in
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/ipmipower.8.in,v
retrieving revision 1.53.2.1
diff -t -u -5 -r1.53.2.1 ipmipower.8.in
--- ipmipower/ipmipower.8.in	8 Mar 2007 03:57:41 -0000	1.53.2.1
+++ ipmipower/ipmipower.8.in	27 Apr 2007 17:19:50 -0000
@@ -97,15 +97,19 @@
 Prompt for password to avoid possibility of listing it in process
 lists.
 .TP
 .I "-k, --k-g str"
 Sets the K_g BMC Key to use when authenticating with the BMC for
-ipmi 2.0.  If not specified, a null key is assumed.
+ipmi 2.0.  If not specified, a null key is assumed.  To input the key
+in hexadecimal form, prefix the string with '0x'.  E.g., the key 'abc'
+can be entered with the either the string 'abc' or the
+string '0x616263'
 .TP
 .I "-K, --k-g-prompt"
 Prompt for K_g to avoid possibility of listing it in process
-lists.
+lists.  To input the key in hexadecimal form, prefix the string
+with '0x'.
 .TP
 .I "-n, --on"
 Power on the target hosts.
 .TP
 .I "-f, --off"
@@ -420,11 +424,12 @@
 .TP
 .I "password [str]"
 specify a new password, no str for null password
 .TP
 .I "k_g [str]"
-specify a new K_g BMC Key, no str for null
+specify a new K_g BMC Key, no str for null, prefix with '0x' to enter
+a key in hexadecimal
 .TP
 .I "on [host(s)]"
 turns on all hosts, or only the specified host(s).
 .TP
 .I "off [host(s)]"
Index: ipmipower/ipmipower.conf.5.in
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/ipmipower.conf.5.in,v
retrieving revision 1.31
diff -t -u -5 -r1.31 ipmipower.conf.5.in
--- ipmipower/ipmipower.conf.5.in	2 Nov 2006 19:36:45 -0000	1.31
+++ ipmipower/ipmipower.conf.5.in	27 Apr 2007 17:19:50 -0000
@@ -85,11 +85,12 @@
 .TP
 .I password str
 Specify the default password to use.
 .TP
 .I k_g str
-Specify the BMC key (K_g) to use.
+Specify the BMC key (K_g) to use.  Prefix the string with '0x' to
+specify the key in hexadecimal.
 .TP
 .I authentication-type str
 Specify the default authentication type to use.  
 .B Ipmipower 
 currently supports the following authentication types:
Index: ipmipower/src/Makefile.am
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/src/Makefile.am,v
retrieving revision 1.12.2.2
diff -t -u -5 -r1.12.2.2 Makefile.am
--- ipmipower/src/Makefile.am	7 Mar 2007 04:11:04 -0000	1.12.2.2
+++ ipmipower/src/Makefile.am	27 Apr 2007 17:19:50 -0000
@@ -21,10 +21,11 @@
         wrappers.c
  
 ipmipower_LDADD = \
         ../../common/src/libcbuf.la \
         ../../common/src/libllnlcommon.la \
+        ../../common/src/libipmicommon.la \
         ../../libfreeipmi/src/libfreeipmi.la
  
 ipmipower_CPPFLAGS = \
         -I$(srcdir)/../../common/src \
         -I$(srcdir)/../../libfreeipmi/include \
@@ -56,9 +57,12 @@
         $(MAKE) -C $(dir $@) $(notdir $@)
 
 ../../common/src/libllnlcommon.la: force-dependency-check
         $(MAKE) -C $(dir $@) $(notdir $@)
 
+../../common/src/libipmicommon.la: force-dependency-check
+        $(MAKE) -C $(dir $@) $(notdir $@)
+
 ../../libfreeipmi/src/libfreeipmi.la: force-dependency-check
         $(MAKE) -C $(dir $@) $(notdir $@)
  
 force-dependency-check:
Index: ipmipower/src/ipmipower.h
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/src/ipmipower.h,v
retrieving revision 1.65
diff -t -u -5 -r1.65 ipmipower.h
--- ipmipower/src/ipmipower.h	21 Oct 2006 01:36:27 -0000	1.65
+++ ipmipower/src/ipmipower.h	27 Apr 2007 17:19:51 -0000
@@ -550,11 +550,11 @@
 {
   hostlist_t               hosts;
   int                      hosts_count;
   char                     username[IPMI_MAX_USER_NAME_LENGTH+1];
   char                     password[IPMI_2_0_MAX_PASSWORD_LENGTH+1];
-  char                     k_g[IPMI_MAX_K_G_LENGTH+1];
+  char                     k_g[IPMI_MAX_K_G_LENGTH];
   power_cmd_t              powercmd;
   char                     configfile[MAXPATHLEN+1];
 
   authentication_type_t    authentication_type;
   privilege_type_t         privilege;
@@ -592,10 +592,11 @@
   /* Flags indicating if option was set on the command line */
   ipmipower_bool_t         hosts_set;
   ipmipower_bool_t         username_set;
   ipmipower_bool_t         password_set;
   ipmipower_bool_t         k_g_set;
+  ipmipower_bool_t         k_g_configured;
   ipmipower_bool_t         authentication_type_set;
   ipmipower_bool_t         privilege_set;
   ipmipower_bool_t         ipmi_version_set;
   ipmipower_bool_t         cipher_suite_id_set;
   ipmipower_bool_t         on_if_off_set;
Index: ipmipower/src/ipmipower_config.c
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/src/ipmipower_config.c,v
retrieving revision 1.44.2.3
diff -t -u -5 -r1.44.2.3 ipmipower_config.c
--- ipmipower/src/ipmipower_config.c	28 Mar 2007 23:24:42 -0000	1.44.2.3
+++ ipmipower/src/ipmipower_config.c	27 Apr 2007 17:19:51 -0000
@@ -51,10 +51,11 @@
 #include "ipmipower_privilege.h"
 #include "ipmipower_util.h"
 #include "ipmipower_wrappers.h"
 
 #include "secure.h"
+#include "ipmi-common.h"
       
 extern struct ipmipower_config *conf;
 extern struct ipmipower_connection *ics;
 
 void 
@@ -87,11 +88,11 @@
 
   conf->hosts = NULL;
   conf->hosts_count = 0;
   memset(conf->username, '\0', IPMI_MAX_USER_NAME_LENGTH+1);
   memset(conf->password, '\0', IPMI_2_0_MAX_PASSWORD_LENGTH+1);
-  memset(conf->k_g, '\0', IPMI_MAX_K_G_LENGTH+1);
+  memset(conf->k_g, '\0', IPMI_MAX_K_G_LENGTH);
   conf->powercmd = POWER_CMD_NONE;
   memset(conf->configfile, '\0', MAXPATHLEN+1);
 
   conf->authentication_type = AUTHENTICATION_TYPE_AUTO;
   conf->privilege = PRIVILEGE_TYPE_AUTO;
@@ -265,10 +266,11 @@
 {
   char c;
   char *ptr;
   char *pw;
   char *kg;
+  int rv;
 
   /* achu: Here's are what options are left and available
      lower case: de
      upper case: EGJNOQZ
    */
@@ -380,13 +382,14 @@
             err_exit("password too long");
           strcpy(conf->password, pw);
           conf->password_set = IPMIPOWER_TRUE;
           break;
         case 'k':       /* --k-g */
-          if (strlen(optarg) > IPMI_MAX_K_G_LENGTH)
-            err_exit("Command Line Error: K_g too long");
-          strcpy(conf->k_g, optarg);
+          if ((rv = parse_kg(conf->k_g, IPMI_MAX_K_G_LENGTH, optarg)) < 0)
+            err_exit("Command Line Error: Invalid K_g");
+          if (rv > 0)
+            conf->k_g_configured = IPMIPOWER_TRUE;
           conf->k_g_set = IPMIPOWER_TRUE;
           if (optarg)
             {
               int n;
               n = strlen(optarg);
@@ -394,13 +397,14 @@
             }
           break;
         case 'K':       /* --k-g-prompt */
           if (!(kg = getpass("K_g: ")))
             err_exit("getpass: %s", strerror(errno));
-          if (strlen(kg) > IPMI_MAX_K_G_LENGTH)
-            err_exit("K_g too long");
-          strcpy(conf->k_g, kg);
+          if ((rv = parse_kg(conf->k_g, IPMI_MAX_K_G_LENGTH, kg)) < 0)
+            err_exit("K_g invalid");
+          if (rv > 0)
+            conf->k_g_configured = IPMIPOWER_TRUE;
           conf->k_g_set = IPMIPOWER_TRUE;
           break;
         case 'n':       /* --on */ 
           conf->powercmd = POWER_CMD_POWER_ON;
           break;
@@ -732,17 +736,19 @@
 static int 
 _cb_k_g(conffile_t cf, struct conffile_data *data,
              char *optionname, int option_type, void *option_ptr,
              int option_data, void *app_ptr, int app_data) 
 {
+  int rv;
+
   if (conf->k_g_set == IPMIPOWER_TRUE)
     return 0;
 
-  if (strlen(data->string) > IPMI_MAX_K_G_LENGTH)
-    err_exit("Config File Error: K_g too long");
-
-  strcpy(conf->k_g, data->string);
+  if ((rv = parse_kg(conf->k_g, IPMI_MAX_K_G_LENGTH, data->string)) < 0)
+    err_exit("Config File Error: K_g invalid");
+  if (rv > 0)
+    conf->k_g_configured = IPMIPOWER_TRUE;
   return 0;
 }
 
 void 
 ipmipower_config_conffile_parse(char *configfile) 
@@ -874,11 +880,11 @@
     err_exit("Error: password cannot be set for authentication type \"%s\"",
              ipmipower_authentication_type_string(conf->authentication_type));
 
   if (conf->ipmi_version != IPMI_VERSION_AUTO
       && conf->ipmi_version != IPMI_VERSION_2_0
-      && strlen(conf->k_g) > 0)
+      && conf->k_g_configured == IPMIPOWER_TRUE)
     err_exit("Error: k_g is only used for IPMI 2.0");
 
   if (conf->ipmi_version == IPMI_VERSION_1_5
       && strlen(conf->password) >= IPMI_1_5_MAX_PASSWORD_LENGTH)
     err_exit("Error: password too long");
Index: ipmipower/src/ipmipower_powercmd.c
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/src/ipmipower_powercmd.c,v
retrieving revision 1.98.2.1
diff -t -u -5 -r1.98.2.1 ipmipower_powercmd.c
--- ipmipower/src/ipmipower_powercmd.c	20 Apr 2007 05:14:35 -0000	1.98.2.1
+++ ipmipower/src/ipmipower_powercmd.c	27 Apr 2007 17:19:51 -0000
@@ -1179,12 +1179,12 @@
       ipmipower_output(MSG_TYPE_PERMISSION, ip->ic->hostname);
 #endif /* !NDEBUG */
       return -1;
     }
 
-  if ((!strlen(conf->k_g) && authentication_status_k_g)
-      || (strlen(conf->k_g) && !authentication_status_k_g))
+  if ((conf->k_g_configured != IPMIPOWER_TRUE && authentication_status_k_g)
+      || (conf->k_g_configured == IPMIPOWER_TRUE && !authentication_status_k_g))
     {
 #ifndef NDEBUG
       ipmipower_output(MSG_TYPE_K_G, ip->ic->hostname);
 #else  /* !NDEBUG */
       ipmipower_output(MSG_TYPE_PERMISSION, ip->ic->hostname);
@@ -1842,11 +1842,11 @@
   if (conf->intel_2_0_session 
       && ip->authentication_algorithm == IPMI_AUTHENTICATION_ALGORITHM_RAKP_HMAC_MD5
       && password_len > IPMI_1_5_MAX_PASSWORD_LENGTH)
     password_len = IPMI_1_5_MAX_PASSWORD_LENGTH;
 
-  if (strlen(conf->k_g))
+  if (conf->k_g_configured == IPMIPOWER_TRUE)
     k_g = (uint8_t *)conf->k_g;
   else
     k_g = NULL;
   
   managed_system_random_number_len = Fiid_obj_get_data(ip->obj_rakp_message_2_res,
@@ -1858,11 +1858,11 @@
                                            ip->integrity_algorithm,
                                            ip->confidentiality_algorithm,
                                            password,
                                            password_len,
                                            k_g,
-                                           (k_g) ? strlen((char *)k_g) : 0,
+                                           (k_g) ? IPMI_MAX_K_G_LENGTH : 0,
                                            ip->remote_console_random_number,
                                            IPMI_REMOTE_CONSOLE_RANDOM_NUMBER_LENGTH,
                                            managed_system_random_number,
                                            managed_system_random_number_len,
                                            ip->name_only_lookup,
Index: ipmipower/src/ipmipower_prompt.c
===================================================================
RCS file: /sources/freeipmi/freeipmi/ipmipower/src/ipmipower_prompt.c,v
retrieving revision 1.38.2.1
diff -t -u -5 -r1.38.2.1 ipmipower_prompt.c
--- ipmipower/src/ipmipower_prompt.c	15 Dec 2006 23:37:11 -0000	1.38.2.1
+++ ipmipower/src/ipmipower_prompt.c	27 Apr 2007 17:19:52 -0000
@@ -54,10 +54,12 @@
 #include "ipmipower_powercmd.h"
 #include "ipmipower_output.h"
 #include "ipmipower_privilege.h"
 #include "ipmipower_wrappers.h"
 
+#include "ipmi-common.h"
+
 extern cbuf_t ttyout;
 extern cbuf_t ttyin;    
 extern cbuf_t ttyerr;
 extern struct ipmipower_config *conf;
 extern struct ipmipower_connection *ics;
@@ -342,32 +344,41 @@
 }
 
 static void 
 _cmd_k_g(char **argv) 
 {
+  int rv = 0;
+  char buf[IPMI_MAX_K_G_LENGTH*2+1];
   assert(argv != NULL);
 
   if (conf->ipmi_version != IPMI_VERSION_AUTO
       && conf->ipmi_version != IPMI_VERSION_2_0)
     cbuf_printf(ttyout, "k_g is only used for IPMI 2.0");
-  else if (argv[1] == NULL 
-           || (argv[1] && strlen(argv[1]) <= IPMI_MAX_K_G_LENGTH)) 
+  else
     {
-      memset(conf->k_g, '\0', IPMI_MAX_K_G_LENGTH+1);
+      memset(conf->k_g, '\0', IPMI_MAX_K_G_LENGTH);
 
       if (argv[1])
-        strcpy(conf->k_g, argv[1]);
-
+        rv = parse_kg(conf->k_g, IPMI_MAX_K_G_LENGTH, argv[1]);
+      
+      if (rv < 0)
+        cbuf_printf(ttyout, "k_g invalid\n");
+      else
+        {
+          if (rv == 0)
+            conf->k_g_configured = IPMIPOWER_FALSE;
+          else 
+            conf->k_g_configured = IPMIPOWER_TRUE;
 #ifdef NDEBUG
-      cbuf_printf(ttyout, "k_g changed\n");
+          cbuf_printf(ttyout, "k_g changed\n");
 #else  /* !NDEBUG */
-      cbuf_printf(ttyout, "k_g: %s\n", 
-                  (strlen(conf->k_g)) ? conf->k_g : "NULL");
+          cbuf_printf(ttyout, "k_g: %s\n", 
+                      (conf->k_g_configured == IPMIPOWER_TRUE) ? 
+                      format_kg(buf, IPMI_MAX_K_G_LENGTH*2+1, conf->k_g) : "NULL");
 #endif /* !NDEBUG */
+        }
     }
-  else
-    cbuf_printf(ttyout, "k_g invalid length\n");
 }
 
 static void 
 _cmd_authentication_type(char **argv) 
 {
@@ -568,10 +579,12 @@
 #endif /* NDEBUG */
 
 static void 
 _cmd_config(void) 
 {
+  char buf[IPMI_MAX_K_G_LENGTH*2+1];
+
   if (conf->hosts != NULL) 
     {
       int rv;
       char buffer[IPMIPOWER_HOSTLIST_BUFLEN];
 #ifndef NDEBUG
@@ -647,11 +660,12 @@
 
 #ifndef NDEBUG
   cbuf_printf(ttyout, "Password:                     %s\n", 
               (strlen(conf->password)) ? conf->password : "NULL");
   cbuf_printf(ttyout, "K_g:                          %s\n", 
-              (strlen(conf->k_g)) ? conf->k_g : "NULL");
+              (conf->k_g_configured == IPMIPOWER_TRUE) ? 
+              format_kg(buf, IPMI_MAX_K_G_LENGTH*2+2, conf->k_g) : "NULL");
 #else  /* !NDEBUG */
   cbuf_printf(ttyout, "Password:                     *****\n");
   cbuf_printf(ttyout, "K_g:                          *****\n");
 #endif /* !NDEBUG */
 
_______________________________________________
Freeipmi-devel mailing list
Freeipmi-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/freeipmi-devel

Reply via email to