You can subscribe to this list here.
2003 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
(6) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2004 |
Jan
(9) |
Feb
(11) |
Mar
(22) |
Apr
(73) |
May
(78) |
Jun
(146) |
Jul
(80) |
Aug
(27) |
Sep
(5) |
Oct
(14) |
Nov
(18) |
Dec
(27) |
2005 |
Jan
(20) |
Feb
(30) |
Mar
(19) |
Apr
(28) |
May
(50) |
Jun
(31) |
Jul
(32) |
Aug
(14) |
Sep
(36) |
Oct
(43) |
Nov
(74) |
Dec
(63) |
2006 |
Jan
(34) |
Feb
(32) |
Mar
(21) |
Apr
(76) |
May
(106) |
Jun
(72) |
Jul
(70) |
Aug
(175) |
Sep
(130) |
Oct
(39) |
Nov
(81) |
Dec
(43) |
2007 |
Jan
(81) |
Feb
(36) |
Mar
(20) |
Apr
(43) |
May
(54) |
Jun
(34) |
Jul
(44) |
Aug
(55) |
Sep
(44) |
Oct
(54) |
Nov
(43) |
Dec
(41) |
2008 |
Jan
(42) |
Feb
(84) |
Mar
(73) |
Apr
(30) |
May
(119) |
Jun
(54) |
Jul
(54) |
Aug
(93) |
Sep
(173) |
Oct
(130) |
Nov
(145) |
Dec
(153) |
2009 |
Jan
(59) |
Feb
(12) |
Mar
(28) |
Apr
(18) |
May
(56) |
Jun
(9) |
Jul
(28) |
Aug
(62) |
Sep
(16) |
Oct
(19) |
Nov
(15) |
Dec
(17) |
2010 |
Jan
(14) |
Feb
(36) |
Mar
(37) |
Apr
(30) |
May
(33) |
Jun
(53) |
Jul
(42) |
Aug
(50) |
Sep
(67) |
Oct
(66) |
Nov
(69) |
Dec
(36) |
2011 |
Jan
(52) |
Feb
(45) |
Mar
(49) |
Apr
(21) |
May
(34) |
Jun
(13) |
Jul
(19) |
Aug
(37) |
Sep
(43) |
Oct
(10) |
Nov
(23) |
Dec
(30) |
2012 |
Jan
(42) |
Feb
(36) |
Mar
(46) |
Apr
(25) |
May
(96) |
Jun
(146) |
Jul
(40) |
Aug
(28) |
Sep
(61) |
Oct
(45) |
Nov
(100) |
Dec
(53) |
2013 |
Jan
(79) |
Feb
(24) |
Mar
(134) |
Apr
(156) |
May
(118) |
Jun
(75) |
Jul
(278) |
Aug
(145) |
Sep
(136) |
Oct
(168) |
Nov
(137) |
Dec
(439) |
2014 |
Jan
(284) |
Feb
(158) |
Mar
(231) |
Apr
(275) |
May
(259) |
Jun
(91) |
Jul
(222) |
Aug
(215) |
Sep
(165) |
Oct
(166) |
Nov
(211) |
Dec
(150) |
2015 |
Jan
(164) |
Feb
(324) |
Mar
(299) |
Apr
(214) |
May
(111) |
Jun
(109) |
Jul
(105) |
Aug
(36) |
Sep
(58) |
Oct
(131) |
Nov
(68) |
Dec
(30) |
2016 |
Jan
(46) |
Feb
(87) |
Mar
(135) |
Apr
(174) |
May
(132) |
Jun
(135) |
Jul
(149) |
Aug
(125) |
Sep
(79) |
Oct
(49) |
Nov
(95) |
Dec
(102) |
2017 |
Jan
(104) |
Feb
(75) |
Mar
(72) |
Apr
(53) |
May
(18) |
Jun
(5) |
Jul
(14) |
Aug
(19) |
Sep
(2) |
Oct
(13) |
Nov
(21) |
Dec
(67) |
2018 |
Jan
(56) |
Feb
(50) |
Mar
(148) |
Apr
(41) |
May
(37) |
Jun
(34) |
Jul
(34) |
Aug
(11) |
Sep
(52) |
Oct
(48) |
Nov
(28) |
Dec
(46) |
2019 |
Jan
(29) |
Feb
(63) |
Mar
(95) |
Apr
(54) |
May
(14) |
Jun
(71) |
Jul
(60) |
Aug
(49) |
Sep
(3) |
Oct
(64) |
Nov
(115) |
Dec
(57) |
2020 |
Jan
(15) |
Feb
(9) |
Mar
(38) |
Apr
(27) |
May
(60) |
Jun
(53) |
Jul
(35) |
Aug
(46) |
Sep
(37) |
Oct
(64) |
Nov
(20) |
Dec
(25) |
2021 |
Jan
(20) |
Feb
(31) |
Mar
(27) |
Apr
(23) |
May
(21) |
Jun
(30) |
Jul
(30) |
Aug
(7) |
Sep
(18) |
Oct
|
Nov
(15) |
Dec
(4) |
2022 |
Jan
(3) |
Feb
(1) |
Mar
(10) |
Apr
|
May
(2) |
Jun
(26) |
Jul
(5) |
Aug
|
Sep
(1) |
Oct
(2) |
Nov
(9) |
Dec
(2) |
2023 |
Jan
(4) |
Feb
(4) |
Mar
(5) |
Apr
(10) |
May
(29) |
Jun
(17) |
Jul
|
Aug
|
Sep
(1) |
Oct
(1) |
Nov
(2) |
Dec
|
2024 |
Jan
|
Feb
(6) |
Mar
|
Apr
(1) |
May
(6) |
Jun
|
Jul
(5) |
Aug
|
Sep
(3) |
Oct
|
Nov
|
Dec
|
2025 |
Jan
|
Feb
(3) |
Mar
|
Apr
|
May
|
Jun
|
Jul
(6) |
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
From: Ying X. <yin...@wi...> - 2020-10-23 12:28:12
|
On 10/10/20 10:56 PM, jm...@re... wrote: > From: Jon Maloy <jm...@re...> > > TIPC reserves 64 service types for current and future internal use. > Therefore, the bind() function is meant to block regular user sockets > from being bound to these values, while it should let through such > bindings from internal users. > > However, since we at the design moment saw no way to distinguish > between regular and internal users the filter function ended up > with allowing all bindings of the types which were really in use > ([0,1]), and block all the rest ([2,63]). > > This is dangerous, since a regular user may bind to the service type > representing the topology server (TIPC_TOP_SRV == 1) or the one used > for indicating neigboring node status (TIPC_CFG_SRV == 0), and wreak > havoc for users of those services. I.e., practically all users. > > The reality is however that TIPC_CFG_SRV never is bound through the > bind() function, since it doesn't represent a regular socket, and > TIPC_TOP_SRV can easily be filtered out, since it is the very first > binding performed when the system is starting. > > We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV > to be bound once, and the correct behavior is achieved. This is what we > do in this commit. > > It should be noted that, although this is a change of the API semantics, > there is no risk we will break any currently working applications by > doing this. Any application trying to bind to the values in question > would be badly broken from the outset, so there is no chance we would > find any such applications in real-world production systems. > > Signed-off-by: Jon Maloy <jm...@re...> Acked-by: Ying Xue <yin...@wi...> > --- > net/tipc/socket.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/tipc/socket.c b/net/tipc/socket.c > index e795a8a2955b..67875a5761d0 100644 > --- a/net/tipc/socket.c > +++ b/net/tipc/socket.c > @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, > struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; > struct tipc_sock *tsk = tipc_sk(sk); > int res = -EINVAL; > + u32 stype, dnode; > > lock_sock(sk); > if (unlikely(!uaddr_len)) { > @@ -691,9 +692,10 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, > goto exit; > } > > - if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) && > - (addr->addr.nameseq.type != TIPC_TOP_SRV) && > - (addr->addr.nameseq.type != TIPC_CFG_SRV)) { > + stype = addr->addr.nameseq.type; > + if (stype < TIPC_RESERVED_TYPES && > + (stype != TIPC_TOP_SRV || > + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) { > res = -EACCES; > goto exit; > } > |
From: Ying X. <yin...@wi...> - 2020-10-23 12:25:26
|
On 10/21/20 10:58 PM, Jon Maloy wrote: > That is extremely simple. Take the hello_server under > tipcutils/demos/hello_world and change the server name to {1,1}. > It works! And it shouldn't of course, because it will steal traffic > directed to the toplogy server. Thanks Jon for your explanation! |
From: Tung N. <tun...@de...> - 2020-10-23 08:20:12
|
Commit ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()") replaced skb_unshare() with skb_copy() to not reduce the data reference counter of the original skb intentionally. This is not the correct way to handle the cloned skb because it causes memory leak in 2 following cases: 1/ Sending multicast messages via broadcast link The original skb list is cloned to the local skb list for local destination. After that, the data reference counter of each skb in the original list has the value of 2. This causes each skb not to be freed after receiving ACK: tipc_link_advance_transmq() { ... /* release skb */ __skb_unlink(skb, &l->transmq); kfree_skb(skb); <-- memory exists after being freed } 2/ Sending multicast messages via replicast link Similar to the above case, each skb cannot be freed after purging the skb list: tipc_mcast_xmit() { ... __skb_queue_purge(pkts); <-- memory exists after being freed } This commit fixes this issue by using skb_unshare() instead. Besides, to avoid use-after-free error reported by KASAN, kfree_skb(head) in tipc_buf_append() is called only if the pointer to the appending skb is not NULL. v2: improve condition for freeing the appending skb to cover all error cases. Fixes: ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()") Reported-by: Thang Hoang Ngo <tha...@de...> Signed-off-by: Tung Nguyen <tun...@de...> --- net/tipc/msg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 2a78aa701572..46c36c5093de 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -150,8 +150,7 @@ int tipc_buf_append(struct sk_buff **headbuf, struct sk_buff **buf) if (fragid == FIRST_FRAGMENT) { if (unlikely(head)) goto err; - if (skb_cloned(frag)) - frag = skb_copy(frag, GFP_ATOMIC); + frag = skb_unshare(frag, GFP_ATOMIC); if (unlikely(!frag)) goto err; head = *headbuf = frag; @@ -797,7 +796,8 @@ bool tipc_msg_reassemble(struct sk_buff_head *list, struct sk_buff_head *rcvq) return true; error: pr_warn("Failed do clone local mcast rcv buffer\n"); - kfree_skb(head); + if (head) + kfree_skb(head); return false; } -- 2.17.1 |
From: Tung N. <tun...@de...> - 2020-10-23 07:28:11
|
Commit ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()") replaced skb_unshare() with skb_copy() to not reduce the data reference counter of the original skb intentionally. This is not the correct way to handle the cloned skb because it causes memory leak in 2 following cases: 1/ Sending multicast messages via broadcast link The original skb list is cloned to the local skb list for local destination. After that, the data reference counter of each skb in the original list has the value of 2. This causes each skb not to be freed after receiving ACK: tipc_link_advance_transmq() { ... /* release skb */ __skb_unlink(skb, &l->transmq); kfree_skb(skb); <-- memory exists after being freed } 2/ Sending multicast messages via replicast link Similar to the above case, each skb cannot be freed after purging the skb list: tipc_mcast_xmit() { ... __skb_queue_purge(pkts); <-- memory exists after being freed } This commit fixes this issue by using skb_unshare() instead. Besides, to avoid use-after-free error reported by KASAN, kfree_skb(head) calling is removed from tipc_msg_reassemble() because the appending skb is always freed inside tipc_buf_append() in the case of error. Fixes: ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()") Reported-by: Thang Hoang Ngo <tha...@de...> Signed-off-by: Tung Nguyen <tun...@de...> --- net/tipc/msg.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 2a78aa701572..a193da26eb44 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -150,8 +150,7 @@ int tipc_buf_append(struct sk_buff **headbuf, struct sk_buff **buf) if (fragid == FIRST_FRAGMENT) { if (unlikely(head)) goto err; - if (skb_cloned(frag)) - frag = skb_copy(frag, GFP_ATOMIC); + frag = skb_unshare(frag, GFP_ATOMIC); if (unlikely(!frag)) goto err; head = *headbuf = frag; @@ -797,7 +796,6 @@ bool tipc_msg_reassemble(struct sk_buff_head *list, struct sk_buff_head *rcvq) return true; error: pr_warn("Failed do clone local mcast rcv buffer\n"); - kfree_skb(head); return false; } -- 2.17.1 |
From: Ying X. <yin...@wi...> - 2020-10-21 17:29:40
|
On 10/10/20 10:56 PM, jm...@re... wrote: > From: Jon Maloy <jm...@re...> > > TIPC reserves 64 service types for current and future internal use. > Therefore, the bind() function is meant to block regular user sockets > from being bound to these values, while it should let through such > bindings from internal users. > > However, since we at the design moment saw no way to distinguish > between regular and internal users the filter function ended up > with allowing all bindings of the types which were really in use > ([0,1]), and block all the rest ([2,63]). > > This is dangerous, since a regular user may bind to the service type > representing the topology server (TIPC_TOP_SRV == 1) or the one used > for indicating neigboring node status (TIPC_CFG_SRV == 0), and wreak > havoc for users of those services. I.e., practically all users. > Sorry, Jon. I could not understand how such scenarios described above happened. Can you please take an example to demonstrate a regular user can bind to the same service type as TIPC_TOP_SRV or TIPC_CFG_SRV under the current code logic? Thanks, Ying > The reality is however that TIPC_CFG_SRV never is bound through the > bind() function, since it doesn't represent a regular socket, and > TIPC_TOP_SRV can easily be filtered out, since it is the very first > binding performed when the system is starting. > > We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV > to be bound once, and the correct behavior is achieved. This is what we > do in this commit. > > It should be noted that, although this is a change of the API semantics, > there is no risk we will break any currently working applications by > doing this. Any application trying to bind to the values in question > would be badly broken from the outset, so there is no chance we would > find any such applications in real-world production systems. > > Signed-off-by: Jon Maloy <jm...@re...> > --- > net/tipc/socket.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/tipc/socket.c b/net/tipc/socket.c > index e795a8a2955b..67875a5761d0 100644 > --- a/net/tipc/socket.c > +++ b/net/tipc/socket.c > @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, > struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; > struct tipc_sock *tsk = tipc_sk(sk); > int res = -EINVAL; > + u32 stype, dnode; > > lock_sock(sk); > if (unlikely(!uaddr_len)) { > @@ -691,9 +692,10 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, > goto exit; > } > > - if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) && > - (addr->addr.nameseq.type != TIPC_TOP_SRV) && > - (addr->addr.nameseq.type != TIPC_CFG_SRV)) { > + stype = addr->addr.nameseq.type; > + if (stype < TIPC_RESERVED_TYPES && > + (stype != TIPC_TOP_SRV || > + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) { > res = -EACCES; > goto exit; > } > |
From: Jon M. <jm...@re...> - 2020-10-21 14:59:11
|
On 10/21/20 10:55 AM, Ying Xue wrote: > On 10/10/20 10:56 PM, jm...@re... wrote: >> From: Jon Maloy <jm...@re...> >> >> TIPC reserves 64 service types for current and future internal use. >> Therefore, the bind() function is meant to block regular user sockets >> from being bound to these values, while it should let through such >> bindings from internal users. >> >> However, since we at the design moment saw no way to distinguish >> between regular and internal users the filter function ended up >> with allowing all bindings of the types which were really in use >> ([0,1]), and block all the rest ([2,63]). >> >> This is dangerous, since a regular user may bind to the service type >> representing the topology server (TIPC_TOP_SRV == 1) or the one used >> for indicating neigboring node status (TIPC_CFG_SRV == 0), and wreak >> havoc for users of those services. I.e., practically all users. >> > Sorry, Jon. I could not understand how such scenarios described above > happened. Can you please take an example to demonstrate a regular user > can bind to the same service type as TIPC_TOP_SRV or TIPC_CFG_SRV under > the current code logic? > > Thanks, > Ying That is extremely simple. Take the hello_server under tipcutils/demos/hello_world and change the server name to {1,1}. It works! And it shouldn't of course, because it will steal traffic directed to the toplogy server. ///jon > >> The reality is however that TIPC_CFG_SRV never is bound through the >> bind() function, since it doesn't represent a regular socket, and >> TIPC_TOP_SRV can easily be filtered out, since it is the very first >> binding performed when the system is starting. >> >> We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV >> to be bound once, and the correct behavior is achieved. This is what we >> do in this commit. >> >> It should be noted that, although this is a change of the API semantics, >> there is no risk we will break any currently working applications by >> doing this. Any application trying to bind to the values in question >> would be badly broken from the outset, so there is no chance we would >> find any such applications in real-world production systems. >> >> Signed-off-by: Jon Maloy <jm...@re...> >> --- >> net/tipc/socket.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/net/tipc/socket.c b/net/tipc/socket.c >> index e795a8a2955b..67875a5761d0 100644 >> --- a/net/tipc/socket.c >> +++ b/net/tipc/socket.c >> @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, >> struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; >> struct tipc_sock *tsk = tipc_sk(sk); >> int res = -EINVAL; >> + u32 stype, dnode; >> >> lock_sock(sk); >> if (unlikely(!uaddr_len)) { >> @@ -691,9 +692,10 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, >> goto exit; >> } >> >> - if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) && >> - (addr->addr.nameseq.type != TIPC_TOP_SRV) && >> - (addr->addr.nameseq.type != TIPC_CFG_SRV)) { >> + stype = addr->addr.nameseq.type; >> + if (stype < TIPC_RESERVED_TYPES && >> + (stype != TIPC_TOP_SRV || >> + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) { >> res = -EACCES; >> goto exit; >> } >> |
From: David A. <ds...@gm...> - 2020-10-20 15:07:03
|
On 10/16/20 10:01 AM, Tuong Lien wrote: > This series adds two new options in the 'iproute2/tipc' command, enabling users > to use the new TIPC encryption features, i.e. the master key and rekeying which > have been recently merged in kernel. > > The help menu of the "tipc node set key" command is also updated accordingly: > > # tipc node set key --help > Usage: tipc node set key KEY [algname ALGNAME] [PROPERTIES] > tipc node set key rekeying REKEYING > > KEY > Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form: > [KEY: 16, 24 or 32 octets][SALT: 4 octets] > > ALGNAME > Cipher algorithm [default: "gcm(aes)"] > > PROPERTIES > master - Set KEY as a cluster master key > <empty> - Set KEY as a cluster key > nodeid NODEID - Set KEY as a per-node key for own or peer > > REKEYING > INTERVAL - Set rekeying interval (in minutes) [0: disable] > now - Trigger one (first) rekeying immediately > > EXAMPLES > tipc node set key this_is_a_master_key master > tipc node set key 0x746869735F69735F615F6B657931365F73616C74 > tipc node set key this_is_a_key16_salt algname "gcm(aes)" nodeid 1001002 > tipc node set key rekeying 600 > > Tuong Lien (2): > tipc: add option to set master key for encryption > tipc: add option to set rekeying for encryption > > tipc/cmdl.c | 2 +- > tipc/cmdl.h | 1 + > tipc/node.c | 81 +++++++++++++++++++++++++++++++++++++++-------------- > 3 files changed, 62 insertions(+), 22 deletions(-) > applied to iproute2-next |
From: Tuong L. <tuo...@de...> - 2020-10-18 05:56:05
|
As supported in kernel, the TIPC encryption rekeying can be tuned using the netlink attribute - 'TIPC_NLA_NODE_REKEYING'. Now we add the 'rekeying' option correspondingly to the 'tipc node set key' command so that user will be able to perform that tuning: tipc node set key rekeying REKEYING where the 'REKEYING' value can be: INTERVAL - Set rekeying interval (in minutes) [0: disable] now - Trigger one (first) rekeying immediately For example: $ tipc node set key rekeying 60 $ tipc node set key rekeying now The command's help menu is also updated with these descriptions for the new command option. Acked-by: Jon Maloy <jm...@re...> Signed-off-by: Tuong Lien <tuo...@de...> --- tipc/cmdl.c | 2 +- tipc/cmdl.h | 1 + tipc/node.c | 47 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/tipc/cmdl.c b/tipc/cmdl.c index f2f259cc..981e268e 100644 --- a/tipc/cmdl.c +++ b/tipc/cmdl.c @@ -33,7 +33,7 @@ static const struct cmd *find_cmd(const struct cmd *cmds, char *str) return match; } -static struct opt *find_opt(struct opt *opts, char *str) +struct opt *find_opt(struct opt *opts, char *str) { struct opt *o; struct opt *match = NULL; diff --git a/tipc/cmdl.h b/tipc/cmdl.h index 03db3599..dcade362 100644 --- a/tipc/cmdl.h +++ b/tipc/cmdl.h @@ -46,6 +46,7 @@ struct opt { char *val; }; +struct opt *find_opt(struct opt *opts, char *str); struct opt *get_opt(struct opt *opts, char *key); bool has_opt(struct opt *opts, char *key); int parse_opts(struct opt *opts, struct cmdl *cmdl); diff --git a/tipc/node.c b/tipc/node.c index 1ff0baa4..05246013 100644 --- a/tipc/node.c +++ b/tipc/node.c @@ -160,7 +160,8 @@ static int cmd_node_set_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd, static void cmd_node_set_key_help(struct cmdl *cmdl) { fprintf(stderr, - "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n\n" + "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n" + " %s node set key rekeying REKEYING\n\n" "KEY\n" " Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form:\n" " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" @@ -170,11 +171,16 @@ static void cmd_node_set_key_help(struct cmdl *cmdl) " master - Set KEY as a cluster master key\n" " <empty> - Set KEY as a cluster key\n" " nodeid NODEID - Set KEY as a per-node key for own or peer\n\n" + "REKEYING\n" + " INTERVAL - Set rekeying interval (in minutes) [0: disable]\n" + " now - Trigger one (first) rekeying immediately\n\n" "EXAMPLES\n" " %s node set key this_is_a_master_key master\n" " %s node set key 0x746869735F69735F615F6B657931365F73616C74\n" - " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n\n", - cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); + " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n" + " %s node set key rekeying 600\n\n", + cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], + cmdl->argv[0], cmdl->argv[0]); } static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, @@ -190,12 +196,15 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, { "algname", OPT_KEYVAL, NULL }, { "nodeid", OPT_KEYVAL, NULL }, { "master", OPT_KEY, NULL }, + { "rekeying", OPT_KEYVAL, NULL }, { NULL } }; struct nlattr *nest; - struct opt *opt_algname, *opt_nodeid, *opt_master; + struct opt *opt_algname, *opt_nodeid, *opt_master, *opt_rekeying; char buf[MNL_SOCKET_BUFFER_SIZE]; uint8_t id[TIPC_NODEID_LEN] = {0,}; + uint32_t rekeying = 0; + bool has_key = false; int keysize; char *str; @@ -204,17 +213,31 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, return -EINVAL; } + /* Check if command starts with opts i.e. "rekeying" opt without key */ + if (find_opt(opts, cmdl->argv[cmdl->optind])) + goto get_ops; /* Get user key */ + has_key = true; str = shift_cmdl(cmdl); if (str2key(str, &input.key)) { fprintf(stderr, "error, invalid key input\n"); return -EINVAL; } +get_ops: if (parse_opts(opts, cmdl) < 0) return -EINVAL; + /* Get rekeying time */ + opt_rekeying = get_opt(opts, "rekeying"); + if (opt_rekeying) { + if (!strcmp(opt_rekeying->val, "now")) + rekeying = TIPC_REKEYING_NOW; + else + rekeying = atoi(opt_rekeying->val); + } + /* Get algorithm name, default: "gcm(aes)" */ opt_algname = get_opt(opts, "algname"); if (!opt_algname) @@ -246,12 +269,16 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, } nest = mnl_attr_nest_start(nlh, TIPC_NLA_NODE); - keysize = tipc_aead_key_size(&input.key); - mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); - if (opt_nodeid) - mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); - if (opt_master) - mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + if (has_key) { + keysize = tipc_aead_key_size(&input.key); + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); + if (opt_nodeid) + mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); + if (opt_master) + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + } + if (opt_rekeying) + mnl_attr_put_u32(nlh, TIPC_NLA_NODE_REKEYING, rekeying); mnl_attr_nest_end(nlh, nest); return msg_doit(nlh, NULL, NULL); -- 2.26.2 |
From: Tuong L. <tuo...@de...> - 2020-10-18 05:55:09
|
In addition to the support of master key in kernel, we add the 'master' option to the 'tipc node set key' command for user to be able to specify a key as master key during the key setting. This is carried out by turning on the new netlink flag - 'TIPC_NLA_NODE_KEY_MASTER'. For example: $ tipc node set key "this_is_a_master_key" master The command's help menu is also updated to give a better description of all the available options. Acked-by: Jon Maloy <jm...@re...> Signed-off-by: Tuong Lien <tuo...@de...> --- tipc/node.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/tipc/node.c b/tipc/node.c index ffdaeaea..1ff0baa4 100644 --- a/tipc/node.c +++ b/tipc/node.c @@ -160,19 +160,21 @@ static int cmd_node_set_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd, static void cmd_node_set_key_help(struct cmdl *cmdl) { fprintf(stderr, - "Usage: %s node set key KEY [algname ALGNAME] [nodeid NODEID]\n\n" + "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n\n" + "KEY\n" + " Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form:\n" + " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" + "ALGNAME\n" + " Cipher algorithm [default: \"gcm(aes)\"]\n\n" "PROPERTIES\n" - " KEY - Symmetric KEY & SALT as a normal or hex string\n" - " that consists of two parts:\n" - " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" - " algname ALGNAME - Default: \"gcm(aes)\"\n\n" - " nodeid NODEID - Own or peer node identity to which the key will\n" - " be attached. If not present, the key is a cluster\n" - " key!\n\n" + " master - Set KEY as a cluster master key\n" + " <empty> - Set KEY as a cluster key\n" + " nodeid NODEID - Set KEY as a per-node key for own or peer\n\n" "EXAMPLES\n" - " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid node1\n" - " %s node set key 0x746869735F69735F615F6B657931365F73616C74 nodeid node2\n\n", - cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); + " %s node set key this_is_a_master_key master\n" + " %s node set key 0x746869735F69735F615F6B657931365F73616C74\n" + " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n\n", + cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); } static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, @@ -187,24 +189,21 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, struct opt opts[] = { { "algname", OPT_KEYVAL, NULL }, { "nodeid", OPT_KEYVAL, NULL }, + { "master", OPT_KEY, NULL }, { NULL } }; struct nlattr *nest; - struct opt *opt_algname, *opt_nodeid; + struct opt *opt_algname, *opt_nodeid, *opt_master; char buf[MNL_SOCKET_BUFFER_SIZE]; uint8_t id[TIPC_NODEID_LEN] = {0,}; int keysize; char *str; - if (help_flag) { + if (help_flag || cmdl->optind >= cmdl->argc) { (cmd->help)(cmdl); return -EINVAL; } - if (cmdl->optind >= cmdl->argc) { - fprintf(stderr, "error, missing key\n"); - return -EINVAL; - } /* Get user key */ str = shift_cmdl(cmdl); @@ -230,17 +229,30 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, return -EINVAL; } + /* Get master key indication */ + opt_master = get_opt(opts, "master"); + + /* Sanity check if wrong option */ + if (opt_nodeid && opt_master) { + fprintf(stderr, "error, per-node key cannot be master\n"); + return -EINVAL; + } + /* Init & do the command */ nlh = msg_init(buf, TIPC_NL_KEY_SET); if (!nlh) { fprintf(stderr, "error, message initialisation failed\n"); return -1; } + nest = mnl_attr_nest_start(nlh, TIPC_NLA_NODE); keysize = tipc_aead_key_size(&input.key); mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); if (opt_nodeid) mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); + if (opt_master) + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + mnl_attr_nest_end(nlh, nest); return msg_doit(nlh, NULL, NULL); } -- 2.26.2 |
From: Tuong L. <tuo...@de...> - 2020-10-18 05:54:16
|
This series adds two new options in the 'iproute2/tipc' command, enabling users to use the new TIPC encryption features, i.e. the master key and rekeying which have been recently merged in kernel. The help menu of the "tipc node set key" command is also updated accordingly: # tipc node set key --help Usage: tipc node set key KEY [algname ALGNAME] [PROPERTIES] tipc node set key rekeying REKEYING KEY Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form: [KEY: 16, 24 or 32 octets][SALT: 4 octets] ALGNAME Cipher algorithm [default: "gcm(aes)"] PROPERTIES master - Set KEY as a cluster master key <empty> - Set KEY as a cluster key nodeid NODEID - Set KEY as a per-node key for own or peer REKEYING INTERVAL - Set rekeying interval (in minutes) [0: disable] now - Trigger one (first) rekeying immediately EXAMPLES tipc node set key this_is_a_master_key master tipc node set key 0x746869735F69735F615F6B657931365F73616C74 tipc node set key this_is_a_key16_salt algname "gcm(aes)" nodeid 1001002 tipc node set key rekeying 600 Tuong Lien (2): tipc: add option to set master key for encryption tipc: add option to set rekeying for encryption tipc/cmdl.c | 2 +- tipc/cmdl.h | 1 + tipc/node.c | 81 +++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 62 insertions(+), 22 deletions(-) -- 2.26.2 |
From: Jon M. <jm...@re...> - 2020-10-15 19:21:47
|
On 10/14/20 3:07 AM, Hoang Huu Le wrote: > In commit 16ad3f4022bb > ("tipc: introduce variable window congestion control"), we applied > the algorithm to select window size from minimum window to the > configured maximum window for unicast link, and, besides we chose > to keep the window size for broadcast link unchanged and equal (i.e > fix window 50) > > However, when setting maximum window variable via command, the window > variable was re-initialized to unexpect value (i.e 32). > > We fix this by updating the fix window for broadcast as we stated. > > Fixes: 16ad3f4022bb ("tipc: introduce variable window congestion control") > Signed-off-by: Hoang Huu Le <hoa...@de...> > --- > net/tipc/bcast.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > index c77fd13e2777..bc88f21ec0b2 100644 > --- a/net/tipc/bcast.c > +++ b/net/tipc/bcast.c > @@ -589,7 +589,7 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 max_win) > if (max_win > TIPC_MAX_LINK_WIN) > return -EINVAL; > tipc_bcast_lock(net); > - tipc_link_set_queue_limits(l, BCLINK_WIN_MIN, max_win); > + tipc_link_set_queue_limits(l, tipc_link_min_win(l), max_win); > tipc_bcast_unlock(net); > return 0; > } Acked-by: Jon Maloy <jm...@re...> |
From: Hoang H. Le <hoa...@de...> - 2020-10-15 02:25:48
|
Thanks for your reviewing. Yes, in this commit, we intend to fix the queue calculation limited, and, besides we're planning to fix both in another fix. However, it should be used the default (i.e BCLINK_WIN_DEFAULT) one. Since, we keep to choose fix window size for broadcast link. Regards, Hoang > -----Original Message----- > From: Jakub Kicinski <ku...@ke...> > Sent: Thursday, October 15, 2020 7:47 AM > To: Hoang Huu Le <hoa...@de...> > Cc: tip...@li...; jm...@re...; ma...@do...; yin...@wi...; > ne...@vg... > Subject: Re: [net] tipc: re-configure queue limit for broadcast link > > On Tue, 13 Oct 2020 13:18:10 +0700 Hoang Huu Le wrote: > > The queue limit of the broadcast link is being calculated base on initial > > MTU. However, when MTU value changed (e.g manual changing MTU on NIC > > device, MTU negotiation etc.,) we do not re-calculate queue limit. > > This gives throughput does not reflect with the change. > > > > So fix it by calling the function to re-calculate queue limit of the > > broadcast link. > > > > Acked-by: Jon Maloy <jm...@re...> > > Signed-off-by: Hoang Huu Le <hoa...@de...> > > --- > > net/tipc/bcast.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > > index 940d176e0e87..c77fd13e2777 100644 > > --- a/net/tipc/bcast.c > > +++ b/net/tipc/bcast.c > > @@ -108,6 +108,7 @@ static void tipc_bcbase_select_primary(struct net *net) > > { > > struct tipc_bc_base *bb = tipc_bc_base(net); > > int all_dests = tipc_link_bc_peers(bb->link); > > + int max_win = tipc_link_max_win(bb->link); > > int i, mtu, prim; > > > > bb->primary_bearer = INVALID_BEARER_ID; > > @@ -121,8 +122,11 @@ static void tipc_bcbase_select_primary(struct net *net) > > continue; > > > > mtu = tipc_bearer_mtu(net, i); > > - if (mtu < tipc_link_mtu(bb->link)) > > + if (mtu < tipc_link_mtu(bb->link)) { > > tipc_link_set_mtu(bb->link, mtu); > > + tipc_link_set_queue_limits(bb->link, max_win, > > + max_win); > > Is max/max okay here? Other places seem to use BCLINK_WIN_MIN. > > > + } > > bb->bcast_support &= tipc_bearer_bcast_support(net, i); > > if (bb->dests[i] < all_dests) > > continue; |
From: Hoang H. Le <hoa...@de...> - 2020-10-14 07:41:50
|
In commit 16ad3f4022bb ("tipc: introduce variable window congestion control"), we applied the algorithm to select window size from minimum window to the configured maximum window for unicast link, and, besides we chose to keep the window size for broadcast link unchanged and equal (i.e fix window 50) However, when setting maximum window variable via command, the window variable was re-initialized to unexpect value (i.e 32). We fix this by updating the fix window for broadcast as we stated. Fixes: 16ad3f4022bb ("tipc: introduce variable window congestion control") Signed-off-by: Hoang Huu Le <hoa...@de...> --- net/tipc/bcast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index c77fd13e2777..bc88f21ec0b2 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -589,7 +589,7 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 max_win) if (max_win > TIPC_MAX_LINK_WIN) return -EINVAL; tipc_bcast_lock(net); - tipc_link_set_queue_limits(l, BCLINK_WIN_MIN, max_win); + tipc_link_set_queue_limits(l, tipc_link_min_win(l), max_win); tipc_bcast_unlock(net); return 0; } -- 2.25.1 |
From: Jon M. <jm...@re...> - 2020-10-13 10:42:32
|
On 10/5/20 6:34 AM, Tung Nguyen wrote: > In case the backlog transmit queue for system-importance messages is > overloaded, tipc_link_xmit() returns -ENOBUFS but the skb list is not > purged. This leads to memory leak and failure when a skb is allocated. > > This commit fixes this issue by purging the skb list before > tipc_link_xmit() returns. > > Reported-by: Thang Hoang Ngo <tha...@de...> > Signed-off-by: Tung Nguyen <tun...@de...> > --- > net/tipc/link.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index cef38a910107..ca0bb09482d0 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -1028,6 +1028,7 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list, > if (unlikely(l->backlog[imp].len >= l->backlog[imp].limit)) { > if (imp == TIPC_SYSTEM_IMPORTANCE) { > pr_warn("%s<%s>, link overflow", link_rst_msg, l->name); > + __skb_queue_purge(list); > return -ENOBUFS; > } > rc = link_schedule_user(l, hdr); Acked-by: Jon Maloy <jm...@re...> |
From: Hoang H. Le <hoa...@de...> - 2020-10-13 06:18:41
|
The queue limit of the broadcast link is being calculated base on initial MTU. However, when MTU value changed (e.g manual changing MTU on NIC device, MTU negotiation etc.,) we do not re-calculate queue limit. This gives throughput does not reflect with the change. So fix it by calling the function to re-calculate queue limit of the broadcast link. Acked-by: Jon Maloy <jm...@re...> Signed-off-by: Hoang Huu Le <hoa...@de...> --- net/tipc/bcast.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 940d176e0e87..c77fd13e2777 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -108,6 +108,7 @@ static void tipc_bcbase_select_primary(struct net *net) { struct tipc_bc_base *bb = tipc_bc_base(net); int all_dests = tipc_link_bc_peers(bb->link); + int max_win = tipc_link_max_win(bb->link); int i, mtu, prim; bb->primary_bearer = INVALID_BEARER_ID; @@ -121,8 +122,11 @@ static void tipc_bcbase_select_primary(struct net *net) continue; mtu = tipc_bearer_mtu(net, i); - if (mtu < tipc_link_mtu(bb->link)) + if (mtu < tipc_link_mtu(bb->link)) { tipc_link_set_mtu(bb->link, mtu); + tipc_link_set_queue_limits(bb->link, max_win, + max_win); + } bb->bcast_support &= tipc_bearer_bcast_support(net, i); if (bb->dests[i] < all_dests) continue; -- 2.25.1 |
From: Hoang H. Le <hoa...@de...> - 2020-10-13 02:01:00
|
> -----Original Message----- > From: Jon Maloy <jm...@re...> > Sent: Tuesday, October 13, 2020 6:39 AM > To: Hoang Huu Le <hoa...@de...>; tip...@li...; ma...@do...; > yin...@wi... > Subject: Re: [net] tipc: fix incorrect setting window for bcast link > > Hi Hoang, > I apologize for not paying enough attention to this problem until now. > See below. > > > On 9/30/20 9:43 PM, Hoang Huu Le wrote: > > In commit 16ad3f4022bb > > ("tipc: introduce variable window congestion control"), we applied > > the Reno algorithm to select window size from minimum window to the > > configured maximum window for unicast link. > > > > However, when setting window variable we do not specific to unicast link. > > This lead to the window for broadcast link had effect as unexpect value > > (i.e the window size is constantly 32). > > > > We fix this by updating the window for broadcast as its configuration. > > > > Signed-off-by: Hoang Huu Le <hoa...@de...> > > --- > > net/tipc/bcast.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > > index 940d176e0e87..abac9443b4d9 100644 > > --- a/net/tipc/bcast.c > > +++ b/net/tipc/bcast.c > > @@ -585,7 +585,7 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 max_win) > > if (max_win > TIPC_MAX_LINK_WIN) > > return -EINVAL; > > tipc_bcast_lock(net); > > - tipc_link_set_queue_limits(l, BCLINK_WIN_MIN, max_win); > > + tipc_link_set_queue_limits(l, max_win, max_win); > I think this is dangerous. The broadcast link puts a much higher stress > on the switch, and the risk of massive packet loss with ditto > retransmissions is very high. > A safer bet to stay with a fix window of 50 for now, to solve the > problem you have at sites, and then you can possibly experiment with a > variable window later. So, should we return to 'not support' when user changes the window for broadcast link. Or just silent ignore the setting? > If it gives significant higher throughput it might be worthwhile trying, > but I am pretty sure that e.g. the base for calculating ssthresh (300) > is too big. > > ///jon > > tipc_bcast_unlock(net); > > return 0; > > } |
From: Jon M. <jm...@re...> - 2020-10-12 23:42:41
|
On 10/8/20 11:36 PM, Hoang Huu Le wrote: > Hi Jon, > > See my inline comment. > > Thanks, > Hoang >> -----Original Message----- >> From: Jon Maloy <jm...@re...> >> Sent: Friday, October 9, 2020 1:27 AM >> To: Hoang Huu Le <hoa...@de...>; ma...@do...; yin...@wi...; tipc- >> dis...@li... >> Subject: Re: [net-next] tipc: introduce smooth change to replicast >> >> >> >> On 10/7/20 10:22 PM, Hoang Huu Le wrote: >>> In commit cad2929dc432 ("tipc: update a binding service via broadcast"), >>> We use broadcast to update a binding service for a large cluster. >>> However, if we try to publish a thousands of services at the >>> same time, we may to get "link overflow" happen because of queue limit >>> had reached. >>> >>> We now introduce a smooth change to replicast if the broadcast link has >>> reach to the limit of queue. >> To me this beats the whole purpose of using broadcast distribution in >> the first place. >> We wanted to save CPU and network resources by using broadcast, and >> then, when things get tough, we fall back to the supposedly less >> efficient replicast method. Not good. >> >> I wonder what is really happening when this overflow situation occurs. >> First, the reset limit is dimensioned so that it should be possible to >> publish MAX_PUBLICATIONS (65k) publications in one shot. >> With full bundling, which is what I expect here, there are 1460/20 = 73 >> publication items in each buffer, so the reset limit (==max_bulk) should >> be 65k/73 = 897 buffers. > [Hoang] No, it's not right! > As I staged in another commit that the reset limit is 350 buffers (65k/(3744/20)) => #1. > where: > FB_MTU=3744 > and if a user set window size is 50, we are fallback to 32 window-size => #2. > So, if the broadcast link is under high load traffic, we will reach to the limit easily (#1 + #2). Yes. I didn't review your previous patches before making this comment. > >> My figures are just from the top of my head, so you should double check >> them, but I find it unlikely that we hit this limit unless there is a >> lot of other broadcast going on at the same time, and even then I find >> it unlikely. > [Hoang] > I just implement a simple application: > [...] > num_service = 10k > for (i=0;i<num_service; i++) > bind(socket, service<i>); > [...] > > Then, run this app; sleep 2; kill SIGINT app; Then, the problem is reproducible. > >> I suggest you try to find out what is really going on when we reach this >> situation. >> -What exactly is in the backlog queue? >> -Only publications? >> -How many? >> -A mixture of publications and other traffic? > Only publication/withdrawn buffers, around more thousands. > >> -Has bundling really worked as supposed? >> -Do we still have some issue with the broadcast link that stops buffers >> being acked and released in a timely manner? I suspect you mean NO in this case ;-) >> - Have you been able to dump out such info when this problem occurs? >> - Are you able to re-produce it in your own system? > These answers are YES > >> In the end it might be as simple as increasing the reset limit, but we >> really should try to understand what is happening first. > Yes, I think so. As previous patch I made the code change to update queue backlog supporting to 873 buffers. > But if I increase number of services in my app up to 20k (not real??>) . The issue is able to reproduce as well. Why does it still happen? The new limit is calculated to be safe for 65k publications, so why does it fail already at 20k? According to the loop you show above you only do publications, no withdrawals, so yo should not even theoretically be able to reach the reset limit. What is happening? Let's discuss this tomorrow. ///jon > That is the reason why I propose this new solution + combine with two previous patches to solve the problem completely. >> Regards >> ///jon >> >> >>> Signed-off-by: Hoang Huu Le <hoa...@de...> >>> --- >>> net/tipc/link.c | 5 ++++- >>> net/tipc/node.c | 12 ++++++++++-- >>> 2 files changed, 14 insertions(+), 3 deletions(-) >>> >>> diff --git a/net/tipc/link.c b/net/tipc/link.c >>> index 06b880da2a8e..ca908ead753a 100644 >>> --- a/net/tipc/link.c >>> +++ b/net/tipc/link.c >>> @@ -1022,7 +1022,10 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list, >>> /* Allow oversubscription of one data msg per source at congestion */ >>> if (unlikely(l->backlog[imp].len >= l->backlog[imp].limit)) { >>> if (imp == TIPC_SYSTEM_IMPORTANCE) { >>> - pr_warn("%s<%s>, link overflow", link_rst_msg, l->name); >>> + pr_warn_ratelimited("%s<%s>, link overflow", >>> + link_rst_msg, l->name); >>> + if (link_is_bc_sndlink(l)) >>> + return -EOVERFLOW; >>> return -ENOBUFS; >>> } >>> rc = link_schedule_user(l, hdr); >>> diff --git a/net/tipc/node.c b/net/tipc/node.c >>> index d269ebe382e1..a37976610367 100644 >>> --- a/net/tipc/node.c >>> +++ b/net/tipc/node.c >>> @@ -1750,15 +1750,23 @@ void tipc_node_broadcast(struct net *net, struct sk_buff *skb, int rc_dests) >>> struct tipc_node *n; >>> u16 dummy; >>> u32 dst; >>> + int rc = 0; >>> >>> /* Use broadcast if all nodes support it */ >>> if (!rc_dests && tipc_bcast_get_mode(net) != BCLINK_MODE_RCAST) { >>> + txskb = pskb_copy(skb, GFP_ATOMIC); >>> + if (!txskb) >>> + goto rcast; >>> __skb_queue_head_init(&xmitq); >>> - __skb_queue_tail(&xmitq, skb); >>> - tipc_bcast_xmit(net, &xmitq, &dummy); >>> + __skb_queue_tail(&xmitq, txskb); >>> + rc = tipc_bcast_xmit(net, &xmitq, &dummy); >>> + if (rc == -EOVERFLOW) >>> + goto rcast; >>> + kfree_skb(skb); >>> return; >>> } >>> >>> +rcast: >>> /* Otherwise use legacy replicast method */ >>> rcu_read_lock(); >>> list_for_each_entry_rcu(n, tipc_nodes(net), list) { |
From: Jon M. <jm...@re...> - 2020-10-12 23:40:02
|
On 10/1/20 2:51 AM, Hoang Huu Le wrote: > The queue limit of the broadcast link is being calculated base on initial > MTU. However, when MTU value changed (e.g manual changing MTU on NIC device, > MTU negotiation etc.,) we do not re-calculate queue limit. This gives > throughput does not reflect with the change. > > So fix it by calling the function to re-calculate queue limit of the > broadcast link. > > Signed-off-by: Hoang Huu Le <hoa...@de...> > --- > net/tipc/bcast.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > index abac9443b4d9..bc566b304571 100644 > --- a/net/tipc/bcast.c > +++ b/net/tipc/bcast.c > @@ -108,6 +108,7 @@ static void tipc_bcbase_select_primary(struct net *net) > { > struct tipc_bc_base *bb = tipc_bc_base(net); > int all_dests = tipc_link_bc_peers(bb->link); > + int max_win = tipc_link_max_win(bb->link); > int i, mtu, prim; > > bb->primary_bearer = INVALID_BEARER_ID; > @@ -121,8 +122,11 @@ static void tipc_bcbase_select_primary(struct net *net) > continue; > > mtu = tipc_bearer_mtu(net, i); > - if (mtu < tipc_link_mtu(bb->link)) > + if (mtu < tipc_link_mtu(bb->link)) { > tipc_link_set_mtu(bb->link, mtu); > + tipc_link_set_queue_limits(bb->link, max_win, > + max_win); > + } > bb->bcast_support &= tipc_bearer_bcast_support(net, i); > if (bb->dests[i] < all_dests) > continue; An obvious bug that explains a lot, and needed to be fixed. Thanks. Acked-by: Jon Maloy <jm...@re...> |
From: Jon M. <jm...@re...> - 2020-10-12 23:39:34
|
Hi Hoang, I apologize for not paying enough attention to this problem until now. See below. On 9/30/20 9:43 PM, Hoang Huu Le wrote: > In commit 16ad3f4022bb > ("tipc: introduce variable window congestion control"), we applied > the Reno algorithm to select window size from minimum window to the > configured maximum window for unicast link. > > However, when setting window variable we do not specific to unicast link. > This lead to the window for broadcast link had effect as unexpect value > (i.e the window size is constantly 32). > > We fix this by updating the window for broadcast as its configuration. > > Signed-off-by: Hoang Huu Le <hoa...@de...> > --- > net/tipc/bcast.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c > index 940d176e0e87..abac9443b4d9 100644 > --- a/net/tipc/bcast.c > +++ b/net/tipc/bcast.c > @@ -585,7 +585,7 @@ static int tipc_bc_link_set_queue_limits(struct net *net, u32 max_win) > if (max_win > TIPC_MAX_LINK_WIN) > return -EINVAL; > tipc_bcast_lock(net); > - tipc_link_set_queue_limits(l, BCLINK_WIN_MIN, max_win); > + tipc_link_set_queue_limits(l, max_win, max_win); I think this is dangerous. The broadcast link puts a much higher stress on the switch, and the risk of massive packet loss with ditto retransmissions is very high. A safer bet to stay with a fix window of 50 for now, to solve the problem you have at sites, and then you can possibly experiment with a variable window later. If it gives significant higher throughput it might be worthwhile trying, but I am pretty sure that e.g. the base for calculating ssthresh (300) is too big. ///jon > tipc_bcast_unlock(net); > return 0; > } |
From: Jon M. <jm...@re...> - 2020-10-12 22:13:27
|
On 10/11/20 5:13 AM, Tuong Lien wrote: > This series adds two new options in the 'iproute2/tipc' command, enabling users > to use the new TIPC encryption features, i.e. the master key and rekeying which > have been recently merged in kernel. > > The help menu of the "tipc node set key" command is also updated accordingly: > > # tipc node set key --help > Usage: tipc node set key KEY [algname ALGNAME] [PROPERTIES] > tipc node set key rekeying REKEYING > > KEY > Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form: > [KEY: 16, 24 or 32 octets][SALT: 4 octets] > > ALGNAME > Cipher algorithm [default: "gcm(aes)"] > > PROPERTIES > master - Set KEY as a cluster master key > <empty> - Set KEY as a cluster key > nodeid NODEID - Set KEY as a per-node key for own or peer > > REKEYING > INTERVAL - Set rekeying interval (in minutes) [0: disable] > now - Trigger one (first) rekeying immediately > > EXAMPLES > tipc node set key this_is_a_master_key master > tipc node set key 0x746869735F69735F615F6B657931365F73616C74 > tipc node set key this_is_a_key16_salt algname "gcm(aes)" nodeid 1001002 > tipc node set key rekeying 600 > > Tuong Lien (2): > tipc: add option to set master key for encryption > tipc: add option to set rekeying for encryption > > tipc/cmdl.c | 2 +- > tipc/cmdl.h | 1 + > tipc/node.c | 81 +++++++++++++++++++++++++++++++++++++++-------------- > 3 files changed, 62 insertions(+), 22 deletions(-) > Looks good to me. Series Acked-by: Jon Maloy <jm...@re...> |
From: Rune T. <ru...@in...> - 2020-10-12 14:20:19
|
I think I see now. This would block anybody from calling bind() on the topology server (except first user in kernel) Using connect() should be fine. Sorry, did not read clearly enough. ________________________________ From: Rune Torgersen <ru...@in...> Sent: Monday, October 12, 2020 8:51 AM To: jm...@re... <jm...@re...>; tip...@li... <tip...@li...> Cc: xi...@re... <xi...@re...> Subject: Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types Hi. We use the tipc topology server extensively in our system to keep track of other instances of an app. Dies this chage mea we cannot use the topology server anymore? This si the userland code we use (this one would check on open sockets on 200:0 to 200:255 void * TipcTopologyServerThread(void * context) { int tipcStatusSocket; struct tipc_event event; struct sockaddr_tipc topsrv; struct tipc_subscr subscr = {{200, 0, 255}, TIPC_WAIT_FOREVER, TIPC_SUB_SERVICE, {5,5,5,5,5,5,5,5}}; memset(&topsrv, 0, sizeof(topsrv)); topsrv.family = AF_TIPC; topsrv.addrtype = TIPC_ADDR_NAME; topsrv.addr.name.name.type = TIPC_TOP_SRV; topsrv.addr.name.name.instance = TIPC_TOP_SRV; tipcStatusSocket = socket(AF_TIPC, SOCK_SEQPACKET,0); if (tipcStatusSocket < 0) { perror("TipcTopologyThread: Could not make TIPC socket"); } // Connect to topology server: if (0 > connect(tipcStatusSocket,(struct sockaddr*)&topsrv,sizeof(topsrv))) { perro("TipcTopologyThread: Could not connect to TIPC topology server"); return NULL; } if (send(tipcStatusSocket,&subscr,sizeof(subscr),0) != sizeof(subscr)) { printf("TipcTopologyThread: Failed to send topology server subscription"); return NULL; } // Now wait for the subscriptions to fire: while(true) { int ret = recv(tipcStatusSocket,&event,sizeof(event),0); if (ret == sizeof(event)) { printf("Received an event\n"); if (event.event == TIPC_PUBLISHED) { printf("received a TIPC_PUBLISH msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); // do something } else if (event.event == TIPC_WITHDRAWN) { printf("received a TIPC_WITHDRAWN msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); // do something } else if (event.event == TIPC_SUBSCR_TIMEOUT) { printf("received a TIPC_SUBSCR_TIMEOUT msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); } } } return NULL; } ________________________________ From: jm...@re... <jm...@re...> Sent: Saturday, October 10, 2020 9:56 AM To: tip...@li... <tip...@li...> Cc: xi...@re... <xi...@re...> Subject: [tipc-discussion] [net ] tipc: add stricter control of reserved service types This email originated from outside Innovative Systems. Do not click links or open attachments unless you recognize the sender and know the content is safe. From: Jon Maloy <jm...@re...> TIPC reserves 64 service types for current and future internal use. Therefore, the bind() function is meant to block regular user sockets from being bound to these values, while it should let through such bindings from internal users. However, since we at the design moment saw no way to distinguish between regular and internal users the filter function ended up with allowing all bindings of the types which were really in use ([0,1]), and block all the rest ([2,63]). This is dangerous, since a regular user may bind to the service type representing the topology server (TIPC_TOP_SRV == 1) or the one used for indicating neigboring node status (TIPC_CFG_SRV == 0), and wreak havoc for users of those services. I.e., practically all users. The reality is however that TIPC_CFG_SRV never is bound through the bind() function, since it doesn't represent a regular socket, and TIPC_TOP_SRV can easily be filtered out, since it is the very first binding performed when the system is starting. We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV to be bound once, and the correct behavior is achieved. This is what we do in this commit. It should be noted that, although this is a change of the API semantics, there is no risk we will break any currently working applications by doing this. Any application trying to bind to the values in question would be badly broken from the outset, so there is no chance we would find any such applications in real-world production systems. Signed-off-by: Jon Maloy <jm...@re...> --- net/tipc/socket.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/net/tipc/socket.c b/net/tipc/socket.c index e795a8a2955b..67875a5761d0 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; struct tipc_sock *tsk = tipc_sk(sk); int res = -EINVAL; + u32 stype, dnode; lock_sock(sk); if (unlikely(!uaddr_len)) { @@ -691,9 +692,10 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, goto exit; } - if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) && - (addr->addr.nameseq.type != TIPC_TOP_SRV) && - (addr->addr.nameseq.type != TIPC_CFG_SRV)) { + stype = addr->addr.nameseq.type; + if (stype < TIPC_RESERVED_TYPES && + (stype != TIPC_TOP_SRV || + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) { res = -EACCES; goto exit; } -- 2.25.4 _______________________________________________ tipc-discussion mailing list tip...@li... https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Ftipc-discussion&data=04%7C01%7Crunet%40innovsys.com%7Cf5c6eae3597f44e1c37e08d86d2cbb7c%7C7a48ce45ee974a95ac183390878a179b%7C0%7C0%7C637379386198215419%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BCCVlEaV2ObPW5zknkRAxXhGkWFcG%2Fgwaz6LuRHR3kc%3D&reserved=0 |
From: Rune T. <ru...@in...> - 2020-10-12 14:18:08
|
Hi. We use the tipc topology server extensively in our system to keep track of other instances of an app. Dies this chage mea we cannot use the topology server anymore? This si the userland code we use (this one would check on open sockets on 200:0 to 200:255 void * TipcTopologyServerThread(void * context) { int tipcStatusSocket; struct tipc_event event; struct sockaddr_tipc topsrv; struct tipc_subscr subscr = {{200, 0, 255}, TIPC_WAIT_FOREVER, TIPC_SUB_SERVICE, {5,5,5,5,5,5,5,5}}; memset(&topsrv, 0, sizeof(topsrv)); topsrv.family = AF_TIPC; topsrv.addrtype = TIPC_ADDR_NAME; topsrv.addr.name.name.type = TIPC_TOP_SRV; topsrv.addr.name.name.instance = TIPC_TOP_SRV; tipcStatusSocket = socket(AF_TIPC, SOCK_SEQPACKET,0); if (tipcStatusSocket < 0) { perror("TipcTopologyThread: Could not make TIPC socket"); } // Connect to topology server: if (0 > connect(tipcStatusSocket,(struct sockaddr*)&topsrv,sizeof(topsrv))) { perro("TipcTopologyThread: Could not connect to TIPC topology server"); return NULL; } if (send(tipcStatusSocket,&subscr,sizeof(subscr),0) != sizeof(subscr)) { printf("TipcTopologyThread: Failed to send topology server subscription"); return NULL; } // Now wait for the subscriptions to fire: while(true) { int ret = recv(tipcStatusSocket,&event,sizeof(event),0); if (ret == sizeof(event)) { printf("Received an event\n"); if (event.event == TIPC_PUBLISHED) { printf("received a TIPC_PUBLISH msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); // do something } else if (event.event == TIPC_WITHDRAWN) { printf("received a TIPC_WITHDRAWN msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); // do something } else if (event.event == TIPC_SUBSCR_TIMEOUT) { printf("received a TIPC_SUBSCR_TIMEOUT msg, type: %u, found_lower: %u, found_upper: %u\n", event.s.seq.type, event.found_lower, event.found_upper); } } } return NULL; } ________________________________ From: jm...@re... <jm...@re...> Sent: Saturday, October 10, 2020 9:56 AM To: tip...@li... <tip...@li...> Cc: xi...@re... <xi...@re...> Subject: [tipc-discussion] [net ] tipc: add stricter control of reserved service types This email originated from outside Innovative Systems. Do not click links or open attachments unless you recognize the sender and know the content is safe. From: Jon Maloy <jm...@re...> TIPC reserves 64 service types for current and future internal use. Therefore, the bind() function is meant to block regular user sockets from being bound to these values, while it should let through such bindings from internal users. However, since we at the design moment saw no way to distinguish between regular and internal users the filter function ended up with allowing all bindings of the types which were really in use ([0,1]), and block all the rest ([2,63]). This is dangerous, since a regular user may bind to the service type representing the topology server (TIPC_TOP_SRV == 1) or the one used for indicating neigboring node status (TIPC_CFG_SRV == 0), and wreak havoc for users of those services. I.e., practically all users. The reality is however that TIPC_CFG_SRV never is bound through the bind() function, since it doesn't represent a regular socket, and TIPC_TOP_SRV can easily be filtered out, since it is the very first binding performed when the system is starting. We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV to be bound once, and the correct behavior is achieved. This is what we do in this commit. It should be noted that, although this is a change of the API semantics, there is no risk we will break any currently working applications by doing this. Any application trying to bind to the values in question would be badly broken from the outset, so there is no chance we would find any such applications in real-world production systems. Signed-off-by: Jon Maloy <jm...@re...> --- net/tipc/socket.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/net/tipc/socket.c b/net/tipc/socket.c index e795a8a2955b..67875a5761d0 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; struct tipc_sock *tsk = tipc_sk(sk); int res = -EINVAL; + u32 stype, dnode; lock_sock(sk); if (unlikely(!uaddr_len)) { @@ -691,9 +692,10 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr, goto exit; } - if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) && - (addr->addr.nameseq.type != TIPC_TOP_SRV) && - (addr->addr.nameseq.type != TIPC_CFG_SRV)) { + stype = addr->addr.nameseq.type; + if (stype < TIPC_RESERVED_TYPES && + (stype != TIPC_TOP_SRV || + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) { res = -EACCES; goto exit; } -- 2.25.4 _______________________________________________ tipc-discussion mailing list tip...@li... https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Ftipc-discussion&data=04%7C01%7Crunet%40innovsys.com%7Cf5c6eae3597f44e1c37e08d86d2cbb7c%7C7a48ce45ee974a95ac183390878a179b%7C0%7C0%7C637379386198215419%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BCCVlEaV2ObPW5zknkRAxXhGkWFcG%2Fgwaz6LuRHR3kc%3D&reserved=0 |
From: Tuong L. <tuo...@de...> - 2020-10-11 09:14:57
|
As supported in kernel, the TIPC encryption rekeying can be tuned using the netlink attribute - 'TIPC_NLA_NODE_REKEYING'. Now we add the 'rekeying' option correspondingly to the 'tipc node set key' command so that user will be able to perform that tuning: tipc node set key rekeying REKEYING where the 'REKEYING' value can be: INTERVAL - Set rekeying interval (in minutes) [0: disable] now - Trigger one (first) rekeying immediately For example: $ tipc node set key rekeying 60 $ tipc node set key rekeying now The command's help menu is also updated with these descriptions for the new command option. Signed-off-by: Tuong Lien <tuo...@de...> --- tipc/cmdl.c | 2 +- tipc/cmdl.h | 1 + tipc/node.c | 47 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/tipc/cmdl.c b/tipc/cmdl.c index f2f259cc..981e268e 100644 --- a/tipc/cmdl.c +++ b/tipc/cmdl.c @@ -33,7 +33,7 @@ static const struct cmd *find_cmd(const struct cmd *cmds, char *str) return match; } -static struct opt *find_opt(struct opt *opts, char *str) +struct opt *find_opt(struct opt *opts, char *str) { struct opt *o; struct opt *match = NULL; diff --git a/tipc/cmdl.h b/tipc/cmdl.h index 03db3599..dcade362 100644 --- a/tipc/cmdl.h +++ b/tipc/cmdl.h @@ -46,6 +46,7 @@ struct opt { char *val; }; +struct opt *find_opt(struct opt *opts, char *str); struct opt *get_opt(struct opt *opts, char *key); bool has_opt(struct opt *opts, char *key); int parse_opts(struct opt *opts, struct cmdl *cmdl); diff --git a/tipc/node.c b/tipc/node.c index 1ff0baa4..05246013 100644 --- a/tipc/node.c +++ b/tipc/node.c @@ -160,7 +160,8 @@ static int cmd_node_set_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd, static void cmd_node_set_key_help(struct cmdl *cmdl) { fprintf(stderr, - "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n\n" + "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n" + " %s node set key rekeying REKEYING\n\n" "KEY\n" " Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form:\n" " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" @@ -170,11 +171,16 @@ static void cmd_node_set_key_help(struct cmdl *cmdl) " master - Set KEY as a cluster master key\n" " <empty> - Set KEY as a cluster key\n" " nodeid NODEID - Set KEY as a per-node key for own or peer\n\n" + "REKEYING\n" + " INTERVAL - Set rekeying interval (in minutes) [0: disable]\n" + " now - Trigger one (first) rekeying immediately\n\n" "EXAMPLES\n" " %s node set key this_is_a_master_key master\n" " %s node set key 0x746869735F69735F615F6B657931365F73616C74\n" - " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n\n", - cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); + " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n" + " %s node set key rekeying 600\n\n", + cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], + cmdl->argv[0], cmdl->argv[0]); } static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, @@ -190,12 +196,15 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, { "algname", OPT_KEYVAL, NULL }, { "nodeid", OPT_KEYVAL, NULL }, { "master", OPT_KEY, NULL }, + { "rekeying", OPT_KEYVAL, NULL }, { NULL } }; struct nlattr *nest; - struct opt *opt_algname, *opt_nodeid, *opt_master; + struct opt *opt_algname, *opt_nodeid, *opt_master, *opt_rekeying; char buf[MNL_SOCKET_BUFFER_SIZE]; uint8_t id[TIPC_NODEID_LEN] = {0,}; + uint32_t rekeying = 0; + bool has_key = false; int keysize; char *str; @@ -204,17 +213,31 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, return -EINVAL; } + /* Check if command starts with opts i.e. "rekeying" opt without key */ + if (find_opt(opts, cmdl->argv[cmdl->optind])) + goto get_ops; /* Get user key */ + has_key = true; str = shift_cmdl(cmdl); if (str2key(str, &input.key)) { fprintf(stderr, "error, invalid key input\n"); return -EINVAL; } +get_ops: if (parse_opts(opts, cmdl) < 0) return -EINVAL; + /* Get rekeying time */ + opt_rekeying = get_opt(opts, "rekeying"); + if (opt_rekeying) { + if (!strcmp(opt_rekeying->val, "now")) + rekeying = TIPC_REKEYING_NOW; + else + rekeying = atoi(opt_rekeying->val); + } + /* Get algorithm name, default: "gcm(aes)" */ opt_algname = get_opt(opts, "algname"); if (!opt_algname) @@ -246,12 +269,16 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, } nest = mnl_attr_nest_start(nlh, TIPC_NLA_NODE); - keysize = tipc_aead_key_size(&input.key); - mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); - if (opt_nodeid) - mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); - if (opt_master) - mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + if (has_key) { + keysize = tipc_aead_key_size(&input.key); + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); + if (opt_nodeid) + mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); + if (opt_master) + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + } + if (opt_rekeying) + mnl_attr_put_u32(nlh, TIPC_NLA_NODE_REKEYING, rekeying); mnl_attr_nest_end(nlh, nest); return msg_doit(nlh, NULL, NULL); -- 2.26.2 |
From: Tuong L. <tuo...@de...> - 2020-10-11 09:14:43
|
In addition to the support of master key in kernel, we add the 'master' option to the 'tipc node set key' command for user to be able to specify a key as master key during the key setting. This is carried out by turning on the new netlink flag - 'TIPC_NLA_NODE_KEY_MASTER'. For example: $ tipc node set key "this_is_a_master_key" master The command's help menu is also updated to give a better description of all the available options. Signed-off-by: Tuong Lien <tuo...@de...> --- tipc/node.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/tipc/node.c b/tipc/node.c index ffdaeaea..1ff0baa4 100644 --- a/tipc/node.c +++ b/tipc/node.c @@ -160,19 +160,21 @@ static int cmd_node_set_nodeid(struct nlmsghdr *nlh, const struct cmd *cmd, static void cmd_node_set_key_help(struct cmdl *cmdl) { fprintf(stderr, - "Usage: %s node set key KEY [algname ALGNAME] [nodeid NODEID]\n\n" + "Usage: %s node set key KEY [algname ALGNAME] [PROPERTIES]\n\n" + "KEY\n" + " Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form:\n" + " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" + "ALGNAME\n" + " Cipher algorithm [default: \"gcm(aes)\"]\n\n" "PROPERTIES\n" - " KEY - Symmetric KEY & SALT as a normal or hex string\n" - " that consists of two parts:\n" - " [KEY: 16, 24 or 32 octets][SALT: 4 octets]\n\n" - " algname ALGNAME - Default: \"gcm(aes)\"\n\n" - " nodeid NODEID - Own or peer node identity to which the key will\n" - " be attached. If not present, the key is a cluster\n" - " key!\n\n" + " master - Set KEY as a cluster master key\n" + " <empty> - Set KEY as a cluster key\n" + " nodeid NODEID - Set KEY as a per-node key for own or peer\n\n" "EXAMPLES\n" - " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid node1\n" - " %s node set key 0x746869735F69735F615F6B657931365F73616C74 nodeid node2\n\n", - cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); + " %s node set key this_is_a_master_key master\n" + " %s node set key 0x746869735F69735F615F6B657931365F73616C74\n" + " %s node set key this_is_a_key16_salt algname \"gcm(aes)\" nodeid 1001002\n\n", + cmdl->argv[0], cmdl->argv[0], cmdl->argv[0], cmdl->argv[0]); } static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, @@ -187,24 +189,21 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, struct opt opts[] = { { "algname", OPT_KEYVAL, NULL }, { "nodeid", OPT_KEYVAL, NULL }, + { "master", OPT_KEY, NULL }, { NULL } }; struct nlattr *nest; - struct opt *opt_algname, *opt_nodeid; + struct opt *opt_algname, *opt_nodeid, *opt_master; char buf[MNL_SOCKET_BUFFER_SIZE]; uint8_t id[TIPC_NODEID_LEN] = {0,}; int keysize; char *str; - if (help_flag) { + if (help_flag || cmdl->optind >= cmdl->argc) { (cmd->help)(cmdl); return -EINVAL; } - if (cmdl->optind >= cmdl->argc) { - fprintf(stderr, "error, missing key\n"); - return -EINVAL; - } /* Get user key */ str = shift_cmdl(cmdl); @@ -230,17 +229,30 @@ static int cmd_node_set_key(struct nlmsghdr *nlh, const struct cmd *cmd, return -EINVAL; } + /* Get master key indication */ + opt_master = get_opt(opts, "master"); + + /* Sanity check if wrong option */ + if (opt_nodeid && opt_master) { + fprintf(stderr, "error, per-node key cannot be master\n"); + return -EINVAL; + } + /* Init & do the command */ nlh = msg_init(buf, TIPC_NL_KEY_SET); if (!nlh) { fprintf(stderr, "error, message initialisation failed\n"); return -1; } + nest = mnl_attr_nest_start(nlh, TIPC_NLA_NODE); keysize = tipc_aead_key_size(&input.key); mnl_attr_put(nlh, TIPC_NLA_NODE_KEY, keysize, &input.key); if (opt_nodeid) mnl_attr_put(nlh, TIPC_NLA_NODE_ID, TIPC_NODEID_LEN, id); + if (opt_master) + mnl_attr_put(nlh, TIPC_NLA_NODE_KEY_MASTER, 0, NULL); + mnl_attr_nest_end(nlh, nest); return msg_doit(nlh, NULL, NULL); } -- 2.26.2 |
From: Tuong L. <tuo...@de...> - 2020-10-11 09:14:40
|
This series adds two new options in the 'iproute2/tipc' command, enabling users to use the new TIPC encryption features, i.e. the master key and rekeying which have been recently merged in kernel. The help menu of the "tipc node set key" command is also updated accordingly: # tipc node set key --help Usage: tipc node set key KEY [algname ALGNAME] [PROPERTIES] tipc node set key rekeying REKEYING KEY Symmetric KEY & SALT as a composite ASCII or hex string (0x...) in form: [KEY: 16, 24 or 32 octets][SALT: 4 octets] ALGNAME Cipher algorithm [default: "gcm(aes)"] PROPERTIES master - Set KEY as a cluster master key <empty> - Set KEY as a cluster key nodeid NODEID - Set KEY as a per-node key for own or peer REKEYING INTERVAL - Set rekeying interval (in minutes) [0: disable] now - Trigger one (first) rekeying immediately EXAMPLES tipc node set key this_is_a_master_key master tipc node set key 0x746869735F69735F615F6B657931365F73616C74 tipc node set key this_is_a_key16_salt algname "gcm(aes)" nodeid 1001002 tipc node set key rekeying 600 Tuong Lien (2): tipc: add option to set master key for encryption tipc: add option to set rekeying for encryption tipc/cmdl.c | 2 +- tipc/cmdl.h | 1 + tipc/node.c | 81 +++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 62 insertions(+), 22 deletions(-) -- 2.26.2 |