diff mbox series

[v2,iproute2,1/1] xfrm: add option to hide keys in state output

Message ID 20190118191217.131649-2-benedictwong@google.com
State Accepted
Delegated to: David Ahern
Headers show
Series Hide keys for state output - changelog | expand

Commit Message

Benedict Wong Jan. 18, 2019, 7:12 p.m. UTC 
ip xfrm state show currently dumps keys unconditionally. This limits its
use in logging, as security information can be leaked.

This patch adds a nokeys option to ip xfrm ( state show | monitor ), which
prevents the printing of keys. This allows ip xfrm state show to be used
in logging without exposing keys.

Signed-off-by: Benedict Wong <benedictwong@google.com>
---
 ip/ipxfrm.c        | 49 +++++++++++++++++++++++++---------------------
 ip/xfrm.h          |  5 +++--
 ip/xfrm_monitor.c  |  7 +++++--
 ip/xfrm_state.c    | 27 ++++++++++++++++++++-----
 man/man8/ip-xfrm.8 | 15 +++++++++++++-
 5 files changed, 71 insertions(+), 32 deletions(-)

Comments

David Ahern Jan. 21, 2019, 4:33 p.m. UTC | #1 
On 1/18/19 12:12 PM, Benedict Wong wrote:
> ip xfrm state show currently dumps keys unconditionally. This limits its
> use in logging, as security information can be leaked.
> 
> This patch adds a nokeys option to ip xfrm ( state show | monitor ), which
> prevents the printing of keys. This allows ip xfrm state show to be used
> in logging without exposing keys.
> 
> Signed-off-by: Benedict Wong <benedictwong@google.com>
> ---
>  ip/ipxfrm.c        | 49 +++++++++++++++++++++++++---------------------
>  ip/xfrm.h          |  5 +++--
>  ip/xfrm_monitor.c  |  7 +++++--
>  ip/xfrm_state.c    | 27 ++++++++++++++++++++-----
>  man/man8/ip-xfrm.8 | 15 +++++++++++++-
>  5 files changed, 71 insertions(+), 32 deletions(-)
> 

applied to iproute2-next. Thanks
diff mbox series

Patch

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 2dea4e37..b153b863 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -497,7 +497,8 @@  void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 }
 
 static void __xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
-			      FILE *fp, const char *prefix, int newline)
+			      FILE *fp, const char *prefix, int newline,
+			      bool nokeys)
 {
 	int keylen;
 	int i;
@@ -521,7 +522,9 @@  static void __xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
 		goto fin;
 	}
 
-	if (keylen > 0) {
+	if (nokeys)
+		fprintf(fp, "<<Keys hidden>>");
+	else if (keylen > 0) {
 		fprintf(fp, "0x");
 		for (i = 0; i < keylen; i++)
 			fprintf(fp, "%.2x", (unsigned char)algo->alg_key[i]);
@@ -536,13 +539,13 @@  static void __xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
 }
 
 static inline void xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
-				   FILE *fp, const char *prefix)
+				   FILE *fp, const char *prefix, bool nokeys)
 {
-	return __xfrm_algo_print(algo, type, len, fp, prefix, 1);
+	return __xfrm_algo_print(algo, type, len, fp, prefix, 1, nokeys);
 }
 
 static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
