Project

General

Profile

Actions

Bug #14724

closed

failed delete of IP address made it undeletable

Added by David Pacheco 26 days ago. Updated 18 days ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Ryan Goodfellow found the following steps to reproduce:

solo@han:~$ ipadm
ADDROBJ           TYPE     STATE        ADDR
lo0/v4            static   ok           127.0.0.1/8
vioif0/v6         dhcp     ok           10.47.0.178/24
lo0/v6            static   ok           ::1/128
sim0/v6           addrconf ok           fe80::a44a:93ff:fe8e:14cf/10
solo@han:~$ ipadm delete-addr sim0/v6
ipadm: could not delete address: Permission denied
solo@han:~$ pfexec ipadm delete-addr sim0/v6
Jun  1 22:05:57 in.ndpd[808]: token does not exist for sim0
Jun  1 22:05:57 in.ndpd[808]: ndpd command on interface sim0 failed with error Invalid argument
ipadm: could not delete address: Invalid argument provided
solo@han:~$ ipadm
ADDROBJ           TYPE     STATE        ADDR
lo0/v4            static   ok           127.0.0.1/8
vioif0/v6         dhcp     ok           10.47.0.178/24
lo0/v6            static   ok           ::1/128
sim0/v6           addrconf ok           fe80::a44a:93ff:fe8e:14cf/10

I had run into this separately. I spent some time tracing this and found that it's the call from `ipadm` over to `ndpd` that's failing with the "invalid argument" error. When this happens, ndpd logs this go syslog:

Jun  1 21:21:53 sock in.ndpd[529]: [ID 649272 daemon.error] token does not exist for net1
Jun  1 21:21:53 sock in.ndpd[529]: [ID 581749 daemon.error] ndpd command on interface net1 failed with error Invalid argument
Actions #1

Updated by Ryan Goodfellow 26 days ago

Thanks for the report. I'll take a swing at this.

Actions #2

Updated by Ryan Goodfellow 25 days ago

After reading through the delete code path for IPv6 addresses, here is what I think is going on.

i_ipadm_delete_ipv6addrs starts by calling i_ipamd_send_ndpd_cmd . This call lands in ndpd_process_cmd which performs a check at the very beginning of the function ipadm_check_auth(). This checks for the NETWORK_INTERFACE_CONFIG_AUTH . So the if the caller has this authorization, they can get as far as deleting the address from ndpd , without pfexec or otherwise being root.

After calling i_ipadm_send_ndpd_cmd , the function i_ipadm_delete_addr is called which contains a number of system calls. In the case where the address being deleted has an lifnum of 0, i_ipadm_set_flags is called which eventually fires off an ioctl with the SIOCSLIFFLAGS command set. This command has the IPI_PRIV flag set which means it can only be called by root.

Alternatively, if the address being deleted does not have an lifnum of zero, then ioctl is called with the command set to SIOCLIFREMOVEIF which also has IPI_PRIV set.

So in either case, lifnum == 0 or lifnum != 0, the ioctl requests fired off by i_ipadm_delete_ipv6addrs requires the caller to be root. I believe this is what leads to the behavior seen in this ticket. The calling user has the NETWORK_INTERFACE_CONFIG_AUTH authorization, but is not root, so we wind up with partially deleted state where the ndpd state is wiped out, but the kernel state remains.

I'm going to try a fix where we just check for root at the beginning of i_ipadm_delete_ipv6addrs so we get all or nothing in terms of permissions.

Actions #3

Updated by Ryan Goodfellow 25 days ago

Testing Notes

Here are the original repro steps with the patch applied. The partially deleted behavior no longer occurs.

ry@bld:~$ pfexec dladm create-simnet sim0
ry@bld:~$ pfexec ipadm create-addr -t -T addrconf sim0/v6
ry@bld:~$ ipadm delete-addr sim0/v6
ipadm: could not delete address: Permission denied
ry@bld:~$ pfexec ipadm delete-addr sim0/v6
ry@bld:~$ ipadm
ADDROBJ           TYPE     STATE        ADDR
lo0/v4            static   ok           127.0.0.1/8
vioif0/v4         dhcp     ok           10.47.0.179/24
lo0/v6            static   ok           ::1/128
Actions #4

Updated by Electric Monk 25 days ago

  • Gerrit CR set to 2169
Actions #5

Updated by Toomas Soome 25 days ago

Ryan Goodfellow wrote in #note-3:

Testing Notes

Here are the original repro steps with the patch applied. The partially deleted behavior no longer occurs.

[...]

Of course, when you need to have privileges to create-addr then the same privileges must be needed for delete-addr:)

Actions #6

Updated by Electric Monk 18 days ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 2514b110a74b7a0ce021feff362fe3c4c2352b43

commit  2514b110a74b7a0ce021feff362fe3c4c2352b43
Author: Ryan Goodfellow <ryan.goodfellow@oxide.computer>
Date:   2022-06-09T14:52:45.000Z

    14724 failed delete of IP address made it undeletable
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Gordon Ross <Gordon.W.Ross@gmail.com>
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Approved by: Joshua M. Clulow <josh@sysmgr.org>

Actions

Also available in: Atom PDF