Mailing List Archive: [PATCH v2] zebra: raise the privileges before calling socket()

[PATCH v2] zebra: raise the privileges before calling socket()

May 16, 2014, 8:13 AM

Post #1 of 4 (1129 views)

Because of recent changes when creating AF_NETLINK socket, kernel will
cache capabilities of the caller and if file descriptor is used or
otherwise handed to another process it will check that current user has
necessary capabilities to use the socket. Hence we need to ensure we
have necessary capabilities when creating the socket and at the time we
use the socket.

See: http://www.spinics.net/lists/netdev/msg280198.html

Signed-off-by: Michal Sekletar <msekleta@redhat.com>
---

V2: added signed-off-by

zebra/rt_netlink.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index ba0b0d7..9855c9e 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -162,6 +162,13 @@ netlink_socket (struct nlsock *nl, unsigned long groups)
int namelen;
int save_errno;

+ /* Bind the socket to the netlink structure for anything. */
+ if (zserv_privs.change (ZPRIVS_RAISE))
+ {
+ zlog (NULL, LOG_ERR, "Can't raise privileges");
+ return -1;
+ }
+
sock = socket (AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (sock < 0)
{
@@ -174,13 +181,6 @@ netlink_socket (struct nlsock *nl, unsigned long groups)
snl.nl_family = AF_NETLINK;
snl.nl_groups = groups;

- /* Bind the socket to the netlink structure for anything. */
- if (zserv_privs.change (ZPRIVS_RAISE))
- {
- zlog (NULL, LOG_ERR, "Can't raise privileges");
- return -1;
- }
-
ret = bind (sock, (struct sockaddr *) &snl, sizeof snl);
save_errno = errno;
if (zserv_privs.change (ZPRIVS_LOWER))
--
1.8.3.1

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

Re: [PATCH v2] zebra: raise the privileges before calling socket() [ In reply to ]

msekleta at redhat

May 26, 2014, 7:49 AM

Post #2 of 4 (1103 views)

Permalink

On Fri, May 16, 2014 at 05:13:43PM +0200, Michal Sekletar wrote:
> Because of recent changes when creating AF_NETLINK socket, kernel will
> cache capabilities of the caller and if file descriptor is used or
> otherwise handed to another process it will check that current user has
> necessary capabilities to use the socket. Hence we need to ensure we
> have necessary capabilities when creating the socket and at the time we
> use the socket.
>
> See: http://www.spinics.net/lists/netdev/msg280198.html
>
> Signed-off-by: Michal Sekletar <msekleta@redhat.com>
> ---

ping?

If there is anything wrong with this submission please let me know.

Cheers,

Michal

>
> V2: added signed-off-by
>
> zebra/rt_netlink.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
> index ba0b0d7..9855c9e 100644
> --- a/zebra/rt_netlink.c
> +++ b/zebra/rt_netlink.c
> @@ -162,6 +162,13 @@ netlink_socket (struct nlsock *nl, unsigned long groups)
> int namelen;
> int save_errno;
>
> + /* Bind the socket to the netlink structure for anything. */
> + if (zserv_privs.change (ZPRIVS_RAISE))
> + {
> + zlog (NULL, LOG_ERR, "Can't raise privileges");
> + return -1;
> + }
> +
> sock = socket (AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> if (sock < 0)
> {
> @@ -174,13 +181,6 @@ netlink_socket (struct nlsock *nl, unsigned long groups)
> snl.nl_family = AF_NETLINK;
> snl.nl_groups = groups;
>
> - /* Bind the socket to the netlink structure for anything. */
> - if (zserv_privs.change (ZPRIVS_RAISE))
> - {
> - zlog (NULL, LOG_ERR, "Can't raise privileges");
> - return -1;
> - }
> -
> ret = bind (sock, (struct sockaddr *) &snl, sizeof snl);
> save_errno = errno;
> if (zserv_privs.change (ZPRIVS_LOWER))
> --
> 1.8.3.1
>

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

Re: [PATCH v2] zebra: raise the privileges before calling socket() [ In reply to ]

equinox at diac24

May 27, 2014, 10:51 AM

Post #3 of 4 (1089 views)

Permalink

On Mon, May 26, 2014 at 04:49:40PM +0200, Michal Sekletar wrote:
> On Fri, May 16, 2014 at 05:13:43PM +0200, Michal Sekletar wrote:
> > Because of recent changes when creating AF_NETLINK socket, kernel will
> > cache capabilities of the caller and if file descriptor is used or
> > otherwise handed to another process it will check that current user has
> > necessary capabilities to use the socket. Hence we need to ensure we
> > have necessary capabilities when creating the socket and at the time we
> > use the socket.
> >
> > See: http://www.spinics.net/lists/netdev/msg280198.html
> >
> > Signed-off-by: Michal Sekletar <msekleta@redhat.com>
> > ---
>
> ping?
>
> If there is anything wrong with this submission please let me know.

Nothing wrong, just the usual speed of things in Quagga.

(Actually, the comment shouldn't have been moved in your change, but
that's in the "fix during apply" category)

Since your fix essentially fixes something that "will become a
regression" (yes, weasling through the rules here), I've accepted it
even though we're in release window shortly before 23-rc1.
(The regression is against "Quagga works on a recent Linux kernel")

Thanks,

-David

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

Re: [PATCH v2] zebra: raise the privileges before calling socket() [ In reply to ]

msekleta at redhat

May 28, 2014, 8:54 AM

Post #4 of 4 (1096 views)

Permalink

On Tue, May 27, 2014 at 07:51:25PM +0200, David Lamparter wrote:
> On Mon, May 26, 2014 at 04:49:40PM +0200, Michal Sekletar wrote:
> > On Fri, May 16, 2014 at 05:13:43PM +0200, Michal Sekletar wrote:
> > > Because of recent changes when creating AF_NETLINK socket, kernel will
> > > cache capabilities of the caller and if file descriptor is used or
> > > otherwise handed to another process it will check that current user has
> > > necessary capabilities to use the socket. Hence we need to ensure we
> > > have necessary capabilities when creating the socket and at the time we
> > > use the socket.
> > >
> > > See: http://www.spinics.net/lists/netdev/msg280198.html
> > >
> > > Signed-off-by: Michal Sekletar <msekleta@redhat.com>
> > > ---
> >
> > ping?
> >
> > If there is anything wrong with this submission please let me know.
>
> Nothing wrong, just the usual speed of things in Quagga.

Wasn't sure and got a bit impatient, apologies.

>
> (Actually, the comment shouldn't have been moved in your change, but
> that's in the "fix during apply" category)
>
> Since your fix essentially fixes something that "will become a
> regression" (yes, weasling through the rules here), I've accepted it

Fedora kernel team introduced this in F20 which is our *stable* release and
users started to notice. There is not much use for routing daemon which is
unable to put routes in kernel routing tables so I wanted to get this in as
quickly as possible.

> even though we're in release window shortly before 23-rc1.
> (The regression is against "Quagga works on a recent Linux kernel")
>
> Thanks,

Thank you very much,

>
> -David

Cheers,

Michal

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev