path: root/system/ipmitool/patches/0009-best-cipher.patch

From f2df2aa5a010544d53589a5b048677406eb40ee8 Mon Sep 17 00:00:00 2001
From: Vernon Mauery <vernon.mauery@intel.com>
Date: Mon, 9 Apr 2018 12:28:57 -0700
Subject: [PATCH] lanplus: Auto-select 'best' cipher suite available

Current cipher suites could be ranked as this:
  17 > 3 >> all the rest

Cherry-picked-from: 7772254b62826b894ca629df8c597030a98f4f72
Cherry-picked-from: f9c699c712f884c82fc1a62f1f61a8d597ac0cfd

Also fetched some functions/macros for helper.h

Equals to getting include/ipmitool/helper.h changes from commits:
(oldest first)

  6c00d44 mc: watchdog get: Update to match IPMI 2.0 spec
  e8e94d8 mc: watchdog set: Refactor to reduce complexity
  0310208 mc: Code refactor to reduce copy-paste ratio
  249e092 general: Make byteswapping arch-independent
  5491b12 refix 249e0929: Fix byteswapping helpers
  bb1a4cc Refactoring. Improve code reuse ratio.
---
 include/ipmitool/helper.h       |  58 ++++++++
 include/ipmitool/ipmi_channel.h |  47 +++++++
 include/ipmitool/ipmi_intf.h    |  39 ++++-
 lib/ipmi_channel.c              | 242 +++++++++++++++-----------------
 lib/ipmi_main.c                 |  23 +--
 src/plugins/ipmi_intf.c         |   5 +-
 src/plugins/lanplus/lanplus.c   | 114 +++++++++++----
 7 files changed, 360 insertions(+), 168 deletions(-)

diff --git a/include/ipmitool/helper.h b/include/ipmitool/helper.h
index c53736f..6a2e5f4 100644
--- a/include/ipmitool/helper.h
+++ b/include/ipmitool/helper.h
@@ -38,6 +38,8 @@
 #include <stdio.h>
 #include <string.h>
 
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
+
 #ifndef TRUE
 #define TRUE    1
 #endif
@@ -111,6 +113,62 @@ FILE * ipmi_open_file(const char * file, int rw);
 void ipmi_start_daemon(struct ipmi_intf *intf);
 uint16_t ipmi_get_oem_id(struct ipmi_intf *intf);
 
+#define IS_SET(v, b) ((v) & (1 << (b)))
+
+/* le16toh(), hto16le(), et. al. don't exist for Windows or Apple */
+/* For portability, let's simply define our own versions here */
+
+/* IPMI is always little-endian */
+static inline uint16_t ipmi16toh(void *ipmi16)
+{
+	uint8_t *ipmi = (uint8_t *)ipmi16;
+	uint16_t h;
+
+	h = ipmi[1] << 8; /* MSB */
+	h |= ipmi[0]; /* LSB */
+
+	return h;
+}
+
+static inline void htoipmi16(uint16_t h, uint8_t *ipmi)
+{
+	ipmi[0] = h & 0xFF; /* LSB */
+	ipmi[1] = h >> 8; /* MSB */
+}
+
+static inline uint32_t ipmi24toh(void *ipmi24)
+{
+	uint8_t *ipmi = (uint8_t *)ipmi24;
+	uint32_t h = 0;
+
+	h = ipmi[2] << 16; /* MSB */
+	h |= ipmi[1] << 8;
+	h |= ipmi[0]; /* LSB */
+
+	return h;
+}
+
+static inline uint32_t ipmi32toh(void *ipmi32)
+{
+	uint8_t *ipmi = ipmi32;
+	uint32_t h;
+
+	h = ipmi[3] << 24; /* MSB */
+	h |= ipmi[2] << 16;
+	h |= ipmi[1] << 8;
+	h |= ipmi[0]; /* LSB */
+
+	return h;
+}
+
+static inline void htoipmi32(uint32_t h, uint8_t *ipmi)
+{
+	ipmi[0] = h & 0xFF; /* LSB */
+	ipmi[1] = (h >> 8) & 0xFF;
+	ipmi[2] = (h >> 16) & 0xFF;
+	ipmi[3] = (h >> 24) & 0xFF; /* MSB */
+}
+
 #define ipmi_open_file_read(file)	ipmi_open_file(file, 0)
 #define ipmi_open_file_write(file)	ipmi_open_file(file, 1)
 
diff --git a/include/ipmitool/ipmi_channel.h b/include/ipmitool/ipmi_channel.h
index 3ade2d5..324c0bb 100644
--- a/include/ipmitool/ipmi_channel.h
+++ b/include/ipmitool/ipmi_channel.h
@@ -37,6 +37,7 @@
 # include <config.h>
 #endif
 #include <ipmitool/ipmi.h>
+#include <ipmitool/ipmi_intf.h>
 
 
 #define IPMI_GET_CHANNEL_AUTH_CAP      0x38
@@ -81,6 +82,50 @@ struct channel_access_t {
 	uint8_t user_level_auth;
 };
 
+/*
+ * The Cipher Suite Record Format from table 22-18 of the IPMI v2.0 spec
+ */
+enum cipher_suite_format_tag {
+	STANDARD_CIPHER_SUITE = 0xc0,
+	OEM_CIPHER_SUITE = 0xc1,
+};
+#ifdef HAVE_PRAGMA_PACK
+#pragma pack(1)
+#endif
+struct std_cipher_suite_record_t {
+	uint8_t start_of_record;
+	uint8_t cipher_suite_id;
+	uint8_t auth_alg;
+	uint8_t integrity_alg;
+	uint8_t crypt_alg;
+} ATTRIBUTE_PACKING;
+struct oem_cipher_suite_record_t {
+	uint8_t start_of_record;
+	uint8_t cipher_suite_id;
+	uint8_t iana[3];
+	uint8_t auth_alg;
+	uint8_t integrity_alg;
+	uint8_t crypt_alg;
+} ATTRIBUTE_PACKING;
+#ifdef HAVE_PRAGMA_PACK
+#pragma pack(0)
+#endif
+#define CIPHER_ALG_MASK 0x3f
+#define MAX_CIPHER_SUITE_RECORD_OFFSET 0x40
+#define MAX_CIPHER_SUITE_DATA_LEN 0x10
+#define LIST_ALGORITHMS_BY_CIPHER_SUITE 0x80
+
+/* Below is the theoretical maximum number of cipher suites that could be
+ * reported by a BMC. That is with the Get Channel Cipher Suites Command, at 16
+ * bytes at a time and 0x40 requests, it can report 1024 bytes, which is about
+ * 204 standard records or 128 OEM records. Really, we probably don't need more
+ * than about 20, which is the full set of standard records plus a few OEM
+ * records.
+ */
+#define MAX_CIPHER_SUITE_COUNT (MAX_CIPHER_SUITE_RECORD_OFFSET * \
+		MAX_CIPHER_SUITE_DATA_LEN / \
+		sizeof(struct std_cipher_suite_record_t))
+
 /*
  * The Get Authentication Capabilities response structure
  * From table 22-15 of the IPMI v2.0 spec
@@ -135,6 +180,8 @@ struct get_channel_auth_cap_rsp {
 int _ipmi_get_channel_access(struct ipmi_intf *intf,
 		struct channel_access_t *channel_access,
 		uint8_t get_volatile_settings);
+int ipmi_get_channel_cipher_suites(struct ipmi_intf *intf, const char *payload_type,
+		uint8_t channel, struct cipher_suite_info *suites, size_t *count);
 int _ipmi_get_channel_info(struct ipmi_intf *intf,
         struct channel_info_t *channel_info);
 int _ipmi_set_channel_access(struct ipmi_intf *intf,
diff --git a/include/ipmitool/ipmi_intf.h b/include/ipmitool/ipmi_intf.h
index 0b8c64b..7a07d66 100644
--- a/include/ipmitool/ipmi_intf.h
+++ b/include/ipmitool/ipmi_intf.h
@@ -61,13 +61,45 @@ enum LANPLUS_SESSION_STATE {
 #define IPMI_AUTHCODE_BUFFER_SIZE 20
 #define IPMI_SIK_BUFFER_SIZE      IPMI_MAX_MD_SIZE
 
+enum cipher_suite_ids {
+	IPMI_LANPLUS_CIPHER_SUITE_0 = 0,
+	IPMI_LANPLUS_CIPHER_SUITE_1 = 1,
+	IPMI_LANPLUS_CIPHER_SUITE_2 = 2,
+	IPMI_LANPLUS_CIPHER_SUITE_3 = 3,
+	IPMI_LANPLUS_CIPHER_SUITE_4 = 4,
+	IPMI_LANPLUS_CIPHER_SUITE_5 = 5,
+	IPMI_LANPLUS_CIPHER_SUITE_6 = 6,
+	IPMI_LANPLUS_CIPHER_SUITE_7 = 7,
+	IPMI_LANPLUS_CIPHER_SUITE_8 = 8,
+	IPMI_LANPLUS_CIPHER_SUITE_9 = 9,
+	IPMI_LANPLUS_CIPHER_SUITE_10 = 10,
+	IPMI_LANPLUS_CIPHER_SUITE_11 = 11,
+	IPMI_LANPLUS_CIPHER_SUITE_12 = 12,
+	IPMI_LANPLUS_CIPHER_SUITE_13 = 13,
+	IPMI_LANPLUS_CIPHER_SUITE_14 = 14,
+#ifdef HAVE_CRYPTO_SHA256
+	IPMI_LANPLUS_CIPHER_SUITE_15 = 15,
+	IPMI_LANPLUS_CIPHER_SUITE_16 = 16,
+	IPMI_LANPLUS_CIPHER_SUITE_17 = 17,
+#endif /* HAVE_CRYPTO_SHA256 */
+	IPMI_LANPLUS_CIPHER_SUITE_RESERVED = 0xff,
+};
+
+struct cipher_suite_info {
+	enum cipher_suite_ids cipher_suite_id;
+	uint8_t auth_alg;
+	uint8_t integrity_alg;
+	uint8_t crypt_alg;
+	uint32_t iana;
+};
+
 struct ipmi_session_params {
 	char * hostname;
 	uint8_t username[17];
 	uint8_t authcode_set[IPMI_AUTHCODE_BUFFER_SIZE + 1];
 	uint8_t authtype_set;
 	uint8_t privlvl;
-	uint8_t cipher_suite_id;
+	enum cipher_suite_ids cipher_suite_id;
 	char sol_escape_char;
 	int password;
 	int port;
@@ -217,7 +249,10 @@ void ipmi_intf_session_set_username(struct ipmi_intf * intf, char * username);
 void ipmi_intf_session_set_password(struct ipmi_intf * intf, char * password);
 void ipmi_intf_session_set_privlvl(struct ipmi_intf * intf, uint8_t privlvl);
 void ipmi_intf_session_set_lookupbit(struct ipmi_intf * intf, uint8_t lookupbit);
-void ipmi_intf_session_set_cipher_suite_id(struct ipmi_intf * intf, uint8_t cipher_suite_id);
+#ifdef IPMI_INTF_LANPLUS
+void ipmi_intf_session_set_cipher_suite_id(struct ipmi_intf * intf,
+		enum cipher_suite_ids cipher_suite_id);
+#endif /* IPMI_INTF_LANPLUS */
 void ipmi_intf_session_set_sol_escape_char(struct ipmi_intf * intf, char sol_escape_char);
 void ipmi_intf_session_set_kgkey(struct ipmi_intf *intf, const uint8_t *kgkey);
 void ipmi_intf_session_set_port(struct ipmi_intf * intf, int port);
diff --git a/lib/ipmi_channel.c b/lib/ipmi_channel.c
index e1fc75f..3ae3104 100644
--- a/lib/ipmi_channel.c
+++ b/lib/ipmi_channel.c
@@ -342,86 +342,116 @@ ipmi_get_channel_auth_cap(struct ipmi_intf *intf, uint8_t channel, uint8_t priv)
 	return 0;
 }
 
