Skip to contentSkip to conversation

improve MACsec support

PATCH Applied Davide Caratti 3 participants 7 messages Jul 26, 2016 – Sep 1, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover improve MACsec support

parsing of 'cipher' and 'icvlen' arguments has been improved; while at it, a couple of missing printouts have been added to usage() functions in "ip addr help" and "ip link help". Finally, some errors in the man pages have been fixed.

Davide Caratti (3):
man: macsec: fix macsec related typos
ip {link,address}: add 'macsec' item to TYPE list
macsec: cipher and icvlen can be set separately

 ip/ipaddress.c           |  2 +-
 ip/iplink.c              |  2 +-
 ip/ipmacsec.c            | 52 ++++++++++++++++--------------------------------
 man/man8/ip-address.8.in |  3 ++-
 man/man8/ip-link.8.in    | 17 +++++++++++-----
 man/man8/ip-macsec.8     | 13 ++++++++----
 6 files changed, 42 insertions(+), 47 deletions(-)
Signature · 1 line
2.5.5
#1 lore raw

PATCH 1/3 man: macsec: fix macsec related typos

- ip-macsec.8: fix wrong 'device' keyword in 'ip link add device eth0'; add missing description of 'validate' keyword; remove spurious bracket near 'encrypt' keyword; add missing reference to configuration of 'port' and 'sci'
- ip-link.8 fix wrong 'es' and 'encoding' keywords in MACsec section

---
 man/man8/ip-link.8.in |  8 ++++----
 man/man8/ip-macsec.8  | 11 ++++++++---
 2 files changed, 12 insertions(+), 7 deletions(-)
man/man8/ip-link.8.in +4 -4
index 2cd6133..c91ef95 100644
@@ -925,14 +925,14 @@ the following additional arguments are supported:
 .BR encrypt " {"
 .BR on " | " off " } ] [ "
 .BR send_sci " { " on " | " off " } ] ["
-.BR es " { " on " | " off " } ] ["
+.BR end_station " { " on " | " off " } ] ["
 .BR scb " { " on " | " off " } ] ["
 .BR protect " { " on " | " off " } ] ["
 .BR replay " { " on " | " off " }"
 .BR window " { "
 .IR 0..2^32-1 " } ] ["
 .BR validate " { " strict " | " check " | " disabled " } ] ["
-.BR encoding " { "
+.BR encodingsa " { "
 .IR 0..3 " } ]"
 
 .in +8
@@ -957,7 +957,7 @@ the following additional arguments are supported:
 - specifies whether the SCI is included in every packet, or only when it is necessary.
 
 .sp
-.BR "es on " or " es off"
+.BR "end_station on " or " end_station off"
 - sets the End Station bit.
 
 .sp
@@ -985,7 +985,7 @@ the following additional arguments are supported:
 - sets the validation mode on the device.
 
 .sp
-.BI encoding " AN "
+.BI encodingsa " AN "
 - sets the active secure association for transmission.
 
 .in -8
man/man8/ip-macsec.8 +8 -3
index e8455d7..f928c43 100644
@@ -3,10 +3,14 @@
 ip-macsec \- MACsec device configuration
 .SH "SYNOPSIS"
 .BI "ip link add link " DEVICE " name " NAME " type macsec "
-[ [
+[
+.BI port " PORT"
+|
+.BI sci  " SCI"
+] [ [
 .BR cipher " { " default " | " gcm-aes-128 " } ] "
 .BI icvlen " ICVLEN"
-] [ [
+] [
 .BR encrypt " { " on " | " off " } ] ["
 .BR send_sci " { " on " | " off " } ] ["
 .BR end_station " { " on " | " off " } ] ["
@@ -15,6 +19,7 @@ ip-macsec \- MACsec device configuration
 .BR replay " { " on " | " off " } ] ["
 .BI window " WINDOW"
 ] [
+.BR validate " { " strict " | " check " | " disabled " } ] ["
 .BI encodingsa " SA"
 ]
 
@@ -74,7 +79,7 @@ type.
 .PP
 .SS Create a MACsec device on link eth0
 .nf
-# ip link add device eth0 macsec0 type macsec port 11 encrypt on
+# ip link add link eth0 macsec0 type macsec port 11 encrypt on
 .PP
 .SS Configure a secure association on that device
 .nf
Signature · 1 line
2.5.5
#2 lore raw

PATCH 2/3 ip {link,address}: add 'macsec' item to TYPE list

In reply to Davide Caratti

fix output of "ip address help" and "ip link help". Update TYPE list in man pages ip-address.8 and ip-link.8 as well.

---
 ip/ipaddress.c           | 2 +-
 ip/iplink.c              | 2 +-
 man/man8/ip-address.8.in | 3 ++-
 man/man8/ip-link.8.in    | 3 ++-
 4 files changed, 6 insertions(+), 4 deletions(-)
ip/ipaddress.c +1 -1
index 60862c5..ab4b1b1 100644
@@ -97,7 +97,7 @@ static void usage(void)
 	fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
 	fprintf(stderr, "          bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan | lowpan |\n");
 	fprintf(stderr, "          gre | gretap | ip6gre | ip6gretap | vti | nlmon | can |\n");
-	fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf | hsr}\n");
+	fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf | hsr | macsec }\n");
 
 	exit(-1);
 }
ip/iplink.c +1 -1
index ef17fd9..f9a7e09 100644
@@ -96,7 +96,7 @@ void iplink_usage(void)
 		fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
 		fprintf(stderr, "          bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n");
 		fprintf(stderr, "          gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n");
-		fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf }\n");
+		fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf | macsec }\n");
 	}
 	exit(-1);
 }
man/man8/ip-address.8.in +2 -1
index 7d6eb9b..4338581 100644
@@ -127,7 +127,8 @@ ip-address \- protocol address management
 .BR nlmon " |"
 .BR ipvlan " |"
 .BR lowpan " |"
-.BR geneve " ]"
+.BR geneve " |"
+.BR macsec " ]"
 
 .SH "DESCRIPTION"
 The
man/man8/ip-link.8.in +2 -1
index c91ef95..ad49c9d 100644
@@ -184,7 +184,8 @@ ip-link \- network device configuration
 .BR ipvlan " |"
 .BR lowpan " |"
 .BR geneve " |"
-.BR vrf " ]"
+.BR vrf " |"
+.BR macsec " ]"
 
 .ti -8
 .IR ETYPE " := [ " TYPE " |"
Signature · 1 line
2.5.5
#3 lore raw

PATCH 3/3 macsec: cipher and icvlen can be set separately

In reply to Davide Caratti

since kernel driver has valid default values for 'cipher' and 'icvlen', there is no need for requiring users to specify both of them when a new link is added. Also, prompt an error message and exit with appropriate exit status in case of unsupported cipher suite.

---
 ip/ipmacsec.c         | 52 +++++++++++++++++----------------------------------
 man/man8/ip-link.8.in |  6 ++++++
 man/man8/ip-macsec.8  |  4 ++--
 3 files changed, 25 insertions(+), 37 deletions(-)
ip/ipmacsec.c +17 -35
index 34ba341..329be00 100644
@@ -1071,34 +1071,6 @@ static void macsec_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 }
 
