net_admin apparmor denial when using Go

Bug #1465724 reported by Rick Spencer
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Snappy
Fix Released
High
Tyler Hicks
linux (Ubuntu)
Fix Released
Undecided
Unassigned
Xenial
Fix Released
Undecided
Unassigned

Bug Description

SRU Justification:

Impact: A noisy AppArmor denial is reported to the system logs when a go program is run as a privileged user. The denial is non-fatal and is simply the result of the proc net systctl code determining what permissions a new inode should have. This noisy denial has a high potential to confuse snap packagers because they may think that their application is not working under Snappy confinement. It has a high potential to confuse Snappy users because they may think that the snaps running on their system are malicious.

Fix: The fix was authored by Tyler Hicks and acked by Serge Hallyn. It creates a new ns_capable() function that calls into the LSM hooks with the noaudit flag set so that the LSM doesn't generate a denial if the application under confinement is missing the CAP_NET_ADMIN capability

Testcase:
  # Load a test AppArmor profile
  $ echo "profile test { file, }" | sudo apparmor_parser -rq
  # Read a proc net sysctl file as root under confinement:
  $ sudo aa-exec -p test -- cat /proc/sys/net/core/somaxconn
  128
  # Manually inspect /var/log/syslog (or, if auditd is running, /var/log/audit/audit.log) to verify that the following denial is *NOT* present:
  # audit: type=1400 audit(1462575670.000:29): apparmor="DENIED" operation="capable" profile="test" pid=1161 comm="cat" capability=12 capname="net_admin"

Original report:

Somewhere in the following code, this denial gets thrown. It's difficult to tell where because the report of the denial seems to be asynchronous, as it comes interspersed with all the other debug information being printed to stdout.

http://bazaar.launchpad.net/~rick-rickspencer3/+junk/go-uploader/view/head:/main.go

Jun 16 14:21:51 localhost kernel: [ 7488.856306] audit: type=1400 audit(1434464511.427:41): apparmor="DENIED" operation="capable" profile="go-uploader.sideload_go-uploader_0.3" pid=1493 comm="go-uploader" capability=12 capname="net_admin"

I can fix it by adding capability net_admin to /var/lib/apparmor/profiles/click_go-uploader.sideload_go-uploader_0.3 and rerunning apparmor_parser

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

For those reading this bug report, "net_admin" is used for the following (from man capabilities):
* interface configuration;
* administration of IP firewall, masquerading, and accounting;
* modify routing tables;
* bind to any address for transparent proxying;
* set type-of-service (TOS)
* clear driver statistics;
* set promiscuous mode;
* enabling multicasting;
* use setsockopt(2) to set the following socket options: SO_DEBUG, SO_MARK, SO_PRIORITY (for a priority outside the range 0 to 6), SO_RCVBUFFORCE, and SO_SNDBUFFORCE.

This is quite a set of privileges and our AppArmor policy is correctly denying the access.

I have a feeling this is a harmless denial related to setsockopt() by the "net/http" import and that go tries to do something with setsockopt and happily proceeds if it cannot.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

FYI,

`go-example-webserver` suffers from the same denial, but operates fine otherwise:
audit: type=1400 audit(1434476614.662:14): apparmor="DENIED" operation="capable" profile="go-example-webserver.canonical_webserver_1.0.7" pid=850 comm="go-example-webs" capability=12 capname="net_admin"

http://bazaar.launchpad.net/~snappy-dev/snappy-hub/snappy-examples/view/head:/go-example-webserver/main.go is a simpler reproducer.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I was wrong about net/http. This is the simplest reproducer:

$ cat /tmp/profile
profile "foo" {
   network,
   ptrace,
   signal,
   file,
}

$ cat /tmp/main.go
package main
func main() {
}

$ sudo apparmor_parser -r /tmp/profile && sudo aa-exec -p foo go run /tmp/main.go

then see this denial:
audit: type=1400 audit(1434479051.291:326): apparmor="DENIED" operation="capable" profile="foo" pid=30185 comm="go" capability=12 capname="net_admin"

Running it as non-root does not show the denial. Eg,
$ sudo apparmor_parser -r /tmp/profile && aa-exec -p foo go run /tmp/main.go

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

