Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types
On 10/10/20 10:56 PM, jma...@redhat.com wrote:
> From: Jon Maloy
>
> 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
Acked-by: Ying Xue
> ---
> 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, ))) {
> res = -EACCES;
> goto exit;
> }
>
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types
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!
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types
On 10/10/20 10:56 PM, jma...@redhat.com wrote:
> From: Jon Maloy
>
> 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
> ---
> 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, ))) {
> res = -EACCES;
> goto exit;
> }
>
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types
On 10/21/20 10:55 AM, Ying Xue wrote:
On 10/10/20 10:56 PM, jma...@redhat.com wrote:
From: Jon Maloy
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
---
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, ))) {
res = -EACCES;
goto exit;
}
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [net ] tipc: add stricter control of reserved service types
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
Sent: Monday, October 12, 2020 8:51 AM
To: jma...@redhat.com ;
tipc-discussion@lists.sourceforge.net
Cc: x...@redhat.com
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(, 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*),sizeof(topsrv)))
{
perro("TipcTopologyThread: Could not connect to TIPC topology server");
return NULL;
}
if (send(tipcStatusSocket,,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,,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: jma...@redhat.com
Sent: Saturday, October 10, 2020 9:56 AM
To: tipc-discussion@lists.sourceforge.net
Cc: x...@redhat.com
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
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
---
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..6
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(, 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*),sizeof(topsrv)))
{
perro("TipcTopologyThread: Could not connect to TIPC topology server");
return NULL;
}
if (send(tipcStatusSocket,,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,,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: jma...@redhat.com
Sent: Saturday, October 10, 2020 9:56 AM
To: tipc-discussion@lists.sourceforge.net
Cc: x...@redhat.com
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
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
---
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, stru
[tipc-discussion] [net ] tipc: add stricter control of reserved service types
From: Jon Maloy
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
---
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, ))) {
res = -EACCES;
goto exit;
}
--
2.25.4
___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