-			    FILE *fp, const char *prefix)
+			    FILE *fp, const char *prefix, bool nokeys)
 {
 	struct xfrm_algo *base_algo = alloca(sizeof(*base_algo) + algo->alg_key_len / 8);
 
@@ -550,7 +553,8 @@  static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
 	base_algo->alg_key_len = algo->alg_key_len;
 	memcpy(base_algo->alg_key, algo->alg_key, algo->alg_key_len / 8);
 
-	__xfrm_algo_print(base_algo, XFRMA_ALG_AEAD, len, fp, prefix, 0);
+	__xfrm_algo_print(base_algo, XFRMA_ALG_AEAD, len, fp, prefix, 0,
+			  nokeys);
 
 	fprintf(fp, " %d", algo->alg_icv_len);
 
@@ -558,7 +562,7 @@  static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
 }
 
 static void xfrm_auth_trunc_print(struct xfrm_algo_auth *algo, int len,
-				  FILE *fp, const char *prefix)
+				  FILE *fp, const char *prefix, bool nokeys)
 {
 	struct xfrm_algo *base_algo = alloca(sizeof(*base_algo) + algo->alg_key_len / 8);
 
@@ -566,7 +570,8 @@  static void xfrm_auth_trunc_print(struct xfrm_algo_auth *algo, int len,
 	base_algo->alg_key_len = algo->alg_key_len;
 	memcpy(base_algo->alg_key, algo->alg_key, algo->alg_key_len / 8);
 
-	__xfrm_algo_print(base_algo, XFRMA_ALG_AUTH_TRUNC, len, fp, prefix, 0);
+	__xfrm_algo_print(base_algo, XFRMA_ALG_AUTH_TRUNC, len, fp, prefix, 0,
+			  nokeys);
 
 	fprintf(fp, " %d", algo->alg_trunc_len);
 
@@ -679,7 +684,7 @@  done:
 }
 
 void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
-		      FILE *fp, const char *prefix)
+		      FILE *fp, const char *prefix, bool nokeys)
 {
 	if (tb[XFRMA_MARK]) {
 		struct rtattr *rta = tb[XFRMA_MARK];
@@ -700,36 +705,36 @@  void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
 	if (tb[XFRMA_ALG_AUTH] && !tb[XFRMA_ALG_AUTH_TRUNC]) {
 		struct rtattr *rta = tb[XFRMA_ALG_AUTH];
 
-		xfrm_algo_print(RTA_DATA(rta),
-				XFRMA_ALG_AUTH, RTA_PAYLOAD(rta), fp, prefix);
+		xfrm_algo_print(RTA_DATA(rta), XFRMA_ALG_AUTH, RTA_PAYLOAD(rta),
+				fp, prefix, nokeys);
 	}
 
 	if (tb[XFRMA_ALG_AUTH_TRUNC]) {
 		struct rtattr *rta = tb[XFRMA_ALG_AUTH_TRUNC];
 
-		xfrm_auth_trunc_print(RTA_DATA(rta),
-				      RTA_PAYLOAD(rta), fp, prefix);
+		xfrm_auth_trunc_print(RTA_DATA(rta), RTA_PAYLOAD(rta), fp,
+				      prefix, nokeys);
 	}
 
 	if (tb[XFRMA_ALG_AEAD]) {
 		struct rtattr *rta = tb[XFRMA_ALG_AEAD];
 
-		xfrm_aead_print(RTA_DATA(rta),
-				RTA_PAYLOAD(rta), fp, prefix);
+		xfrm_aead_print(RTA_DATA(rta), RTA_PAYLOAD(rta), fp, prefix,
+				nokeys);
 	}
 
 	if (tb[XFRMA_ALG_CRYPT]) {
 		struct rtattr *rta = tb[XFRMA_ALG_CRYPT];
 
-		xfrm_algo_print(RTA_DATA(rta),
-				XFRMA_ALG_CRYPT, RTA_PAYLOAD(rta), fp, prefix);
+		xfrm_algo_print(RTA_DATA(rta), XFRMA_ALG_CRYPT,
+				RTA_PAYLOAD(rta), fp, prefix, nokeys);
 	}
 
 	if (tb[XFRMA_ALG_COMP]) {
 		struct rtattr *rta = tb[XFRMA_ALG_COMP];
 
-		xfrm_algo_print(RTA_DATA(rta),
-				XFRMA_ALG_COMP, RTA_PAYLOAD(rta), fp, prefix);
+		xfrm_algo_print(RTA_DATA(rta), XFRMA_ALG_COMP, RTA_PAYLOAD(rta),
+				fp, prefix, nokeys);
 	}
 
 	if (tb[XFRMA_ENCAP]) {
@@ -897,7 +902,7 @@  static int xfrm_selector_iszero(struct xfrm_selector *s)
 
 void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
-			    const char *title)
+			    const char *title, bool nokeys)
 {
 	char buf[STRBUF_SIZE] = {};
 	int force_spi = xfrm_xfrmproto_is_ipsec(xsinfo->id.proto);
@@ -943,7 +948,7 @@  void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 		fprintf(fp, " (0x%s)", strxf_mask8(xsinfo->flags));
 	fprintf(fp, "%s", _SL_);
 
-	xfrm_xfrma_print(tb, xsinfo->family, fp, buf);
+	xfrm_xfrma_print(tb, xsinfo->family, fp, buf, nokeys);
 
 	if (!xfrm_selector_iszero(&xsinfo->sel)) {
 		char sbuf[STRBUF_SIZE];
@@ -1071,7 +1076,7 @@  void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
 	if (show_stats > 0)
 		xfrm_lifetime_print(&xpinfo->lft, &xpinfo->curlft, fp, buf);
 
-	xfrm_xfrma_print(tb, xpinfo->sel.family, fp, buf);
+	xfrm_xfrma_print(tb, xpinfo->sel.family, fp, buf, false);
 }
 
 int xfrm_id_parse(xfrm_address_t *saddr, struct xfrm_id *id, __u16 *family,
diff --git a/ip/xfrm.h b/ip/xfrm.h
index 72390d79..9ba5ca61 100644
--- a/ip/xfrm.h
+++ b/ip/xfrm.h
@@ -105,6 +105,7 @@  struct xfrm_filter {
 extern struct xfrm_filter filter;
 
 int xfrm_state_print(struct nlmsghdr *n, void *arg);
+int xfrm_state_print_nokeys(struct nlmsghdr *n, void *arg);
 int xfrm_policy_print(struct nlmsghdr *n, void *arg);
 int do_xfrm_state(int argc, char **argv);
 int do_xfrm_policy(int argc, char **argv);
@@ -124,10 +125,10 @@  const char *strxf_ptype(__u8 ptype);
 void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 			 FILE *fp, const char *prefix);
 void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
-		      FILE *fp, const char *prefix);
+		      FILE *fp, const char *prefix, bool nokeys);
 void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
-			   const char *title);
+			   const char *title, bool nokeys);
 void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
 			    struct rtattr *tb[], FILE *fp, const char *prefix,
 			    const char *title);