FYI, this issue exists all the way back to 12.04 (/tmp/profile should omit ptrace and signal when reproducing).

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Even these two commands trigger the CAP_NET_ADMIN denial:

$ sudo apparmor_parser -r /tmp/profile && sudo aa-exec -p foo go --version
$ sudo apparmor_parser -r /tmp/profile && sudo aa-exec -p foo go --help

Revision history for this message
Tyler Hicks (tyhicks) wrote :

The capable(CAP_NET_ADMIN) check is triggered by this syscall that go makes:

  2223 open("/proc/sys/net/core/somaxconn", O_RDONLY|O_CLOEXEC) = 3

You can reproduce it by running the following command with and without sudo:

$ aa-exec -p foo -- cat /proc/sys/net/core/somaxconn # Won't trigger the denial
$ sudo aa-exec -p foo -- cat /proc/sys/net/core/somaxconn # Will trigger the denial

Revision history for this message
Tyler Hicks (tyhicks) wrote :

If we want to track down the kernel code that is triggering this capable() check when running as root, here's a stack trace called from AppArmor's audit_caps() function. Note that I generated this on a vanilla kernel (4.1-rc8).

CPU: 1 PID: 2199 Comm: go Not tainted 4.1.0-rc8+ #120
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
 00000000ffffffff ffff880079ff3a88 ffffffffb0634f04 0000000080000001
 0000000000001000 ffff880079ff3b48 ffffffffb0348bf1 ffff88007f9d75d0
 00000000001d2480 0000000c00000007 00000000001d2480 ffff880079ff3a03
Call Trace:
 [<ffffffffb0634f04>] dump_stack+0x4f/0x7b
 [<ffffffffb0348bf1>] aa_capable+0x321/0x330
 [<ffffffffb035271d>] apparmor_capable+0x4d/0x70
 [<ffffffffb030d528>] security_capable+0x18/0x20
 [<ffffffffb00719ec>] ns_capable+0x3c/0x70
 [<ffffffffb062a05a>] net_ctl_permissions+0x4a/0xd0
 [<ffffffffb0270370>] proc_sys_permission+0x70/0xc0
 [<ffffffffb01f9c0f>] __inode_permission+0x6f/0xd0
 [<ffffffffb01f9c88>] inode_permission+0x18/0x50
 [<ffffffffb01fcfbb>] link_path_walk+0x6b/0x970
 [<ffffffffb03c7587>] ? debug_smp_processor_id+0x17/0x20
 [<ffffffffb00bef7e>] ? put_lock_stats.isra.22+0xe/0x30
 [<ffffffffb00bf027>] ? lock_release_holdtime.part.23+0x87/0x170
 [<ffffffffb01fd979>] path_init+0xb9/0x880
 [<ffffffffb01fefe6>] ? path_openat+0x66/0x6a0
 [<ffffffffb01fefe6>] path_openat+0x66/0x6a0
 [<ffffffffb000e5b9>] ? sched_clock+0x9/0x10
 [<ffffffffb00a30c5>] ? sched_clock_local+0x25/0x90
 [<ffffffffb02100cf>] ? __alloc_fd+0xaf/0x190
 [<ffffffffb03c7587>] ? debug_smp_processor_id+0x17/0x20
 [<ffffffffb00be3cb>] ? get_lock_stats+0x2b/0x60
 [<ffffffffb02005aa>] do_filp_open+0x3a/0xb0
 [<ffffffffb02100cf>] ? __alloc_fd+0xaf/0x190
 [<ffffffffb0127464>] ? __audit_syscall_entry+0xb4/0x110
 [<ffffffffb01eceb6>] do_sys_open+0x126/0x220
 [<ffffffffb0014433>] ? syscall_trace_enter_phase1+0x103/0x170
 [<ffffffffb01ecfce>] SyS_open+0x1e/0x20
 [<ffffffffb063e42e>] system_call_fastpath+0x12/0x76

Revision history for this message
Tyler Hicks (tyhicks) wrote :

One potential fix, although I don't know if it is technically correct, is to switch the ordering of the calls to ns_capable() and uid_eq() here:

  http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/sysctl_net.c#n48

