| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 19 Jun 2015 10:02:39 -0700
From: Mahesh Bandewar <maheshb@...gle.com>
To: Andy Gospodarek <gospo@...ulusnetworks.com>
Cc: Jay Vosburgh <j.vosburgh@...il.com>,
Andy Gospodarek <andy@...yhouse.net>,
Veaceslav Falico <vfalico@...il.com>,
Nikolay Aleksandrov <nikolay@...hat.com>,
David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH next v3] bonding: Display LACP info only to CAP_NET_ADMIN
capable user
On Thu, Jun 18, 2015 at 8:00 PM, Andy Gospodarek
<gospo@...ulusnetworks.com> wrote:
>
> On Thu, Jun 18, 2015 at 11:30:54AM -0700, Mahesh Bandewar wrote:
> > Actor and Partner details can be accessed via proc-fs, sys-fs
> > entries or netlink interface. These interfaces are world readable
> > at this moment. The earlier patch-series made the LACP communication
> > secure to avoid nuisance attack from within the same L2 domain but
> > it did not prevent "someone unprivileged" looking at that information
> > on host and perform the same act.
> >
> > This patch essentially avoids spitting those entries if the user
> > in question does not have enough privileges.
> >
> > Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
> > ---
> > drivers/net/bonding/bond_netlink.c | 23 +++++----
> > drivers/net/bonding/bond_procfs.c | 101 +++++++++++++++++++------------------
> > drivers/net/bonding/bond_sysfs.c | 12 ++---
> > 3 files changed, 71 insertions(+), 65 deletions(-)
> >
> [...]
> > diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
> > index e7f3047a26df..f514fe5e80a5 100644
> > --- a/drivers/net/bonding/bond_procfs.c
> > +++ b/drivers/net/bonding/bond_procfs.c
> [...]
> > @@ -199,33 +202,35 @@ static void bond_info_show_slave(struct seq_file *seq,
> > seq_printf(seq, "Partner Churned Count: %d\n",
> > port->churn_partner_count);
> >
> > - seq_puts(seq, "details actor lacp pdu:\n");
> > - seq_printf(seq, " system priority: %d\n",
> > - port->actor_system_priority);
> > - seq_printf(seq, " system mac address: %pM\n",
> > - &port->actor_system);
> > - seq_printf(seq, " port key: %d\n",
> > - port->actor_oper_port_key);
> > - seq_printf(seq, " port priority: %d\n",
> > - port->actor_port_priority);
> > - seq_printf(seq, " port number: %d\n",
> > - port->actor_port_number);
> > - seq_printf(seq, " port state: %d\n",
> > - port->actor_oper_port_state);
> > -
> > - seq_puts(seq, "details partner lacp pdu:\n");
> > - seq_printf(seq, " system priority: %d\n",
> > - port->partner_oper.system_priority);
> > - seq_printf(seq, " system mac address: %pM\n",
> > - &port->partner_oper.system);
> > - seq_printf(seq, " oper key: %d\n",
> > - port->partner_oper.key);
> > - seq_printf(seq, " port priority: %d\n",
> > - port->partner_oper.port_priority);
> > - seq_printf(seq, " port number: %d\n",
> > - port->partner_oper.port_number);
> > - seq_printf(seq, " port state: %d\n",
> > - port->partner_oper.port_state);
> > + if (capable(CAP_NET_ADMIN)) {
> > + seq_puts(seq, "details actor lacp pdu:\n");
> > + seq_printf(seq, " system priority: %d\n",
> > + port->actor_system_priority);
> > + seq_printf(seq, " system mac address: %pM\n",
> > + &port->actor_system);
> > + seq_printf(seq, " port key: %d\n",
> > + port->actor_oper_port_key);
> > + seq_printf(seq, " port priority: %d\n",
> > + port->actor_port_priority);
> > + seq_printf(seq, " port number: %d\n",
> > + port->actor_port_number);
> > + seq_printf(seq, " port state: %d\n",
> > + port->actor_oper_port_state);
> > +
> > + seq_puts(seq, "details partner lacp pdu:\n");
> > + seq_printf(seq, " system priority: %d\n",
> > + port->partner_oper.system_priority);
> > + seq_printf(seq, " system mac address: %pM\n",
> > + &port->partner_oper.system);
> > + seq_printf(seq, " oper key: %d\n",
> > + port->partner_oper.key);
> > + seq_printf(seq, " port priority: %d\n",
> > + port->partner_oper.port_priority);
> > + seq_printf(seq, " port number: %d\n",
> > + port->partner_oper.port_number);
> > + seq_printf(seq, " port state: %d\n",
> > + port->partner_oper.port_state);
> > + }
> > } else {
> > seq_puts(seq, "Aggregator ID: N/A\n");
> > }
>
> With this patch, actor_oper_port_state and partner_oper.port_state are
> not displayed in /proc, but that information is available via netlink
> from bond_fill_slave_info().
>
> I suspect you do not deem these two values as critical to the security
> of the system, but wanted to confirm before ACKing.
>
Yes, one can very easily figure out that LACP is used in the system
with parameters like bond-mode, lacp-rate, or the port-state. I feel
these do not need to be hidden from unprivileged users to ensure
security. Principally hiding enough to ensure security would be good
rather than hiding everything. However if there is a scenario where
exposing these values is a threat (in the same sense) then it's not
lot of extra work to achieve that and I'm open to make those change.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
Powered by blists - more mailing lists