diff --git a/ip/xfrm_monitor.c b/ip/xfrm_monitor.c
index 76905ed3..17117f41 100644
--- a/ip/xfrm_monitor.c
+++ b/ip/xfrm_monitor.c
@@ -35,10 +35,11 @@ 
 
 static void usage(void) __attribute__((noreturn));
 static int listen_all_nsid;
+static bool nokeys;
 
 static void usage(void)
 {
-	fprintf(stderr, "Usage: ip xfrm monitor [all-nsid] [ all | OBJECTS | help ]\n");
+	fprintf(stderr, "Usage: ip xfrm monitor [ nokeys ] [ all-nsid ] [ all | OBJECTS | help ]\n");
 	fprintf(stderr, "OBJECTS := { acquire | expire | SA | aevent | policy | report }\n");
 	exit(-1);
 }
@@ -197,7 +198,7 @@  static int xfrm_report_print(struct nlmsghdr *n, void *arg)
 
 	parse_rtattr(tb, XFRMA_MAX, XFRMREP_RTA(xrep), len);
 
-	xfrm_xfrma_print(tb, family, fp, "  ");
+	xfrm_xfrma_print(tb, family, fp, "  ", nokeys);
 
 	if (oneline)
 		fprintf(fp, "\n");
@@ -352,6 +353,8 @@  int do_xfrm_monitor(int argc, char **argv)
 		if (matches(*argv, "file") == 0) {
 			NEXT_ARG();
 			file = *argv;
+		} else if (strcmp(*argv, "nokeys") == 0) {
+			nokeys = true;
 		} else if (strcmp(*argv, "all") == 0) {
 			/* fall out */
 		} else if (matches(*argv, "all-nsid") == 0) {
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index e8c01746..09292da9 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -65,7 +65,9 @@  static void usage(void)
 	fprintf(stderr, "Usage: ip xfrm state allocspi ID [ mode MODE ] [ mark MARK [ mask MASK ] ]\n");
 	fprintf(stderr, "        [ reqid REQID ] [ seq SEQ ] [ min SPI max SPI ]\n");
 	fprintf(stderr, "Usage: ip xfrm state { delete | get } ID [ mark MARK [ mask MASK ] ]\n");
-	fprintf(stderr, "Usage: ip xfrm state { deleteall | list } [ ID ] [ mode MODE ] [ reqid REQID ]\n");
+	fprintf(stderr, "Usage: ip xfrm state deleteall [ ID ] [ mode MODE ] [ reqid REQID ]\n");
+	fprintf(stderr, "        [ flag FLAG-LIST ]\n");
+	fprintf(stderr, "Usage: ip xfrm state list [ nokeys ] [ ID ] [ mode MODE ] [ reqid REQID ]\n");
 	fprintf(stderr, "        [ flag FLAG-LIST ]\n");
 	fprintf(stderr, "Usage: ip xfrm state flush [ proto XFRM-PROTO ]\n");
 	fprintf(stderr, "Usage: ip xfrm state count\n");
@@ -908,7 +910,7 @@  static int xfrm_state_filter_match(struct xfrm_usersa_info *xsinfo)
 	return 1;
 }
 