Revision history for this message
Tyler Hicks (tyhicks) wrote :

I've verified that the proposed fix in comment #8 does work. I'll need to look at other ns_capable() call sites and get upstream's opinion before I can be confident in its correctness. Here's the raw patch:

diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index e7000be..77e6bbf 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -46,8 +46,8 @@ static int net_ctl_permissions(struct ctl_table_header *head,
  kgid_t root_gid = make_kgid(net->user_ns, 0);

  /* Allow network administrator to have same access as root. */
- if (ns_capable(net->user_ns, CAP_NET_ADMIN) ||
- uid_eq(root_uid, current_euid())) {
+ if (uid_eq(root_uid, current_euid()) ||
+ ns_capable(net->user_ns, CAP_NET_ADMIN)) {
   int mode = (table->mode >> 6) & 7;
   return (mode << 6) | (mode << 3) | mode;
  }

Revision history for this message
Rick Spencer (rick-rickspencer3) wrote :

For what it is worth, if any cares, here is a smaller code listing that produces the error

package main

import (
 "fmt"
 "io/ioutil"
 "log"
 "os"
 "path/filepath"
 "time"
)
import "gopkg.in/yaml.v2"

var config Config

type Config struct {
 Duration string
}

func appDataPath() string {
 //use the full path to the app when in a snap package, but
 //for local development, assume that the app is run from bin/arch
 dir := os.Getenv("SNAP_APP_PATH")
 fmt.Println("app path ENV is " + dir)

 if dir == "" {
  fmt.Println("no snap app data path, guessing ../../")
  dir = "../../"
 }

 return dir
}

func initConfig() {
 //create a Config instance and fill it from the yaml file
 dir := appDataPath()

 yamlfilename := filepath.Join(dir, "cnf", "config.yaml")
 fmt.Println("Looking for yaml file at: " + yamlfilename)

 yamlFile, err := ioutil.ReadFile(yamlfilename)

 err = yaml.Unmarshal(yamlFile, &config)
 if err != nil {
  fmt.Println("failed to read config")
  log.Fatal(err)
 } else {
  fmt.Println("config created")
 }
}

func main() {
 fmt.Println("Uploader started")
 initConfig()
 s, err := time.ParseDuration(config.Duration)
 if err != nil {
  fmt.Println("failed to read seconds from config")
  log.Fatal(err)
 }
 ticker := time.NewTicker(s)
 for _ = range ticker.C {
  fmt.Println("tick")
 }

Revision history for this message
Tyler Hicks (tyhicks) wrote :

It is worth noting that this particular AppArmor denial is harmless. It does not affect go in any way because go is still able to read the /proc/sys/net/core/somaxconn file. I'll revisit this issue next week and get a proper patch ready to submit upstream.

Revision history for this message
Rick Spencer (rick-rickspencer3) wrote :

For clarity, this bug causes my service to fail, and I have to work around it by adding the net_admin cap.

Revision history for this message
Michael Vogt (mvo) wrote :

@Tyler did you get a chance to look at this again (as indicated in #11)?

Changed in snappy:
status: New → Incomplete
Revision history for this message
Tyler Hicks (tyhicks) wrote :

I have not yet had a chance to look at this issue again. However, Jamie helped Rick manually add 'capability net_admin,' to his service's AppArmor profile so that the denial wouldn't occur. After that change, Rick was able to verify that the AppArmor denial was not the cause of failure for his service.

We still plan to work with upstream to fix this issue soon but now feel confident that it is a harmless denial for root-runnign go apps.

Changed in snappy:
status: Incomplete → Confirmed
importance: Undecided → High
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I think this may also affect java:

Sep 23 22:46:07 localhost kernel: [ 8184.860885] audit: type=1400 audit(1443048367.549:213): apparmor="DENIED" operation="capable" profile="minecraft-server.sideload_srv_IBQYKDfdODHF" pid=10195 comm="java" capability=12 capname="net_admin"

but the server otherwise runs fine.

Tyler Hicks (tyhicks)
Changed in snappy:
assignee: nobody → Tyler Hicks (tyhicks)
status: Confirmed → In Progress
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Fixes sent to the upstream kernel lists:

  http://thread.gmane.org/gmane.linux.kernel.lsm/27927

Tyler Hicks (tyhicks)
description: updated
Tyler Hicks (tyhicks)
description: updated
Changed in linux (Ubuntu Xenial):
status: New → Fix Committed
Revision history for this message
Brad Figg (brad-figg) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 1465724

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
Revision history for this message
Kamal Mostafa (kamalmostafa) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-xenial' to 'verification-done-xenial'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-xenial
Tyler Hicks (tyhicks)
tags: added: verification-done-xenial
removed: verification-needed-xenial
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (24.1 KiB)

This bug was fixed in the package linux - 4.4.0-25.44

---------------
linux (4.4.0-25.44) xenial; urgency=low

  [ Kamal Mostafa ]

  * Release Tracking Bug
    - LP: #1591289

  * Xenial update to v4.4.13 stable release (LP: #1590455)
    - MIPS64: R6: R2 emulation bugfix
    - MIPS: math-emu: Fix jalr emulation when rd == $0
    - MIPS: MSA: Fix a link error on `_init_msa_upper' with older GCC
    - MIPS: Don't unwind to user mode with EVA
    - MIPS: Avoid using unwind_stack() with usermode
    - MIPS: Fix siginfo.h to use strict posix types
    - MIPS: Fix uapi include in exported asm/siginfo.h
    - MIPS: Fix watchpoint restoration
    - MIPS: Flush highmem pages in __flush_dcache_page
    - MIPS: Handle highmem pages in __update_cache
    - MIPS: Sync icache & dcache in set_pte_at
    - MIPS: ath79: make bootconsole wait for both THRE and TEMT
    - MIPS: Reserve nosave data for hibernation
    - MIPS: Loongson-3: Reserve 32MB for RS780E integrated GPU
    - MIPS: Use copy_s.fmt rather than copy_u.fmt
    - MIPS: Fix MSA ld_*/st_* asm macros to use PTR_ADDU
    - MIPS: Prevent "restoration" of MSA context in non-MSA kernels
    - MIPS: Disable preemption during prctl(PR_SET_FP_MODE, ...)
    - MIPS: ptrace: Fix FP context restoration FCSR regression
    - MIPS: ptrace: Prevent writes to read-only FCSR bits
    - MIPS: Fix sigreturn via VDSO on microMIPS kernel
    - MIPS: Build microMIPS VDSO for microMIPS kernels
    - MIPS: lib: Mark intrinsics notrace
    - MIPS: VDSO: Build with `-fno-strict-aliasing'
    - affs: fix remount failure when there are no options changed
    - ASoC: ak4642: Enable cache usage to fix crashes on resume
    - Input: uinput - handle compat ioctl for UI_SET_PHYS
    - ARM: mvebu: fix GPIO config on the Linksys boards
    - ARM: dts: at91: fix typo in sama5d2 PIN_PD24 description
    - ARM: dts: exynos: Add interrupt line to MAX8997 PMIC on exynos4210-trats
    - ARM: dts: imx35: restore existing used clock enumeration
    - ath9k: Add a module parameter to invert LED polarity.
    - ath9k: Fix LED polarity for some Mini PCI AR9220 MB92 cards.
    - ath10k: fix debugfs pktlog_filter write
    - ath10k: fix firmware assert in monitor mode
    - ath10k: fix rx_channel during hw reconfigure
    - ath10k: fix kernel panic, move arvifs list head init before htt init
    - ath5k: Change led pin configuration for compaq c700 laptop
    - hwrng: exynos - Fix unbalanced PM runtime put on timeout error path
    - rtlwifi: rtl8723be: Add antenna select module parameter
    - rtlwifi: btcoexist: Implement antenna selection
    - rtlwifi: Fix logic error in enter/exit power-save mode
    - rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in
      rtl_pci_reset_trx_ring
    - aacraid: Relinquish CPU during timeout wait
    - aacraid: Fix for aac_command_thread hang
    - aacraid: Fix for KDUMP driver hang
    - hwmon: (ads7828) Enable internal reference
    - mfd: intel-lpss: Save register context on suspend
    - mfd: intel_soc_pmic_core: Terminate panel control GPIO lookup table
      correctly
    - PM / Runtime: Fix error path in pm_runtime_force_resume()
    - cpuidle: Indicate when a device has been unregiste...

Changed in linux (Ubuntu):
status: Incomplete → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (26.1 KiB)

This bug was fixed in the package linux - 4.4.0-28.47

---------------
linux (4.4.0-28.47) xenial; urgency=low

  [ Luis Henriques ]

  * Release Tracking Bug
    - LP: #1595874

  * Linux netfilter local privilege escalation issues (LP: #1595350)
    - netfilter: x_tables: don't move to non-existent next rule
    - netfilter: x_tables: validate targets of jumps
    - netfilter: x_tables: add and use xt_check_entry_offsets
    - netfilter: x_tables: kill check_entry helper
    - netfilter: x_tables: assert minimum target size
    - netfilter: x_tables: add compat version of xt_check_entry_offsets
    - netfilter: x_tables: check standard target size too
    - netfilter: x_tables: check for bogus target offset
    - netfilter: x_tables: validate all offsets and sizes in a rule
    - netfilter: x_tables: don't reject valid target size on some architectures
    - netfilter: arp_tables: simplify translate_compat_table args
    - netfilter: ip_tables: simplify translate_compat_table args
    - netfilter: ip6_tables: simplify translate_compat_table args
    - netfilter: x_tables: xt_compat_match_from_user doesn't need a retval
    - netfilter: x_tables: do compat validation via translate_table
    - netfilter: x_tables: introduce and use xt_copy_counters_from_user

  * Linux netfilter IPT_SO_SET_REPLACE memory corruption (LP: #1555338)
    - netfilter: x_tables: validate e->target_offset early
    - netfilter: x_tables: make sure e->next_offset covers remaining blob size
    - netfilter: x_tables: fix unconditional helper

linux (4.4.0-27.46) xenial; urgency=low

  [ Kamal Mostafa ]

  * Release Tracking Bug
    - LP: #1594906

  * Support Edge Gateway's Bluetooth LED (LP: #1512999)
    - Revert "UBUNTU: SAUCE: Bluetooth: Support for LED on Marvell modules"

linux (4.4.0-26.45) xenial; urgency=low

  [ Kamal Mostafa ]

  * Release Tracking Bug
    - LP: #1594442

  * linux: Implement secure boot state variables (LP: #1593075)
    - SAUCE: UEFI: Add secure boot and MOK SB State disabled sysctl

  * failures building userspace packages that include ethtool.h (LP: #1592930)
    - ethtool.h: define INT_MAX for userland

linux (4.4.0-25.44) xenial; urgency=low

  [ Kamal Mostafa ]

  * Release Tracking Bug
    - LP: #1591289

  * Xenial update to v4.4.13 stable release (LP: #1590455)
    - MIPS64: R6: R2 emulation bugfix
    - MIPS: math-emu: Fix jalr emulation when rd == $0
    - MIPS: MSA: Fix a link error on `_init_msa_upper' with older GCC
    - MIPS: Don't unwind to user mode with EVA
    - MIPS: Avoid using unwind_stack() with usermode
    - MIPS: Fix siginfo.h to use strict posix types
    - MIPS: Fix uapi include in exported asm/siginfo.h
    - MIPS: Fix watchpoint restoration
    - MIPS: Flush highmem pages in __flush_dcache_page
    - MIPS: Handle highmem pages in __update_cache
    - MIPS: Sync icache & dcache in set_pte_at
    - MIPS: ath79: make bootconsole wait for both THRE and TEMT
    - MIPS: Reserve nosave data for hibernation
    - MIPS: Loongson-3: Reserve 32MB for RS780E integrated GPU
    - MIPS: Use copy_s.fmt rather than copy_u.fmt
    - MIPS: Fix MSA ld_*/st_* asm macros to use PTR_ADDU
    - MIPS: Prevent "restoration" of MSA c...

Changed in linux (Ubuntu Xenial):
status: Fix Committed → Fix Released
Tyler Hicks (tyhicks)
Changed in snappy:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.