-
-static int do_cipher_suite(struct cipher_args *cipher, int *argcp,
-			   char ***argvp)
-{
-	char **argv = *argvp;
-	int argc = *argcp;
-
-	if (argc == 0)
-		return -1;
-
-	if (strcmp(*argv, "default") == 0 ||
-	    strcmp(*argv, "gcm-aes-128") == 0 ||
-	    strcmp(*argv, "GCM-AES-128") == 0)
-		cipher->id = MACSEC_DEFAULT_CIPHER_ID;
-	NEXT_ARG();
-
-	if (strcmp(*argv, "icvlen") == 0) {
-		NEXT_ARG();
-		if (cipher->icv_len != 0)
-			duparg2("icvlen", "icvlen");
-		get_icvlen(&cipher->icv_len, *argv);
-	}
-	*argcp = argc;
-	*argvp = argv;
-
-	return 0;
-}
-
 static bool check_txsc_flags(bool es, bool scb, bool sci)
 {
 	if (sci && (es || scb))
@@ -1112,7 +1084,8 @@ static void usage(FILE *f)
 {
 	fprintf(f,
 		"Usage: ... macsec [ port PORT | sci SCI ]\n"
-		"                  [ cipher CIPHER_SUITE ]\n"
+		"                  [ cipher { default | gcm-aes-128 } ]\n"
+		"                  [ icvlen { 8..16 } ]\n"
 		"                  [ encrypt { on | off } ]\n"
 		"                  [ send_sci { on | off } ]\n"
 		"                  [ end_station { on | off } ]\n"
@@ -1122,7 +1095,6 @@ static void usage(FILE *f)
 		"                  [ validate { strict | check | disabled } ]\n"
 		"                  [ encodingsa { 0..3 } ]\n"
 		);
-	fprintf(f, "CIPHER_SUITE := [ default = gcm-aes-128 ] icvlen { 8..32 }\n");
 }
 
 static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
@@ -1154,11 +1126,21 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 
 	while (argc > 0) {
 		if (strcmp(*argv, "cipher") == 0) {
+			NEXT_ARG();
 			if (cipher.id)
-				duparg2("cipher", "cipher");
+				duparg("cipher", *argv);
+			if (strcmp(*argv, "default") == 0 ||
+			    strcmp(*argv, "gcm-aes-128") == 0 ||
+			    strcmp(*argv, "GCM-AES-128") == 0)
+				cipher.id = MACSEC_DEFAULT_CIPHER_ID;
+			else
+				invarg("expected: default or gcm-aes-128",
+				       *argv);
+		} else if (strcmp(*argv, "icvlen") == 0) {
 			NEXT_ARG();
-			if (do_cipher_suite(&cipher, &argc, &argv))
-				return -1;
+			if (cipher.icv_len)
+				duparg("icvlen", *argv);
+			get_icvlen(&cipher.icv_len, *argv);
 		} else if (strcmp(*argv, "encrypt") == 0) {
 			NEXT_ARG();
 			int i;
@@ -1264,12 +1246,12 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 		return -1;
 	}
 
-	if (cipher.id) {
+	if (cipher.id)
 		addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_CIPHER_SUITE,
 			  &cipher.id, sizeof(cipher.id));
+	if (cipher.icv_len)
 		addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_ICV_LEN,
 			  &cipher.icv_len, sizeof(cipher.icv_len));
-	}
 
 	if (replay_protect != -1) {
 		addattr32(hdr, MACSEC_BUFLEN, IFLA_MACSEC_WINDOW, window);
man/man8/ip-link.8.in +6 -0
index ad49c9d..f4782ee 100644
@@ -923,6 +923,8 @@ the following additional arguments are supported:
 ] [
 .BI cipher " CIPHER_SUITE"
 ] [
+.BR icvlen " { "
+.IR 8..16 " } ] ["
 .BR encrypt " {"
 .BR on " | " off " } ] [ "
 .BR send_sci " { " on " | " off " } ] ["
@@ -950,6 +952,10 @@ the following additional arguments are supported:
 - defines the cipher suite to use.
 
 .sp
+.BI icvlen " LENGTH "
+- sets the length of the Integrity Check Value (ICV).
+
+.sp
 .BR "encrypt on " or " encrypt off"
 - switches between authenticated encryption, or authenticity mode only.
 
man/man8/ip-macsec.8 +2 -2
index f928c43..105aeec 100644
@@ -7,8 +7,8 @@ ip-macsec \- MACsec device configuration
 .BI port " PORT"
 |
 .BI sci  " SCI"
-] [ [
-.BR cipher " { " default " | " gcm-aes-128 " } ] "
+] [
+.BR cipher " { " default " | " gcm-aes-128 " } ] ["
 .BI icvlen " ICVLEN"
 ] [
 .BR encrypt " { " on " | " off " } ] ["
Signature · 1 line
2.5.5
Acked-by on the v1 series
#4 lore raw
In reply to Davide Caratti
Quoted 12 lines · Davide Caratti

parsing of 'cipher' and 'icvlen' arguments has been improved; while at it, a couple of missing printouts have been added to usage() functions in "ip addr help" and "ip link help". Finally, some errors in the man pages have been fixed.

Davide Caratti (3):
man: macsec: fix macsec related typos
ip {link,address}: add 'macsec' item to TYPE list
macsec: cipher and icvlen can be set separately

Quoted 7 lines
 ip/ipaddress.c           |  2 +-
 ip/iplink.c              |  2 +-
 ip/ipmacsec.c            | 52 ++++++++++++++++--------------------------------
 man/man8/ip-address.8.in |  3 ++-
 man/man8/ip-link.8.in    | 17 +++++++++++-----
 man/man8/ip-macsec.8     | 13 ++++++++----
 6 files changed, 42 insertions(+), 47 deletions(-)

Stephen, can this go in before the 4.7 release? This fixes parsing bugs and docs, it would be nice to have this.

Thanks,

Signature · 1 line
Sabrina
#5 lore raw
Quoted 21 lines · On Tue, 26 Jul 2016 15:44:39 +0200 Sabrina Dubroca <[email protected]> wrote:
Quoted 12 lines · Davide Caratti

parsing of 'cipher' and 'icvlen' arguments has been improved; while at it, a couple of missing printouts have been added to usage() functions in "ip addr help" and "ip link help". Finally, some errors in the man pages have been fixed.

Davide Caratti (3):
man: macsec: fix macsec related typos
ip {link,address}: add 'macsec' item to TYPE list
macsec: cipher and icvlen can be set separately

Quoted 7 lines
 ip/ipaddress.c           |  2 +-
 ip/iplink.c              |  2 +-
 ip/ipmacsec.c            | 52 ++++++++++++++++--------------------------------
 man/man8/ip-address.8.in |  3 ++-
 man/man8/ip-link.8.in    | 17 +++++++++++-----
 man/man8/ip-macsec.8     | 13 ++++++++----
 6 files changed, 42 insertions(+), 47 deletions(-)  
Quoted 8 lines

Stephen, can this go in before the 4.7 release? This fixes parsing bugs and docs, it would be nice to have this.

Thanks,

Will try, still have a couple open things for 4.7

#6 lore raw
In reply to Davide Caratti
Quoted 12 lines · On Tue, 26 Jul 2016 11:03:17 +0200 Davide Caratti <[email protected]> wrote:

parsing of 'cipher' and 'icvlen' arguments has been improved; while at it, a couple of missing printouts have been added to usage() functions in "ip addr help" and "ip link help". Finally, some errors in the man pages have been fixed.

Davide Caratti (3):
man: macsec: fix macsec related typos
ip {link,address}: add 'macsec' item to TYPE list
macsec: cipher and icvlen can be set separately

Quoted 8 lines
 ip/ipaddress.c           |  2 +-
 ip/iplink.c              |  2 +-
 ip/ipmacsec.c            | 52 ++++++++++++++++--------------------------------
 man/man8/ip-address.8.in |  3 ++-
 man/man8/ip-link.8.in    | 17 +++++++++++-----
 man/man8/ip-macsec.8     | 13 ++++++++----
 6 files changed, 42 insertions(+), 47 deletions(-)

Applied