-int xfrm_state_print(struct nlmsghdr *n, void *arg)
+static int __do_xfrm_state_print(struct nlmsghdr *n, void *arg, bool nokeys)
 {
 	FILE *fp = (FILE *)arg;
 	struct rtattr *tb[XFRMA_MAX+1];
@@ -979,7 +981,7 @@  int xfrm_state_print(struct nlmsghdr *n, void *arg)
 		xsinfo = RTA_DATA(tb[XFRMA_SA]);
 	}
 
-	xfrm_state_info_print(xsinfo, tb, fp, NULL, NULL);
+	xfrm_state_info_print(xsinfo, tb, fp, NULL, NULL, nokeys);
 
 	if (n->nlmsg_type == XFRM_MSG_EXPIRE) {
 		fprintf(fp, "\t");
@@ -994,6 +996,16 @@  int xfrm_state_print(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
+int xfrm_state_print(struct nlmsghdr *n, void *arg)
+{
+	return __do_xfrm_state_print(n, arg, false);
+}
+
+int xfrm_state_print_nokeys(struct nlmsghdr *n, void *arg)
+{
+	return __do_xfrm_state_print(n, arg, true);
+}
+
 static int xfrm_state_get_or_delete(int argc, char **argv, int delete)
 {
 	struct rtnl_handle rth;
@@ -1145,13 +1157,16 @@  static int xfrm_state_list_or_deleteall(int argc, char **argv, int deleteall)
 {
 	char *idp = NULL;
 	struct rtnl_handle rth;
+	bool nokeys = false;
 
 	if (argc > 0)
 		filter.use = 1;
 	filter.xsinfo.family = preferred_family;
 
 	while (argc > 0) {
-		if (strcmp(*argv, "mode") == 0) {
+		if (strcmp(*argv, "nokeys") == 0) {
+			nokeys = true;
+		} else if (strcmp(*argv, "mode") == 0) {
 			NEXT_ARG();
 			xfrm_mode_parse(&filter.xsinfo.mode, &argc, &argv);
 
@@ -1267,7 +1282,9 @@  static int xfrm_state_list_or_deleteall(int argc, char **argv, int deleteall)
 			exit(1);
 		}
 
-		if (rtnl_dump_filter(&rth, xfrm_state_print, stdout) < 0) {
+		rtnl_filter_t filter = nokeys ?
+				xfrm_state_print_nokeys : xfrm_state_print;
+		if (rtnl_dump_filter(&rth, filter, stdout) < 0) {
 			fprintf(stderr, "Dump terminated\n");
 			exit(1);
 		}
diff --git a/man/man8/ip-xfrm.8 b/man/man8/ip-xfrm.8
index 839e06aa..95478085 100644
--- a/man/man8/ip-xfrm.8
+++ b/man/man8/ip-xfrm.8
@@ -89,7 +89,7 @@  ip-xfrm \- transform configuration
 .IR MASK " ] ]"
 
 .ti -8
-.BR "ip xfrm state" " { " deleteall " | " list " } ["
+.BR "ip xfrm state " deleteall " ["
 .IR ID " ]"
 .RB "[ " mode
 .IR MODE " ]"
@@ -98,6 +98,17 @@  ip-xfrm \- transform configuration
 .RB "[ " flag
 .IR FLAG-LIST " ]"
 
+.ti -8
+.BR "ip xfrm state " list " ["
+.IR ID " ]"
+.RB "[ " nokeys " ]"
+.RB "[ " mode
+.IR MODE " ]"
+.RB "[ " reqid
+.IR REQID " ]"
+.RB "[ " flag
+.IR FLAG-LIST " ]"
+
 .ti -8
 .BR "ip xfrm state flush" " [ " proto
 .IR XFRM-PROTO " ]"
@@ -381,6 +392,8 @@  ip-xfrm \- transform configuration
 .BR "ip xfrm monitor" " ["
 .BI all-nsid
 ] [
+.BI nokeys
+] [
 .BI all
  |
 .IR LISTofXFRM-OBJECTS " ]"