-static int
+static size_t
+parse_channel_cipher_suite_data(uint8_t *cipher_suite_data, size_t data_len,
+		struct cipher_suite_info* suites, size_t nr_suites)
+{
+	size_t count = 0;
+	size_t offset = 0;
+	uint32_t iana;
+	uint8_t auth_alg, integrity_alg, crypt_alg;
+	uint8_t cipher_suite_id;
+
+	memset(suites, 0, sizeof(*suites) * nr_suites);
+
+	while (offset < data_len && count < nr_suites) {
+		auth_alg      = IPMI_AUTH_RAKP_NONE;
+		integrity_alg = IPMI_INTEGRITY_NONE;
+		crypt_alg     = IPMI_CRYPT_NONE;
+		if (cipher_suite_data[offset] == STANDARD_CIPHER_SUITE) {
+			struct std_cipher_suite_record_t *record =
+				(struct std_cipher_suite_record_t*)(&cipher_suite_data[offset]);
+			/* standard type */
+			iana = 0;
+
+			/* Verify that we have at least a full record left; id + 3 algs */
+			if ((data_len - offset) < sizeof(*record)) {
+				lprintf(LOG_INFO, "Incomplete data record in cipher suite data");
+				break;
+			}
+			cipher_suite_id = record->cipher_suite_id;
+			auth_alg = CIPHER_ALG_MASK & record->auth_alg;
+			integrity_alg = CIPHER_ALG_MASK & record->integrity_alg;
+			crypt_alg = CIPHER_ALG_MASK & record->crypt_alg;
+			offset += sizeof(*record);
+		} else if (cipher_suite_data[offset] == OEM_CIPHER_SUITE) {
+			/* OEM record type */
+			struct oem_cipher_suite_record_t *record =
+				(struct oem_cipher_suite_record_t*)(&cipher_suite_data[offset]);
+			/* Verify that we have at least a full record left
+			 * id + iana + 3 algs
+			 */
+			if ((data_len - offset) < sizeof(*record)) {
+				lprintf(LOG_INFO, "Incomplete data record in cipher suite data");
+				break;
+			}
+
+			cipher_suite_id = record->cipher_suite_id;
+
+			/* Grab the IANA */
+			iana = ipmi24toh(record->iana);
+			auth_alg = CIPHER_ALG_MASK & record->auth_alg;
+			integrity_alg = CIPHER_ALG_MASK & record->integrity_alg;
+			crypt_alg = CIPHER_ALG_MASK & record->crypt_alg;
+			offset += sizeof(*record);
+		} else {
+			lprintf(LOG_INFO, "Bad start of record byte in cipher suite data (offset %d, value %x)", offset, cipher_suite_data[offset]);
+			break;
+		}
+		suites[count].cipher_suite_id = cipher_suite_id;
+		suites[count].iana = iana;
+		suites[count].auth_alg = auth_alg;
+		suites[count].integrity_alg = integrity_alg;
+		suites[count].crypt_alg = crypt_alg;
+		count++;
+	}
+	return count;
+}
+
+int
 ipmi_get_channel_cipher_suites(struct ipmi_intf *intf, const char *payload_type,
-		uint8_t channel)
+		uint8_t channel, struct cipher_suite_info *suites, size_t *count)
 {
 	struct ipmi_rs *rsp;
 	struct ipmi_rq req;
 
 	uint8_t rqdata[3];
-	uint32_t iana;
-	uint8_t auth_alg, integrity_alg, crypt_alg;
-	uint8_t cipher_suite_id;
 	uint8_t list_index = 0;
 	/* 0x40 sets * 16 bytes per set */
-	uint8_t cipher_suite_data[1024];
-	uint16_t offset = 0;
-	/* how much was returned, total */
-	uint16_t cipher_suite_data_length = 0;
+	uint8_t cipher_suite_data[MAX_CIPHER_SUITE_RECORD_OFFSET *
+		MAX_CIPHER_SUITE_DATA_LEN];
+	size_t offset = 0;
+	size_t nr_suites = 0;
 
+	if (!suites || !count || !*count)
+		return -1;
+
+	nr_suites = *count;
+	*count = 0;
 	memset(cipher_suite_data, 0, sizeof(cipher_suite_data));
-	
+
 	memset(&req, 0, sizeof(req));
 	req.msg.netfn = IPMI_NETFN_APP;
 	req.msg.cmd = IPMI_GET_CHANNEL_CIPHER_SUITES;
 	req.msg.data = rqdata;
-	req.msg.data_len = 3;
+	req.msg.data_len = sizeof(rqdata);
 
 	rqdata[0] = channel;
 	rqdata[1] = ((strncmp(payload_type, "ipmi", 4) == 0)? 0: 1);
-	/* Always ask for cipher suite format */
-	rqdata[2] = 0x80;
-
-	rsp = intf->sendrecv(intf, &req);
-	if (rsp == NULL) {
-		lprintf(LOG_ERR, "Unable to Get Channel Cipher Suites");
-		return -1;
-	}
-	if (rsp->ccode > 0) {
-		lprintf(LOG_ERR, "Get Channel Cipher Suites failed: %s",
-			val2str(rsp->ccode, completion_code_vals));
-		return -1;
-	}
-
-
-	/*
-	 * Grab the returned channel number once.  We assume it's the same
-	 * in future calls.
-	 */
-	if (rsp->data_len >= 1) {
-		channel = rsp->data[0];
-	}
-
-	while ((rsp->data_len > 1) && (rsp->data_len == 17) && (list_index < 0x3F)) {
-		/*
-		 * We got back cipher suite data -- store it.
-		 * printf("copying data to offset %d\n", offset);
-		 * printbuf(rsp->data + 1, rsp->data_len - 1, "this is the data");
-		 */
-		memcpy(cipher_suite_data + offset, rsp->data + 1, rsp->data_len - 1);
-		offset += rsp->data_len - 1;
-		
-		/*
-		 * Increment our list for the next call
-		 */
-		++list_index;
-		rqdata[2] =  (rqdata[2] & 0x80) + list_index; 
 
+	do {
+		/* Always ask for cipher suite format */
+		rqdata[2] = LIST_ALGORITHMS_BY_CIPHER_SUITE | list_index;
 		rsp = intf->sendrecv(intf, &req);
 		if (rsp == NULL) {
 			lprintf(LOG_ERR, "Unable to Get Channel Cipher Suites");
 			return -1;
 		}
-		if (rsp->ccode > 0) {
+		if (rsp->ccode || rsp->data_len < 1) {
 			lprintf(LOG_ERR, "Get Channel Cipher Suites failed: %s",
 					val2str(rsp->ccode, completion_code_vals));
 			return -1;
 		}
-	}
-
-	/* Copy last chunk */
-	if(rsp->data_len > 1) {
 		/*
 		 * We got back cipher suite data -- store it.
 		 * printf("copying data to offset %d\n", offset);
@@ -429,88 +459,46 @@ ipmi_get_channel_cipher_suites(struct ipmi_intf *intf, const char *payload_type,
 		 */
 		memcpy(cipher_suite_data + offset, rsp->data + 1, rsp->data_len - 1);
 		offset += rsp->data_len - 1;
-	}
 
-	/* We can chomp on all our data now. */
-	cipher_suite_data_length = offset;
-	offset = 0;
+		/*
+		 * Increment our list for the next call
+		 */
+		++list_index;
+	} while ((rsp->data_len == (sizeof(uint8_t) + MAX_CIPHER_SUITE_DATA_LEN)) &&
+			 (list_index < MAX_CIPHER_SUITE_RECORD_OFFSET));
 
-	if (! csv_output) {
-		printf("ID   IANA    Auth Alg        Integrity Alg   Confidentiality Alg\n");
-	}
-	while (offset < cipher_suite_data_length) {
-		if (cipher_suite_data[offset++] == 0xC0) {
-			/* standard type */
-			iana = 0;
+	*count = parse_channel_cipher_suite_data(cipher_suite_data, offset, suites,
+	                                         nr_suites);
+	return 0;
+}
 
-			/* Verify that we have at least a full record left; id + 3 algs */
-			if ((cipher_suite_data_length - offset) < 4) {
-				lprintf(LOG_ERR, "Incomplete data record in cipher suite data");
-				return -1;
-			}
-			cipher_suite_id = cipher_suite_data[offset++];
-		} else if (cipher_suite_data[offset++] == 0xC1) {
-			/* OEM record type */
-			/* Verify that we have at least a full record left
-			 * id + iana + 3 algs
-			 */
-			if ((cipher_suite_data_length - offset) < 4) {
-				lprintf(LOG_ERR, "Incomplete data record in cipher suite data");
-				return -1;
-			}
+static int
+ipmi_print_channel_cipher_suites(struct ipmi_intf *intf, const char *payload_type,
+		uint8_t channel)
+{
+	int rc;
+	size_t i = 0;
+	struct cipher_suite_info suites[MAX_CIPHER_SUITE_COUNT];
+	size_t nr_suites = sizeof(*suites);
 
-			cipher_suite_id = cipher_suite_data[offset++];
+	rc = ipmi_get_channel_cipher_suites(intf, payload_type, channel,
+			suites, &nr_suites);
 
-			/* Grab the IANA */
-			iana =
-				cipher_suite_data[offset]            | 
-				(cipher_suite_data[offset + 1] << 8) | 
-				(cipher_suite_data[offset + 2] << 16);
-			offset += 3;
-		} else {
-			lprintf(LOG_ERR, "Bad start of record byte in cipher suite data");
-			return -1;
-		}
+	if (rc < 0)
+		return rc;
 
-		/*
-		 * Grab the algorithms for this cipher suite.  I guess we can't be
-		 * sure of what order they'll come in.  Also, I suppose we default
-		 * to the NONE algorithm if one were absent.  This part of the spec is
-		 * poorly written -- I have read the errata document.  For now, I'm only
-		 * allowing one algorithm per type (auth, integrity, crypt) because I
-		 * don't I understand how it could be otherwise.
-		 */
-		auth_alg      = IPMI_AUTH_RAKP_NONE;
-		integrity_alg = IPMI_INTEGRITY_NONE;
-		crypt_alg     = IPMI_CRYPT_NONE;
-		
-		while (((cipher_suite_data[offset] & 0xC0) != 0xC0) &&
-			   ((cipher_suite_data_length - offset) > 0))
-		{
-			switch (cipher_suite_data[offset] & 0xC0)
-			{
-			case 0x00:
-				/* Authentication algorithm specifier */
-				auth_alg = cipher_suite_data[offset++] & 0x3F;
-				break;
-			case 0x40:
-				/* Interity algorithm specifier */
-				integrity_alg = cipher_suite_data[offset++] & 0x3F;
-				break;
-			case 0x80:
-				/* Confidentiality algorithm specifier */
-				crypt_alg = cipher_suite_data[offset++] & 0x3F;
-				break;
-			}
-		}
+	if (! csv_output) {
+		printf("ID   IANA    Auth Alg        Integrity Alg   Confidentiality Alg\n");
+	}
+	for (i = 0; i < nr_suites; i++) {
 		/* We have everything we need to spit out a cipher suite record */
 		printf((csv_output? "%d,%s,%s,%s,%s\n" :
 			"%-4d %-7s %-15s %-15s %-15s\n"),
-		       cipher_suite_id,
-		       iana_string(iana),
-		       val2str(auth_alg, ipmi_auth_algorithms),
-		       val2str(integrity_alg, ipmi_integrity_algorithms),
-		       val2str(crypt_alg, ipmi_encryption_algorithms));
+		       suites[i].cipher_suite_id,
+		       iana_string(suites[i].iana),
+		       val2str(suites[i].auth_alg, ipmi_auth_algorithms),
+		       val2str(suites[i].integrity_alg, ipmi_integrity_algorithms),
+		       val2str(suites[i].crypt_alg, ipmi_encryption_algorithms));
 	}
 	return 0;
 }
@@ -973,7 +961,7 @@ ipmi_channel_main(struct ipmi_intf *intf, int argc, char **argv)
 				return (-1);
 			}
 		}
-		retval = ipmi_get_channel_cipher_suites(intf,
+		retval = ipmi_print_channel_cipher_suites(intf,
 							argv[1], /* ipmi | sol */
 							channel);
 	} else if (strncmp(argv[0], "setkg", 5) == 0) {
diff --git a/lib/ipmi_main.c b/lib/ipmi_main.c
index 811c80b..6aee102 100644
--- a/lib/ipmi_main.c
+++ b/lib/ipmi_main.c
@@ -323,6 +323,7 @@ ipmi_main(int argc, char ** argv,
 	uint8_t target_addr = 0;
 	uint8_t target_channel = 0;
 
+	uint8_t u8tmp = 0;
 	uint8_t transit_addr = 0;
 	uint8_t transit_channel = 0;
 	uint8_t target_lun     = 0;
@@ -347,7 +348,10 @@ ipmi_main(int argc, char ** argv,
 	char * seloem   = NULL;
 	int port = 0;
 	int devnum = 0;
-	int cipher_suite_id = 3; /* See table 22-19 of the IPMIv2 spec */
+#ifdef IPMI_INTF_LANPLUS
+	/* lookup best cipher suite available */
+	enum cipher_suite_ids cipher_suite_id = IPMI_LANPLUS_CIPHER_SUITE_RESERVED;
+#endif /* IPMI_INTF_LANPLUS */
 	int argflag, i, found;
 	int rc = -1;
 	int ai_family = AF_UNSPEC;
@@ -425,19 +429,18 @@ ipmi_main(int argc, char ** argv,
 				goto out_free;
 			}
 			break;
+#ifdef IPMI_INTF_LANPLUS
 		case 'C':
-			if (str2int(optarg, &cipher_suite_id) != 0) {
-				lprintf(LOG_ERR, "Invalid parameter given or out of range for '-C'.");
-				rc = -1;
-				goto out_free;
-			}
-			/* add check Cipher is -gt 0 */
-			if (cipher_suite_id < 0) {
-				lprintf(LOG_ERR, "Cipher suite ID %i is invalid.", cipher_suite_id);
+			/* Cipher Suite ID is a byte as per IPMI specification */
+			if (str2uchar(optarg, &u8tmp) != 0) {
+				lprintf(LOG_ERR, "Invalid parameter given or out of "
+				                 "range [0-255] for '-C'.");
 				rc = -1;
 				goto out_free;
 			}
+			cipher_suite_id = u8tmp;
 			break;
+#endif /* IPMI_INTF_LANPLUS */
 		case 'v':
 			verbose++;
 			break;
@@ -870,7 +873,9 @@ ipmi_main(int argc, char ** argv,
 
 	ipmi_intf_session_set_lookupbit(ipmi_main_intf, lookupbit);
 	ipmi_intf_session_set_sol_escape_char(ipmi_main_intf, sol_escape_char);
+#ifdef IPMI_INTF_LANPLUS
 	ipmi_intf_session_set_cipher_suite_id(ipmi_main_intf, cipher_suite_id);
+#endif /* IPMI_INTF_LANPLUS */
 
 	ipmi_main_intf->devnum = devnum;
 
diff --git a/src/plugins/ipmi_intf.c b/src/plugins/ipmi_intf.c
index 1d9e87b..00b0918 100644
--- a/src/plugins/ipmi_intf.c
+++ b/src/plugins/ipmi_intf.c
@@ -252,11 +252,14 @@ ipmi_intf_session_set_lookupbit(struct ipmi_intf * intf, uint8_t lookupbit)
 	intf->ssn_params.lookupbit = lookupbit;
 }
 
+#ifdef IPMI_INTF_LANPLUS
 void
-ipmi_intf_session_set_cipher_suite_id(struct ipmi_intf * intf, uint8_t cipher_suite_id)
+ipmi_intf_session_set_cipher_suite_id(struct ipmi_intf * intf,
+		enum cipher_suite_ids cipher_suite_id)
 {
 	intf->ssn_params.cipher_suite_id = cipher_suite_id;
 }
+#endif /* IPMI_INTF_LANPLUS */
 
 void
 ipmi_intf_session_set_sol_escape_char(struct ipmi_intf * intf, char sol_escape_char)
diff --git a/src/plugins/lanplus/lanplus.c b/src/plugins/lanplus/lanplus.c
index a0e388c..3087348 100644
--- a/src/plugins/lanplus/lanplus.c
+++ b/src/plugins/lanplus/lanplus.c
@@ -164,114 +164,109 @@ extern int verbose;
  * returns 0 on success
  *         1 on failure
  */
-int lanplus_get_requested_ciphers(int       cipher_suite_id,
+int lanplus_get_requested_ciphers(enum cipher_suite_ids cipher_suite_id,
 								  uint8_t * auth_alg,
 								  uint8_t * integrity_alg,
 								  uint8_t * crypt_alg)
 {
-#ifdef HAVE_CRYPTO_SHA256
-	if ((cipher_suite_id < 0) || (cipher_suite_id > 17)) {
-		return 1;
-	}
-#else
-	if ((cipher_suite_id < 0) || (cipher_suite_id > 14))
-		return 1;
-#endif /* HAVE_CRYPTO_SHA256 */
 		/* See table 22-19 for the source of the statement */
 	switch (cipher_suite_id)
 	{
-	case 0:
+	case IPMI_LANPLUS_CIPHER_SUITE_0:
 		*auth_alg      = IPMI_AUTH_RAKP_NONE;
 		*integrity_alg = IPMI_INTEGRITY_NONE;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 1:
+	case IPMI_LANPLUS_CIPHER_SUITE_1:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA1;
 		*integrity_alg = IPMI_INTEGRITY_NONE;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 2:
+	case IPMI_LANPLUS_CIPHER_SUITE_2:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA1;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA1_96;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 3:
+	case IPMI_LANPLUS_CIPHER_SUITE_3:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA1;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA1_96;
 		*crypt_alg     = IPMI_CRYPT_AES_CBC_128;
 		break;
-	case 4:
+	case IPMI_LANPLUS_CIPHER_SUITE_4:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA1;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA1_96;
 		*crypt_alg     = IPMI_CRYPT_XRC4_128;
 		break;
-	case 5:
+	case IPMI_LANPLUS_CIPHER_SUITE_5:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA1;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA1_96;
 		*crypt_alg     = IPMI_CRYPT_XRC4_40;
 		break;
-	case 6:
+	case IPMI_LANPLUS_CIPHER_SUITE_6:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_NONE;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 7:
+	case IPMI_LANPLUS_CIPHER_SUITE_7:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 8:
+	case IPMI_LANPLUS_CIPHER_SUITE_8:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_AES_CBC_128;
 		break;
-	case 9:
+	case IPMI_LANPLUS_CIPHER_SUITE_9:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_XRC4_128;
 		break;
-	case 10:
+	case IPMI_LANPLUS_CIPHER_SUITE_10:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_XRC4_40;
 		break;
-	case 11:
+	case IPMI_LANPLUS_CIPHER_SUITE_11:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 12:
+	case IPMI_LANPLUS_CIPHER_SUITE_12:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_AES_CBC_128;
 		break;
-	case 13:
+	case IPMI_LANPLUS_CIPHER_SUITE_13:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_XRC4_128;
 		break;
-	case 14:
+	case IPMI_LANPLUS_CIPHER_SUITE_14:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_MD5;
 		*integrity_alg = IPMI_INTEGRITY_MD5_128;
 		*crypt_alg     = IPMI_CRYPT_XRC4_40;
 		break;
 #ifdef HAVE_CRYPTO_SHA256
-	case 15:
+	case IPMI_LANPLUS_CIPHER_SUITE_15:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA256;
 		*integrity_alg = IPMI_INTEGRITY_NONE;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 16:
+	case IPMI_LANPLUS_CIPHER_SUITE_16:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA256;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA256_128;
 		*crypt_alg     = IPMI_CRYPT_NONE;
 		break;
-	case 17:
+	case IPMI_LANPLUS_CIPHER_SUITE_17:
 		*auth_alg      = IPMI_AUTH_RAKP_HMAC_SHA256;
 		*integrity_alg = IPMI_INTEGRITY_HMAC_SHA256_128;
 		*crypt_alg     = IPMI_CRYPT_AES_CBC_128;
 		break;
 #endif /* HAVE_CRYPTO_SHA256 */
+	case IPMI_LANPLUS_CIPHER_SUITE_RESERVED:
+	default:
+		return 1;
 	}
 
 	return 0;
@@ -3441,6 +3436,57 @@ ipmi_set_session_privlvl_cmd(struct ipmi_intf * intf)
 	return 0;
 }
 
+static uint8_t
+ipmi_find_best_cipher_suite(struct ipmi_intf *intf)
+{
+	enum cipher_suite_ids best_suite = IPMI_LANPLUS_CIPHER_SUITE_RESERVED;
+#ifdef HAVE_CRYPTO_SHA256
+	struct cipher_suite_info suites[MAX_CIPHER_SUITE_COUNT];
+	size_t nr_suites = ARRAY_SIZE(suites);
+	/* cipher suite best order is chosen with this criteria:
+	 * HMAC-MD5 and MD5 are BAD; xRC4 is bad; AES128 is required
+	 * HMAC-SHA256 > HMAC-SHA1
+	 * secure authentication > encrypted content
+	 *
+	 * With xRC4 out, all cipher suites with MD5 out, and cipher suite 3 being
+	 * required by the spec, the only better defined standard cipher suite is
+	 * 17. So if SHA256 is available, we should try to use that, otherwise,
+	 * fall back to 3.
+	 */
+	const enum cipher_suite_ids cipher_order_preferred[] = {
+		IPMI_LANPLUS_CIPHER_SUITE_17,
+		IPMI_LANPLUS_CIPHER_SUITE_3,
+	};
+	const size_t nr_preferred = ARRAY_SIZE(cipher_order_preferred);
+	size_t ipref, i;
+
+	if (ipmi_get_channel_cipher_suites(intf, "ipmi", IPMI_LAN_CHANNEL_E,
+	                                   suites, &nr_suites) < 0)
+	{
+		/* default legacy behavior - cipher suite 3 if none is requested */
+		return IPMI_LANPLUS_CIPHER_SUITE_3;
+	}
+	for (ipref = 0; ipref < nr_preferred &&
+	                IPMI_LANPLUS_CIPHER_SUITE_RESERVED == best_suite; ipref++)
+	{
+		for (i = 0; i < nr_suites; i++) {
+			if (cipher_order_preferred[ipref] == suites[i].cipher_suite_id) {
+				best_suite = cipher_order_preferred[ipref];
+				break;
+			}
+		}
+	}
+#endif /* HAVE_CRYPTO_SHA256 */
+	if (IPMI_LANPLUS_CIPHER_SUITE_RESERVED == best_suite) {
+		/* IPMI 2.0 spec requires that cipher suite 3 is implemented
+		 * so we should always be able to fall back to that if better
+		 * options are not available. */
+		best_suite = IPMI_LANPLUS_CIPHER_SUITE_3;
+	}
+	lprintf(LOG_INFO, "Using best available cipher suite %d\n", best_suite);
+	return best_suite;
+}
+
 /**
  * ipmi_lanplus_open
  */
@@ -3514,6 +3560,16 @@ ipmi_lanplus_open(struct ipmi_intf * intf)
 		lprintf(LOG_INFO, "This BMC does not support IPMI v2 / RMCP+");
 		goto fail;
 	}
+	/*
+	 * If no cipher suite was provided, query the channel cipher suite list and
+	 * pick the best one available
+	 */
+	if (IPMI_LANPLUS_CIPHER_SUITE_RESERVED ==
+	    intf->ssn_params.cipher_suite_id)
+	{
+		ipmi_intf_session_set_cipher_suite_id(intf,
+			ipmi_find_best_cipher_suite(intf));
+	}
 
 	/*
 	 * If the open/rakp1/rakp3 sequence encounters a timeout, the whole sequence
@@ -3728,7 +3784,7 @@ static int ipmi_lanplus_setup(struct ipmi_intf * intf)
 
 static void ipmi_lanp_set_max_rq_data_size(struct ipmi_intf * intf, uint16_t size)
 {
-	if (intf->ssn_params.cipher_suite_id == 3) {
+	if (intf->ssn_params.cipher_suite_id == IPMI_LANPLUS_CIPHER_SUITE_3) {
 		/*
 		 * encrypted payload can only be multiple of 16 bytes
 		 */
@@ -3746,7 +3802,7 @@ static void ipmi_lanp_set_max_rq_data_size(struct ipmi_intf * intf, uint16_t siz
 
 static void ipmi_lanp_set_max_rp_data_size(struct ipmi_intf * intf, uint16_t size)
 {
-	if (intf->ssn_params.cipher_suite_id == 3) {
+	if (intf->ssn_params.cipher_suite_id == IPMI_LANPLUS_CIPHER_SUITE_3) {
 		/*
 		 * encrypted payload can only be multiple of 16 bytes
 		 */
-- 
2.20.1