* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: James Bottomley @ 2019-05-14 23:54 UTC (permalink / raw)
To: Rob Landley, Andy Lutomirski
Cc: Arvind Sankar, LKML, Linux API, Linux FS Devel, linux-integrity,
initramfs
In-Reply-To: <4da3dbda-bb76-5d71-d5c5-c03d98350ab0@landley.net>
On Tue, 2019-05-14 at 18:39 -0500, Rob Landley wrote:
> On 5/14/19 2:18 PM, James Bottomley wrote:
> > > I think Rob is right here. If /init was statically built into
> > > the kernel image, it has no more ability to compromise the kernel
> > > than anything else in the kernel. What's the problem here?
> >
> > The specific problem is that unless you own the kernel signing key,
> > which is really untrue for most distribution consumers because the
> > distro owns the key, you cannot build the initrd statically into
> > the kernel. You can take the distro signed kernel, link it with
> > the initrd then resign the combination with your key, provided you
> > insert your key into the MoK variables as a trusted secure boot
> > key, but the distros have been unhappy recommending this as
> > standard practice.
> >
> > If our model for security is going to be to link the kernel and the
> > initrd statically to give signature protection over the aggregate
> > then we need to figure out how to execute this via the distros. If
> > we accept that the split model, where the distro owns and signs the
> > kernel but the machine owner builds and is responsible for the
> > initrd, then we need to explore split security models like this
> > proposal.
>
> You can have a built-in and an external initrd? The second extracts
> over the first? (I know because once upon a time conflicting files
> would append. It sounds like the desired behavior here is O_EXCL fail
> and move on.)
Technically yes, because the first initrd could find the second by some
predefined means, extract it to a temporary directory and do a
pivot_root() and then the second would do some stuff, find the real
root and do a pivot_root() again. However, while possible, wouldn't it
just add to the rendezvous complexity without adding any benefits? even
if the first initrd is built and signed by the distro and the second is
built by you, the first has to verify the second somehow. I suppose
the second could be tar extracted, which would add xattrs, if that's
the goal?
James
^ permalink raw reply
* Re: [LTP] [EXT] Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yury Norov @ 2019-05-14 23:41 UTC (permalink / raw)
To: Cyril Hrubis
Cc: Yuri Norov, Andreas Schwab, ltp@lists.linux.it,
linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, Prasun Kapoor,
Alexander Graf, Bamvor Zhangjian, Geert Uytterhoeven, Dave Martin,
Adam Borowski, Manuel Montezelo
In-Reply-To: <20190514104311.GA24708@rei>
On Tue, May 14, 2019 at 12:43:11PM +0200, Cyril Hrubis wrote:
> Hi!
> > > There is a problem with the stack size accounting during execve when
> > > there is no stack limit:
> > >
> > > $ ulimit -s
> > > 8192
> > > $ ./hello.ilp32
> > > Hello World!
> > > $ ulimit -s unlimited
> > > $ ./hello.ilp32
> > > Segmentation fault
> > > $ strace ./hello.ilp32
> > > execve("./hello.ilp32", ["./hello.ilp32"], 0xfffff10548f0 /* 77 vars */) = -1 ENOMEM (Cannot allocate memory)
> > > +++ killed by SIGSEGV +++
> > > Segmentation fault (core dumped)
> > >
> > > Andreas.
> >
> > Thanks Andreas, I will take a look. Do we have such test in LTP?
>
> We do have a test that we can run a binary with very small stack size
> i.e. 512kB but there does not seem to be anything that would catch this
> specific problem.
>
> Can you please open an issue and describe how to reproduce the problem
> at our github tracker:
>
> https://github.com/linux-test-project/ltp/issues
>
> Then we can create testcase based on that reproducer later on.
This is it:
https://github.com/linux-test-project/ltp/issues/530
Yury
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-14 23:39 UTC (permalink / raw)
To: James Bottomley, Andy Lutomirski
Cc: Arvind Sankar, LKML, Linux API, Linux FS Devel, linux-integrity,
initramfs
In-Reply-To: <1557861511.3378.19.camel@HansenPartnership.com>
On 5/14/19 2:18 PM, James Bottomley wrote:
>> I think Rob is right here. If /init was statically built into the
>> kernel image, it has no more ability to compromise the kernel than
>> anything else in the kernel. What's the problem here?
>
> The specific problem is that unless you own the kernel signing key,
> which is really untrue for most distribution consumers because the
> distro owns the key, you cannot build the initrd statically into the
> kernel. You can take the distro signed kernel, link it with the initrd
> then resign the combination with your key, provided you insert your key
> into the MoK variables as a trusted secure boot key, but the distros
> have been unhappy recommending this as standard practice.
>
> If our model for security is going to be to link the kernel and the
> initrd statically to give signature protection over the aggregate then
> we need to figure out how to execute this via the distros. If we
> accept that the split model, where the distro owns and signs the kernel
> but the machine owner builds and is responsible for the initrd, then we
> need to explore split security models like this proposal.
You can have a built-in and an external initrd? The second extracts over the
first? (I know because once upon a time conflicting files would append. It
sounds like the desired behavior here is O_EXCL fail and move on.)
Rob
^ permalink raw reply
* Re: [LTP] [EXT] Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yury Norov @ 2019-05-14 23:01 UTC (permalink / raw)
To: Cyril Hrubis
Cc: Yuri Norov, Andreas Schwab, ltp@lists.linux.it,
linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, Prasun Kapoor,
Alexander Graf, Bamvor Zhangjian, Geert Uytterhoeven, Dave Martin,
Adam Borowski, Manuel Montezelo
In-Reply-To: <20190514104311.GA24708@rei>
On Tue, May 14, 2019 at 12:43:11PM +0200, Cyril Hrubis wrote:
> Hi!
> > > There is a problem with the stack size accounting during execve when
> > > there is no stack limit:
> > >
> > > $ ulimit -s
> > > 8192
> > > $ ./hello.ilp32
> > > Hello World!
> > > $ ulimit -s unlimited
> > > $ ./hello.ilp32
> > > Segmentation fault
> > > $ strace ./hello.ilp32
> > > execve("./hello.ilp32", ["./hello.ilp32"], 0xfffff10548f0 /* 77 vars */) = -1 ENOMEM (Cannot allocate memory)
> > > +++ killed by SIGSEGV +++
> > > Segmentation fault (core dumped)
> > >
> > > Andreas.
> >
> > Thanks Andreas, I will take a look. Do we have such test in LTP?
So the problem was in not converting new compat-sensitive code:
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 5bdf357169d8..c509f83fa506 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -63,7 +63,7 @@
TASK_SIZE_32 : TASK_SIZE_64)
#define TASK_SIZE_OF(tsk) (is_compat_thread(tsk) ? \
TASK_SIZE_32 : TASK_SIZE_64)
-#define DEFAULT_MAP_WINDOW (test_thread_flag(TIF_32BIT) ? \
+#define DEFAULT_MAP_WINDOW (is_compat_task() ? \
TASK_SIZE_32 : DEFAULT_MAP_WINDOW_64)
#else
#define TASK_SIZE TASK_SIZE_64
The fix is incorporated in ilp32-5.1.1:
https://github.com/norov/linux/tree/ilp32-5.1.1
> We do have a test that we can run a binary with very small stack size
> i.e. 512kB but there does not seem to be anything that would catch this
> specific problem.
>
> Can you please open an issue and describe how to reproduce the problem
> at our github tracker:
>
> https://github.com/linux-test-project/ltp/issues
>
> Then we can create testcase based on that reproducer later on.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
OK, I'll do.
Yury
^ permalink raw reply related
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: James Bottomley @ 2019-05-14 19:18 UTC (permalink / raw)
To: Andy Lutomirski, Roberto Sassu
Cc: Rob Landley, Arvind Sankar, LKML, Linux API, Linux FS Devel,
linux-integrity, initramfs
In-Reply-To: <CALCETrV3b205L38xqPr6QqwGn6-vxQdPoJGUygJJpgM-JqqXfQ@mail.gmail.com>
On Tue, 2019-05-14 at 08:19 -0700, Andy Lutomirski wrote:
> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.c
> om> wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
[...]
> > > > The only reason why opening .xattr-list works is that IMA is
> > > > not yet initialized (late_initcall vs rootfs_initcall).
> > >
> > > Launching init before enabling ima is bad because... you didn't
> > > think of it?
> >
> > No, because /init can potentially compromise the integrity of the
> > system.
>
> I think Rob is right here. If /init was statically built into the
> kernel image, it has no more ability to compromise the kernel than
> anything else in the kernel. What's the problem here?
The specific problem is that unless you own the kernel signing key,
which is really untrue for most distribution consumers because the
distro owns the key, you cannot build the initrd statically into the
kernel. You can take the distro signed kernel, link it with the initrd
then resign the combination with your key, provided you insert your key
into the MoK variables as a trusted secure boot key, but the distros
have been unhappy recommending this as standard practice.
If our model for security is going to be to link the kernel and the
initrd statically to give signature protection over the aggregate then
we need to figure out how to execute this via the distros. If we
accept that the split model, where the distro owns and signs the kernel
but the machine owner builds and is responsible for the initrd, then we
need to explore split security models like this proposal.
James
^ permalink raw reply
* [PATCH v4 12/13] platform/x86: asus-wmi: Switch fan boost mode
From: Yurii Pavlovskyi @ 2019-05-14 19:07 UTC (permalink / raw)
Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel,
linux-api
In-Reply-To: <c8cdb347-e206-76b2-0d43-546ef660ffb7@gmail.com>
The WMI exposes a write-only device ID where up to three fan modes can be
switched on some laptops (TUF Gaming FX505GM). There is a hotkey
combination Fn-F5 that does have a fan icon, which is designed to toggle
between fan modes. The DSTS of the device ID returns information about the
presence of this capability and the presence of each of the two additional
fan modes as a bitmask (0x01 - overboost present, 0x02 - silent present)
[1].
Add a SysFS entry that reads the last written value and updates value in
WMI on write and a hotkey handler that toggles the modes taking into
account their availability according to DSTS.
Modes:
* 0x00 - normal or balanced,
* 0x01 - overboost, increased fan RPM,
* 0x02 - silent, decreased fan RPM
[1] Link: https://lkml.org/lkml/2019/4/12/110
Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
Suggested-by: Daniel Drake <drake@endlessm.com>
---
.../ABI/testing/sysfs-platform-asus-wmi | 10 ++
drivers/platform/x86/asus-wmi.c | 151 +++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 1 +
3 files changed, 154 insertions(+), 8 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
index 019e1e29370e..87ae5cc983bf 100644
--- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
+++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
@@ -36,3 +36,13 @@ KernelVersion: 3.5
Contact: "AceLan Kao" <acelan.kao@canonical.com>
Description:
Resume on lid open. 1 means on, 0 means off.
+
+What: /sys/devices/platform/<platform>/fan_mode
+Date: Apr 2019
+KernelVersion: 5.2
+Contact: "Yurii Pavlovskyi" <yurii.pavlovskyi@gmail.com>
+Description:
+ Fan boost mode:
+ * 0 - normal,
+ * 1 - overboost,
+ * 2 - silent
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ffb4e2530ea4..feb8d72fc3c5 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");
#define NOTIFY_KBD_BRTUP 0xc4
#define NOTIFY_KBD_BRTDWN 0xc5
#define NOTIFY_KBD_BRTTOGGLE 0xc7
+#define NOTIFY_KBD_FBM 0x99
#define ASUS_WMI_FNLOCK_BIOS_DISABLED BIT(0)
@@ -80,6 +81,13 @@ MODULE_LICENSE("GPL");
#define ASUS_FAN_CTRL_MANUAL 1
#define ASUS_FAN_CTRL_AUTO 2
+#define ASUS_FAN_MODE_NORMAL 0
+#define ASUS_FAN_MODE_OVERBOOST 1
+#define ASUS_FAN_MODE_OVERBOOST_MASK 0x01
+#define ASUS_FAN_MODE_SILENT 2
+#define ASUS_FAN_MODE_SILENT_MASK 0x02
+#define ASUS_FAN_MODES_MASK 0x03
+
#define USB_INTEL_XUSB2PR 0xD0
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
@@ -187,6 +195,10 @@ struct asus_wmi {
int asus_hwmon_num_fans;
int asus_hwmon_pwm;
+ bool fan_mode_available;
+ u8 fan_mode_mask;
+ u8 fan_mode;
+
struct hotplug_slot hotplug_slot;
struct mutex hotplug_lock;
struct mutex wmi_lock;
@@ -1483,6 +1495,116 @@ static int asus_wmi_fan_init(struct asus_wmi *asus)
return 0;
}
+/* Fan mode *******************************************************************/
+
+static int fan_mode_check_present(struct asus_wmi *asus)
+{
+ u32 result;
+ int err;
+
+ asus->fan_mode_available = false;
+
+ err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_FAN_MODE, &result);
+ if (err) {
+ if (err == -ENODEV)
+ return 0;
+ else
+ return err;
+ }
+
+ if ((result & ASUS_WMI_DSTS_PRESENCE_BIT) &&
+ (result & ASUS_FAN_MODES_MASK)) {
+ asus->fan_mode_available = true;
+ asus->fan_mode_mask = result & ASUS_FAN_MODES_MASK;
+ }
+
+ return 0;
+}
+
+static int fan_mode_write(struct asus_wmi *asus)
+{
+ int err;
+ u8 value;
+ u32 retval;
+
+ value = asus->fan_mode;
+
+ pr_info("Set fan mode: %u\n", value);
+ err = asus_wmi_set_devstate(ASUS_WMI_DEVID_FAN_MODE, value, &retval);
+
+ if (err) {
+ pr_warn("Failed to set fan mode: %d\n", err);
+ return err;
+ }
+
+ if (retval != 1) {
+ pr_warn("Failed to set fan mode (retval): 0x%x\n", retval);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int fan_mode_switch_next(struct asus_wmi *asus)
+{
+ if (asus->fan_mode == ASUS_FAN_MODE_NORMAL) {
+ if (asus->fan_mode_mask & ASUS_FAN_MODE_OVERBOOST_MASK)
+ asus->fan_mode = ASUS_FAN_MODE_OVERBOOST;
+ else if (asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK)
+ asus->fan_mode = ASUS_FAN_MODE_SILENT;
+ } else if (asus->fan_mode == ASUS_FAN_MODE_OVERBOOST) {
+ if (asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK)
+ asus->fan_mode = ASUS_FAN_MODE_SILENT;
+ else
+ asus->fan_mode = ASUS_FAN_MODE_NORMAL;
+ } else {
+ asus->fan_mode = ASUS_FAN_MODE_NORMAL;
+ }
+
+ return fan_mode_write(asus);
+}
+
+static ssize_t fan_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ return scnprintf(buf, PAGE_SIZE, "%d\n", asus->fan_mode);
+}
+
+static ssize_t fan_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int result;
+ u8 new_mode;
+
+ struct asus_wmi *asus = dev_get_drvdata(dev);
+
+ result = kstrtou8(buf, 10, &new_mode);
+ if (result < 0) {
+ pr_warn("Trying to store invalid value\n");
+ return result;
+ }
+
+ if (new_mode == ASUS_FAN_MODE_OVERBOOST) {
+ if (!(asus->fan_mode_mask & ASUS_FAN_MODE_OVERBOOST_MASK))
+ return -EINVAL;
+ } else if (new_mode == ASUS_FAN_MODE_SILENT) {
+ if (!(asus->fan_mode_mask & ASUS_FAN_MODE_SILENT_MASK))
+ return -EINVAL;
+ } else if (new_mode != ASUS_FAN_MODE_NORMAL) {
+ return -EINVAL;
+ }
+
+ asus->fan_mode = new_mode;
+ fan_mode_write(asus);
+
+ return result;
+}
+
+// Fan mode: 0 - normal, 1 - overboost, 2 - silent
+static DEVICE_ATTR_RW(fan_mode);
+
/* Backlight ******************************************************************/
static int read_backlight_power(struct asus_wmi *asus)
@@ -1761,6 +1883,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
return;
}
+ if (asus->fan_mode_available && code == NOTIFY_KBD_FBM) {
+ fan_mode_switch_next(asus);
+ return;
+ }
+
if (is_display_toggle(code) && asus->driver->quirks->no_display_toggle)
return;
@@ -1917,6 +2044,7 @@ static struct attribute *platform_attributes[] = {
&dev_attr_touchpad.attr,
&dev_attr_lid_resume.attr,
&dev_attr_als_enable.attr,
+ &dev_attr_fan_mode.attr,
NULL
};
@@ -1938,6 +2066,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
devid = ASUS_WMI_DEVID_LID_RESUME;
else if (attr == &dev_attr_als_enable.attr)
devid = ASUS_WMI_DEVID_ALS_ENABLE;
+ else if (attr == &dev_attr_fan_mode.attr)
+ ok = asus->fan_mode_available;
if (devid != -1)
ok = !(asus_wmi_get_devstate_simple(asus, devid) < 0);
@@ -2037,12 +2167,7 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
asus_wmi_set_devstate(ASUS_WMI_DEVID_CWAP,
asus->driver->quirks->wapf, NULL);
- return asus_wmi_sysfs_init(asus->platform_device);
-}
-
-static void asus_wmi_platform_exit(struct asus_wmi *asus)
-{
- asus_wmi_sysfs_exit(asus->platform_device);
+ return 0;
}
/* debugfs ********************************************************************/
@@ -2221,6 +2346,14 @@ static int asus_wmi_add(struct platform_device *pdev)
if (err)
goto fail_platform;
+ err = fan_mode_check_present(asus);
+ if (err)
+ goto fail_fan_mode;
+
+ err = asus_wmi_sysfs_init(asus->platform_device);
+ if (err)
+ goto fail_sysfs;
+
err = asus_wmi_input_init(asus);
if (err)
goto fail_input;
@@ -2302,7 +2435,9 @@ static int asus_wmi_add(struct platform_device *pdev)
fail_hwmon:
asus_wmi_input_exit(asus);
fail_input:
- asus_wmi_platform_exit(asus);
+ asus_wmi_sysfs_exit(asus->platform_device);
+fail_sysfs:
+fail_fan_mode:
fail_platform:
kfree(asus);
return err;
@@ -2319,7 +2454,7 @@ static int asus_wmi_remove(struct platform_device *device)
asus_wmi_led_exit(asus);
asus_wmi_rfkill_exit(asus);
asus_wmi_debugfs_exit(asus);
- asus_wmi_platform_exit(asus);
+ asus_wmi_sysfs_exit(asus->platform_device);
asus_hwmon_fan_set_auto(asus);
kfree(asus);
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 0668f76df921..8551156b8dca 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -57,6 +57,7 @@
#define ASUS_WMI_DEVID_KBD_BACKLIGHT 0x00050021
#define ASUS_WMI_DEVID_LIGHT_SENSOR 0x00050022 /* ?? */
#define ASUS_WMI_DEVID_LIGHTBAR 0x00050025
+#define ASUS_WMI_DEVID_FAN_MODE 0x00110018
/* Misc */
#define ASUS_WMI_DEVID_CAMERA 0x00060013
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-14 17:44 UTC (permalink / raw)
To: Arvind Sankar
Cc: Rob Landley, Arvind Sankar, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190514155739.GA70223@rani.riverdale.lan>
On 5/14/2019 5:57 PM, Arvind Sankar wrote:
> On Tue, May 14, 2019 at 11:27:04AM -0400, Arvind Sankar wrote:
>> It's also much easier to change/customize it for the end
>> system's requirements rather than setting the process in stone by
>> putting it inside the kernel.
>
> As an example, if you allow unverified external initramfs, it seems to
> me that it can try to play games that wouldn't be prevented by the
> in-kernel code: setup /dev in a weird way to try to trick /init, or more
> easily, replace /init by /bin/sh so you get a shell prompt while only
> the initramfs is loaded. It's easy to imagine that a system would want
> to lock itself down to prevent abuses like this.
Yes, these issues should be addressed. But the purpose of this patch set
is simply to set xattrs. And existing protection mechanisms can be
improved later when the basic functionality is there.
> So you might already want an embedded initramfs that can be trusted and
> that can't be overwritten by an external one even outside the
> security.ima stuff.
The same problems exist also the root filesystem. These should be solved
regardless of the filesystem used, for remote attestation and for local
enforcement.
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-14 17:20 UTC (permalink / raw)
To: Greg KH
Cc: Andy Lutomirski, Rob Landley, Arvind Sankar, LKML, Linux API,
Linux FS Devel, linux-integrity, initramfs
In-Reply-To: <20190514165842.GC28266@kroah.com>
On 5/14/2019 6:58 PM, Greg KH wrote:
> On Tue, May 14, 2019 at 06:33:29PM +0200, Roberto Sassu wrote:
>> On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
>>> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>>>>
>>>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>>
>>>>>
>>>>> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>>>>>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>>>>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>>>>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>>>>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>>>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>>>>>>> been extracted.
>>>>>>>>>>
>>>>>>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>>>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>>>>>>> actually required here?
>>>>>>>>>
>>>>>>>>> It's too late. The /init itself should be signed and verified.
>>>>>>>>
>>>>>>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>>>>>>> the init in it _not_ verified?
>>>>>>>>
>>>>>>>> Ro
>>>>>>>
>>>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>>>> initramfs:
>>>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>>>> signature, so no new code required.
>>>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>>>> is unverified.
>>>>>>
>>>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>>>> verify /init in the embedded initial ram disk.
>>>>>
>>>>> So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>>>>
>>>> The idea is to be able to verify anything that is accessed, as soon as
>>>> rootfs is available, without distinction between embedded or external
>>>> initial ram disk.
>>>>
>>>> Also, requiring an embedded initramfs for xattrs would be an issue for
>>>> systems that use it for other purposes.
>>>>
>>>>
>>>>>> The only reason why
>>>>>> opening .xattr-list works is that IMA is not yet initialized
>>>>>> (late_initcall vs rootfs_initcall).
>>>>>
>>>>> Launching init before enabling ima is bad because... you didn't think of it?
>>>>
>>>> No, because /init can potentially compromise the integrity of the
>>>> system.
>>>
>>> I think Rob is right here. If /init was statically built into the
>>> kernel image, it has no more ability to compromise the kernel than
>>> anything else in the kernel. What's the problem here?
>>
>> Right, the measurement/signature verification of the kernel image is
>> sufficient.
>>
>> Now, assuming that we defer the IMA initialization until /init in the
>> embedded initramfs has been executed, the problem is how to handle
>> processes launched with the user mode helper or files directly read by
>> the kernel (if it can happen before /init is executed). If IMA is not
>> yet enabled, these operations will be performed without measurement and
>> signature verification.
>
> If you really care about this, don't launch any user mode helper
> programs (hint, you have the kernel option to control this and funnel
> everything into one, or no, binaries). And don't allow the kernel to
> read any files either, again, you have control over this.
>
> Or start IMA earlier if you need/want/care about this.
Yes, this is how it works now. It couldn't start earlier than
late_initcall, as it has to wait until the TPM driver is initialized.
Anyway, it is enabled at the time /init is executed. And this would be
an issue because launching /init and reading xattrs from /.xattr-list
would be denied (the signature is missing).
And /.xattr-list won't have a signature, if initramfs is generated
locally.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Greg KH @ 2019-05-14 16:58 UTC (permalink / raw)
To: Roberto Sassu
Cc: Andy Lutomirski, Rob Landley, Arvind Sankar, LKML, Linux API,
Linux FS Devel, linux-integrity, initramfs
In-Reply-To: <9357cb32-3803-2a7e-4949-f9e4554c1ee9@huawei.com>
On Tue, May 14, 2019 at 06:33:29PM +0200, Roberto Sassu wrote:
> On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
> > On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
> > >
> > > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > > >
> > > >
> > > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > > > > On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > > > > > On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > > > > > > On 5/12/19 7:52 AM, Mimi Zohar wrote:
> > > > > > > > On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> > > > > > > > > On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > > > > > > > > > This proposal consists in marshaling pathnames and xattrs in a file called
> > > > > > > > > > .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > > > > > > > > > been extracted.
> > > > > > > > >
> > > > > > > > > Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > > > > > > > > be done equivalently by the initramfs' /init? Why is kernel involvement
> > > > > > > > > actually required here?
> > > > > > > >
> > > > > > > > It's too late. The /init itself should be signed and verified.
> > > > > > >
> > > > > > > If the initramfs cpio.gz image was signed and verified by the extractor, how is
> > > > > > > the init in it _not_ verified?
> > > > > > >
> > > > > > > Ro
> > > > > >
> > > > > > Wouldn't the below work even before enforcing signatures on external
> > > > > > initramfs:
> > > > > > 1. Create an embedded initramfs with an /init that does the xattr
> > > > > > parsing/setting. This will be verified as part of the kernel image
> > > > > > signature, so no new code required.
> > > > > > 2. Add a config option/boot parameter to panic the kernel if an external
> > > > > > initramfs attempts to overwrite anything in the embedded initramfs. This
> > > > > > prevents overwriting the embedded /init even if the external initramfs
> > > > > > is unverified.
> > > > >
> > > > > Unfortunately, it wouldn't work. IMA is already initialized and it would
> > > > > verify /init in the embedded initial ram disk.
> > > >
> > > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> > >
> > > The idea is to be able to verify anything that is accessed, as soon as
> > > rootfs is available, without distinction between embedded or external
> > > initial ram disk.
> > >
> > > Also, requiring an embedded initramfs for xattrs would be an issue for
> > > systems that use it for other purposes.
> > >
> > >
> > > > > The only reason why
> > > > > opening .xattr-list works is that IMA is not yet initialized
> > > > > (late_initcall vs rootfs_initcall).
> > > >
> > > > Launching init before enabling ima is bad because... you didn't think of it?
> > >
> > > No, because /init can potentially compromise the integrity of the
> > > system.
> >
> > I think Rob is right here. If /init was statically built into the
> > kernel image, it has no more ability to compromise the kernel than
> > anything else in the kernel. What's the problem here?
>
> Right, the measurement/signature verification of the kernel image is
> sufficient.
>
> Now, assuming that we defer the IMA initialization until /init in the
> embedded initramfs has been executed, the problem is how to handle
> processes launched with the user mode helper or files directly read by
> the kernel (if it can happen before /init is executed). If IMA is not
> yet enabled, these operations will be performed without measurement and
> signature verification.
If you really care about this, don't launch any user mode helper
programs (hint, you have the kernel option to control this and funnel
everything into one, or no, binaries). And don't allow the kernel to
read any files either, again, you have control over this.
Or start IMA earlier if you need/want/care about this.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-14 16:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Rob Landley, Arvind Sankar, LKML, Linux API, Linux FS Devel,
linux-integrity, initramfs
In-Reply-To: <CALCETrV3b205L38xqPr6QqwGn6-vxQdPoJGUygJJpgM-JqqXfQ@mail.gmail.com>
On 5/14/2019 5:19 PM, Andy Lutomirski wrote:
> On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>>
>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>
>>>
>>> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>>>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
>>>>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>>>>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>>>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>>>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>>>>>>>> been extracted.
>>>>>>>>
>>>>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>>>>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
>>>>>>>> actually required here?
>>>>>>>
>>>>>>> It's too late. The /init itself should be signed and verified.
>>>>>>
>>>>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
>>>>>> the init in it _not_ verified?
>>>>>>
>>>>>> Ro
>>>>>
>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>> initramfs:
>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>> signature, so no new code required.
>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>> is unverified.
>>>>
>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>> verify /init in the embedded initial ram disk.
>>>
>>> So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>>
>> The idea is to be able to verify anything that is accessed, as soon as
>> rootfs is available, without distinction between embedded or external
>> initial ram disk.
>>
>> Also, requiring an embedded initramfs for xattrs would be an issue for
>> systems that use it for other purposes.
>>
>>
>>>> The only reason why
>>>> opening .xattr-list works is that IMA is not yet initialized
>>>> (late_initcall vs rootfs_initcall).
>>>
>>> Launching init before enabling ima is bad because... you didn't think of it?
>>
>> No, because /init can potentially compromise the integrity of the
>> system.
>
> I think Rob is right here. If /init was statically built into the
> kernel image, it has no more ability to compromise the kernel than
> anything else in the kernel. What's the problem here?
Right, the measurement/signature verification of the kernel image is
sufficient.
Now, assuming that we defer the IMA initialization until /init in the
embedded initramfs has been executed, the problem is how to handle
processes launched with the user mode helper or files directly read by
the kernel (if it can happen before /init is executed). If IMA is not
yet enabled, these operations will be performed without measurement and
signature verification.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-14 15:57 UTC (permalink / raw)
To: Roberto Sassu
Cc: Rob Landley, Arvind Sankar, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190514152704.GB37109@rani.riverdale.lan>
On Tue, May 14, 2019 at 11:27:04AM -0400, Arvind Sankar wrote:
> It's also much easier to change/customize it for the end
> system's requirements rather than setting the process in stone by
> putting it inside the kernel.
As an example, if you allow unverified external initramfs, it seems to
me that it can try to play games that wouldn't be prevented by the
in-kernel code: setup /dev in a weird way to try to trick /init, or more
easily, replace /init by /bin/sh so you get a shell prompt while only
the initramfs is loaded. It's easy to imagine that a system would want
to lock itself down to prevent abuses like this.
So you might already want an embedded initramfs that can be trusted and
that can't be overwritten by an external one even outside the
security.ima stuff.
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-14 15:27 UTC (permalink / raw)
To: Roberto Sassu
Cc: Rob Landley, Arvind Sankar, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <de91ef53-6bb3-b937-8773-5f6b34e1acb7@huawei.com>
On Tue, May 14, 2019 at 01:52:42PM +0200, Roberto Sassu wrote:
> On 5/14/2019 8:06 AM, Rob Landley wrote:
> > On 5/13/19 7:47 AM, Roberto Sassu wrote:
> >> On 5/13/2019 11:07 AM, Rob Landley wrote:
> >>>>> Wouldn't the below work even before enforcing signatures on external
> >>>>> initramfs:
> >>>>> 1. Create an embedded initramfs with an /init that does the xattr
> >>>>> parsing/setting. This will be verified as part of the kernel image
> >>>>> signature, so no new code required.
> >>>>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>>>> prevents overwriting the embedded /init even if the external initramfs
> >>>>> is unverified.
> >>>>
> >>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >>>> verify /init in the embedded initial ram disk.
> >>>
> >>> So you made broken infrastructure that's causing you problems. Sounds
> >>> unfortunate.
> >>
> >> The idea is to be able to verify anything that is accessed, as soon as
> >> rootfs is available, without distinction between embedded or external
> >> initial ram disk.
> >
> > If /init is in the internal one and you can't overwrite files with an external
> > one, all your init has to be is something that applies the xattrs, enables your
> > paranoia mode, and then execs something else.
>
> Shouldn't file metadata be handled by the same code that extracts the
> content? Instead, file content is extracted by the kernel, and we are
> adding another step to the boot process, to execute a new binary with a
> link to libc.
>
> From the perspective of a remote verifier that checks the software
> running on the system, would it be easier to check less than 150 lines
> of code, or a CPIO image containing a binary + libc?
>
Isn't the remote attestation process just involving the boot measurement
to verify that software has not changed, not actually reviewing the
code? How does it matter what's inside the kernel image as long as we
know it hasn't changed from when it was installed?
You don't need much of a libc in any case -- the userspace code is not
going to be doing anything different from what you're proposing to do
inside the kernel, and I would expect the userland code to be
easier to test and verify for correctness than code embedded in the
kernel's boot process. It shouldn't be any more complex than the kernel
version.
It's also much easier to change/customize it for the end
system's requirements rather than setting the process in stone by
putting it inside the kernel.
>
> > Heck, I do that sort of set up in shell scripts all the time. Running the shell
> > script as PID 1 and then having it exec the "real init" binary at the end:
> >
> > https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205
> >
> > If your first init binary is in the initramfs statically linked into the kernel
> > image, and the cpio code is doing open(O_EXCL), then it's as verified as any
> > other kernel code and runs "securely" until it decides to run something else.
> >
> >> Also, requiring an embedded initramfs for xattrs would be an issue for
> >> systems that use it for other purposes.
> >
> > I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
> > that will probably never go upstream because I'm a hobbyist and dealing with the
> > linux-kernel clique is the opposite of fun. I'm only in this conversation
> > because I was cc'd.)
> >
> > You can totally use initramfs for lots of purposes simultaneously.
>
> Yes, I agree. However, adding an initramfs to initialize another
> initramfs when you can simply extract file content and metadata with the
> same parser, this for me it is difficult to justify.
>
It's not really the same parser, right? It's just code that runs after
the parser is done, and the question is whether that code needs to live
inside the kernel or can be done in userspace.
>
> >>>> The only reason why
> >>>> opening .xattr-list works is that IMA is not yet initialized
> >>>> (late_initcall vs rootfs_initcall).
> >>>
> >>> Launching init before enabling ima is bad because... you didn't think of it?
> >>
> >> No, because /init can potentially compromise the integrity of the
> >> system.
> >
> > Which isn't a problem if it was statically linked in the kernel, or if your
> > external cpio.gz was signed. You want a signed binary but don't want the
> > signature _in_ the binary...
>
> It is not just for binaries. How you would deal with arbitrary file
> formats?
>
>
> >>>> Allowing a kernel with integrity enforcement to parse the CPIO image
> >>>> without verifying it first is the weak point.
> >>>
> >>> If you don't verify the CPIO image then in theory it could have anything in it,
> >>> yes. You seem to believe that signing individual files is more secure than
> >>> signing the archive. This is certainly a point of view.
> >>
> >> As I wrote above, signing the CPIO image would be more secure, if this
> >> option is available. However, a disadvantage would be that you have to
> >> sign the CPIO image every time a file changes.
> >
> > Which is why there's a cpio in the kernel and an external cpio loaded via the
> > old initrd mechanism and BOTH files wind up in the cpio and there's a way to
> > make it O_EXCL so it can't overwrite, and then the /init binary inside the
> > kernel's cpio can do any other weird verification you need to do before anything
> > else gets a chance to run so why are you having ring 0 kernel code read a file
> > out of the filesystem and act upon it?
>
> The CPIO parser already invokes many system calls.
>
>
> > (Heck, you can mv /newinit /init before the exec /init so the file isn't on the
> > system anymore by the time the other stuff gets to run...)
> >
> >>>> However, extracted files
> >>>> are not used, and before they are used they are verified. At the time
> >>>> they are verified, they (included /init) must already have a signature
> >>>> or otherwise access would be denied.
> >>>
> >>> You build infrastructure that works a certain way, the rest of the system
> >>> doesn't fit your assumptions, so you need to change the rest of the system to
> >>> fit your assumptions.
> >>
> >> Requiring file metadata to make decisions seems reasonable. Also
> >> mandatory access controls do that. The objective of this patch set is to
> >> have uniform behavior regardless of the filesystem used.
> >
> > If it's in the file's contents you get uniform behavior regardless of the
> > filesystem used. And "mandatory access controls do that" is basically restating
> > what _I_ said in the paragraph above.
>
> As I said, that does not work with arbitrary file formats.
>
>
> > The "infrastructure you have that works a certain way" is called "mandatory
> > access controls". Good to know. Your patch changes the rest of the system to
> > match the assumptions of the new code, because changing those assumptions
> > appears literally unthinkable.
>
> All I want to do is to have the same behavior as if there is no initial
> ram disk. And given that inode-based MACs read the labels from xattrs,
> the assumption that the system provides xattrs even in the inital ram
> disk seems reasonable.
>
>
> >>>> This scheme relies on the ability of the kernel to not be corrupted in
> >>>> the event it parses a malformed CPIO image.
> >>>
> >>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> >>> extraction code. You can fill up all physical memory with initramfs and lock the
> >>> system hard, though.
> >>>
> >>> It still only parses them at boot time before launching PID 1, right? So you
> >>> have a local physical exploit and you're trying to prevent people from working
> >>> around your Xbox copy protection without a mod chip?
> >>
> >> What do you mean exactly?
> >
> > That you're not remotely the first person to do this?
> >
> > You're attempting to prevent anyone from running third party code on your system
> > without buying a license from you first. You're creating a system with no user
> > serviceable parts, that only runs authorized software from the Apple Store or
> > other walled garden. No sideloading allowed.
>
> This is one use case. The main purpose of IMA is to preserve the
> integrity of the Trusted Computing Base (TCB, the critical part of the
> system), or to detect integrity violations without enforcement. This is
> done by ensuring that the software comes from the vendor. Applications
> owned by users are allowed to run, as the Discrectionary Access Control
> (DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
> that relies on MAC.
>
>
> > Which is your choice, sure. But why do you need new infrastructure to do it?
> > People have already _done_ this. They're just by nature proprietary and don't
> > like sharing with the group when not forced by lawyers, so they come up with
> > ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
> > ever, for any reason).
> >
> >>>> Mimi suggested to use
> >>>> digital signatures to prevent this issue, but it cannot be used in all
> >>>> scenarios, since conventional systems generate the initial ram disk
> >>>> locally.
> >>>
> >>> So you use a proprietary init binary you can't rebuild from source, and put it
> >>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
> >>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
> >>> clearly keeping the kernel up to date is less important to security...)
> >>
> >> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> >> as the image would be rejected by the boot loader if the signature is
> >> invalid.
> >
> > So you have _more_ assumptions tripping you up. Great. So add a signature in a
> > format your bootloader doesn't recognize, since it's the kernel that should
> > verify it, not your bootloader?
> >
> > It sounds like your problem is bureaucratic, not technical.
>
> The boot loader verifies the CPIO image, when this is possible. The
> kernel verifies individual files when the CPIO image is not signed.
>
> If a remote verifier wants to verify the measurement of the CPIO image,
> and he only has reference digests for each file, he has to build the
> CPIO image with files reference digests were calculated from, and in the
> same way it was done by the system target of the evaluation.
>
>
> >>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> >>> in-band in the file, but you need xattrs for some reason?
> >>
> >> Appending just the signature would be possible. It won't work if you
> >> have multiple metadata for the same file.
> >
> > Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
> > strings? How is this a hard problem?
> >
> >> Also appending the signature alone won't solve the parsing issue. Still,
> >> the kernel has to parse something that could be malformed.
> >
> > Your new in-band signaling file you're making xattrs from could be malformed,
> > one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
> > on for 12 megabytes...
>
> ksys_lsetxattr() checks the limits.
>
> Roberto
>
>
> > Rob
> >
>
> --
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Andy Lutomirski @ 2019-05-14 15:19 UTC (permalink / raw)
To: Roberto Sassu
Cc: Rob Landley, Arvind Sankar, LKML, Linux API, Linux FS Devel,
linux-integrity, initramfs
In-Reply-To: <f7bc547c-61f4-1a17-735c-7e8df97d7965@huawei.com>
On Mon, May 13, 2019 at 5:47 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> On 5/13/2019 11:07 AM, Rob Landley wrote:
> >
> >
> > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> >>>> On 5/12/19 7:52 AM, Mimi Zohar wrote:
> >>>>> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> >>>>>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> >>>>>>> This proposal consists in marshaling pathnames and xattrs in a file called
> >>>>>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> >>>>>>> been extracted.
> >>>>>>
> >>>>>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> >>>>>> be done equivalently by the initramfs' /init? Why is kernel involvement
> >>>>>> actually required here?
> >>>>>
> >>>>> It's too late. The /init itself should be signed and verified.
> >>>>
> >>>> If the initramfs cpio.gz image was signed and verified by the extractor, how is
> >>>> the init in it _not_ verified?
> >>>>
> >>>> Ro
> >>>
> >>> Wouldn't the below work even before enforcing signatures on external
> >>> initramfs:
> >>> 1. Create an embedded initramfs with an /init that does the xattr
> >>> parsing/setting. This will be verified as part of the kernel image
> >>> signature, so no new code required.
> >>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>> prevents overwriting the embedded /init even if the external initramfs
> >>> is unverified.
> >>
> >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >> verify /init in the embedded initial ram disk.
> >
> > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.
>
> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.
>
>
> >> The only reason why
> >> opening .xattr-list works is that IMA is not yet initialized
> >> (late_initcall vs rootfs_initcall).
> >
> > Launching init before enabling ima is bad because... you didn't think of it?
>
> No, because /init can potentially compromise the integrity of the
> system.
I think Rob is right here. If /init was statically built into the
kernel image, it has no more ability to compromise the kernel than
anything else in the kernel. What's the problem here?
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-14 15:12 UTC (permalink / raw)
To: Roberto Sassu, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <de91ef53-6bb3-b937-8773-5f6b34e1acb7@huawei.com>
On 5/14/19 6:52 AM, Roberto Sassu wrote:
> On 5/14/2019 8:06 AM, Rob Landley wrote:
>> On 5/13/19 7:47 AM, Roberto Sassu wrote:
>>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>>> initramfs:
>>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>>> signature, so no new code required.
>>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>>> is unverified.
>>>>>
>>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>>> verify /init in the embedded initial ram disk.
>>>>
>>>> So you made broken infrastructure that's causing you problems. Sounds
>>>> unfortunate.
>>>
>>> The idea is to be able to verify anything that is accessed, as soon as
>>> rootfs is available, without distinction between embedded or external
>>> initial ram disk.
>>
>> If /init is in the internal one and you can't overwrite files with an external
>> one, all your init has to be is something that applies the xattrs, enables your
>> paranoia mode, and then execs something else.
>
> Shouldn't file metadata be handled by the same code that extracts the
> content? Instead, file content is extracted by the kernel, and we are
> adding another step to the boot process, to execute a new binary with a
> link to libc.
I haven't made a dynamically linked initramfs in years (except a couple for
testing purposes). But then I don't deploy glibc, so...
> From the perspective of a remote verifier that checks the software
> running on the system, would it be easier to check less than 150 lines
> of code, or a CPIO image containing a binary + libc?
https://github.com/torvalds/linux/blob/master/tools/include/nolibc/nolibc.h
(I have a todo item to add sh4 and m68k and ppc and such sections to that, but
see "I've needed to resubmit
http://lkml.iu.edu/hypermail/linux/kernel/1709.1/03561.html for a couple years
now but it works for me locally and dealing with linux-kernel is just no fun
anymore"...)
>> You can totally use initramfs for lots of purposes simultaneously.
>
> Yes, I agree. However, adding an initramfs to initialize another
> initramfs when you can simply extract file content and metadata with the
> same parser, this for me it is difficult to justify.
You just said it's simpler to modify the kernel than do a thing you can already
do in userspace. You realize that, right?
>>>>> The only reason why
>>>>> opening .xattr-list works is that IMA is not yet initialized
>>>>> (late_initcall vs rootfs_initcall).
>>>>
>>>> Launching init before enabling ima is bad because... you didn't think of it?
>>>
>>> No, because /init can potentially compromise the integrity of the
>>> system.
>>
>> Which isn't a problem if it was statically linked in the kernel, or if your
>> external cpio.gz was signed. You want a signed binary but don't want the
>> signature _in_ the binary...
>
> It is not just for binaries. How you would deal with arbitrary file
> formats?
I'm confused, are you saying that /init can/should be an arbitrary file format,
or that a cpio statically linked into the kernel can't contain files in
arbitrary formats?
>> Which is why there's a cpio in the kernel and an external cpio loaded via the
>> old initrd mechanism and BOTH files wind up in the cpio and there's a way to
>> make it O_EXCL so it can't overwrite, and then the /init binary inside the
>> kernel's cpio can do any other weird verification you need to do before anything
>> else gets a chance to run so why are you having ring 0 kernel code read a file
>> out of the filesystem and act upon it?
>
> The CPIO parser already invokes many system calls.
The one in the kernel doesn't call system calls, no. Once userspace is running
it can do what it likes. The one statically linked into the kernel was set up by
the same people who built the kernel; if you're letting arbitrary kernels run on
your system it's kinda over already from a security context?
>> If it's in the file's contents you get uniform behavior regardless of the
>> filesystem used. And "mandatory access controls do that" is basically restating
>> what _I_ said in the paragraph above.
>
> As I said, that does not work with arbitrary file formats.
an /init binary can parse your .inbandsignalling file to apply xattrs to
arbitrary files before handing off to something else, and a cpio.gz statically
linked into the kernel can contain arbitrary files.
>> The "infrastructure you have that works a certain way" is called "mandatory
>> access controls". Good to know. Your patch changes the rest of the system to
>> match the assumptions of the new code, because changing those assumptions
>> appears literally unthinkable.
>
> All I want to do is to have the same behavior as if there is no initial
> ram disk. And given that inode-based MACs read the labels from xattrs,
> the assumption that the system provides xattrs even in the inital ram
> disk seems reasonable.
There was a previous proposal for a new revision of cpio that I don't remember
particularly objecting to? Which did things that can't trivially be done in
userspace?
>>> What do you mean exactly?
>>
>> That you're not remotely the first person to do this?
>>
>> You're attempting to prevent anyone from running third party code on your system
>> without buying a license from you first. You're creating a system with no user
>> serviceable parts, that only runs authorized software from the Apple Store or
>> other walled garden. No sideloading allowed.
>
> This is one use case. The main purpose of IMA is to preserve the
> integrity of the Trusted Computing Base (TCB, the critical part of the
> system), or to detect integrity violations without enforcement. This is
> done by ensuring that the software comes from the vendor. Applications
> owned by users are allowed to run, as the Discrectionary Access Control
> (DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
> that relies on MAC.
Sure, same general idea as Apple's lobbying against "right to repair".
https://appleinsider.com/articles/19/03/18/california-reintroduces-right-to-repair-bill-after-previous-effort-failed
The vendor can prevent any unauthorized software from running on the device, or
even retroactively remove older software to force upgrades:
https://www.macrumors.com/2019/05/13/adobe-creative-cloud-legal-action-older-apps/
Or anything else, of course:
https://www.zdnet.com/article/why-amazon-is-within-its-rights-to-remove-access-to-your-kindle-books/
*shrug* Your choice, of course...
>> So you have _more_ assumptions tripping you up. Great. So add a signature in a
>> format your bootloader doesn't recognize, since it's the kernel that should
>> verify it, not your bootloader?
>>
>> It sounds like your problem is bureaucratic, not technical.
>
> The boot loader verifies the CPIO image, when this is possible. The
> kernel verifies individual files when the CPIO image is not signed.
>
> If a remote verifier wants to verify the measurement of the CPIO image,
> and he only has reference digests for each file, he has to build the
> CPIO image with files reference digests were calculated from, and in the
> same way it was done by the system target of the evaluation.
And your init program can parse your .inbandsignaling file to put the xattrs on
the files and then poke the "now enforce" button.
>>>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>>>> in-band in the file, but you need xattrs for some reason?
>>>
>>> Appending just the signature would be possible. It won't work if you
>>> have multiple metadata for the same file.
>>
>> Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
>> strings? How is this a hard problem?
>>
>>> Also appending the signature alone won't solve the parsing issue. Still,
>>> the kernel has to parse something that could be malformed.
>>
>> Your new in-band signaling file you're making xattrs from could be malformed,
>> one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
>> on for 12 megabytes...
>
> ksys_lsetxattr() checks the limits.
Not if it caused an oom error extracting your .inbandsignaling file before it
got consumed. (The kernel has to parse something that could be malformed and
that's bad reading ELF information like linux has done loading binaries since
1996, but it's ok for xattrs with a system call?)
*shrug* I've made my opinion clear and don't think this thread is useful at this
point, I'm not the maintainer with merge authority, so I'm gonna mute it now.
Rob
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] mm/ksm: add option to automerge VMAs
From: Michal Hocko @ 2019-05-14 14:51 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: linux-kernel, Kirill Tkhai, Vlastimil Babka, Matthew Wilcox,
Pavel Tatashin, Timofey Titovets, Aaron Tomlin, Grzegorz Halat,
linux-mm, linux-api, Hugh Dickins
In-Reply-To: <20190514144105.GF4683@dhcp22.suse.cz>
[Forgot Hugh]
On Tue 14-05-19 16:41:05, Michal Hocko wrote:
> [This is adding a new user visible interface so you should be CCing
> linux-api mailing list. Also CC Hugh for KSM in general. Done now]
>
> On Tue 14-05-19 15:16:50, Oleksandr Natalenko wrote:
> > By default, KSM works only on memory that is marked by madvise(). And the
> > only way to get around that is to either:
> >
> > * use LD_PRELOAD; or
> > * patch the kernel with something like UKSM or PKSM.
> >
> > Instead, lets implement a sysfs knob, which allows marking VMAs as
> > mergeable. This can be used manually on some task in question or by some
> > small userspace helper daemon.
> >
> > The knob is named "force_madvise", and it is write-only. It accepts a PID
> > to act on. To mark the VMAs as mergeable, use:
> >
> > # echo PID > /sys/kernel/mm/ksm/force_madvise
> >
> > To unmerge all the VMAs, use the same approach, prepending the PID with
> > the "minus" sign:
> >
> > # echo -PID > /sys/kernel/mm/ksm/force_madvise
> >
> > This patchset is based on earlier Timofey's submission [1], but it doesn't
> > use dedicated kthread to walk through the list of tasks/VMAs. Instead,
> > it is up to userspace to traverse all the tasks in /proc if needed.
> >
> > The previous suggestion [2] was based on amending do_anonymous_page()
> > handler to implement fully automatic mode, but this approach was
> > incorrect due to improper locking and not desired due to excessive
> > complexity.
> >
> > The current approach just implements minimal interface and leaves the
> > decision on how and when to act to userspace.
>
> Please make sure to describe a usecase that warrants adding a new
> interface we have to maintain for ever.
>
> >
> > Thanks.
> >
> > [1] https://lore.kernel.org/patchwork/patch/1012142/
> > [2] http://lkml.iu.edu/hypermail/linux/kernel/1905.1/02417.html
> >
> > Oleksandr Natalenko (4):
> > mm/ksm: introduce ksm_enter() helper
> > mm/ksm: introduce ksm_leave() helper
> > mm/ksm: introduce force_madvise knob
> > mm/ksm: add force merging/unmerging documentation
> >
> > Documentation/admin-guide/mm/ksm.rst | 11 ++
> > mm/ksm.c | 160 +++++++++++++++++++++------
> > 2 files changed, 137 insertions(+), 34 deletions(-)
> >
> > --
> > 2.21.0
> >
>
> --
> Michal Hocko
> SUSE Labs
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-14 14:44 UTC (permalink / raw)
To: Rob Landley
Cc: Mimi Zohar, Arvind Sankar, Arvind Sankar, Roberto Sassu,
linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <66b57ae5-bb5a-c008-8490-2c90e050fc65@landley.net>
On Tue, May 14, 2019 at 01:06:45AM -0500, Rob Landley wrote:
> On 5/13/19 5:09 PM, Mimi Zohar wrote:
> >> Ok, but wouldn't my idea still work? Leave the default compiled-in
> >> policy set to not appraise initramfs. The embedded /init sets all the
> >> xattrs, changes the policy to appraise tmpfs, and then exec's the real
> >> init? Then everything except the embedded /init and the file with the
> >> xattrs will be appraised, and the embedded /init was verified as part of
> >> the kernel image signature. The only additional kernel change needed
> >> then is to add a config option to the kernel to disallow overwriting the
> >> embedded initramfs (or at least the embedded /init).
> >
> > Yes and no. The current IMA design allows a builtin policy to be
> > specified on the boot command line ("ima_policy="), so that it exists
> > from boot, and allows it to be replaced once with a custom policy.
> > After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
> > additional rules may be appended. As your embedded /init solution
> > already replaces the builtin policy, the IMA policy couldn't currently
> > be replaced a second time with a custom policy based on LSM labels.
>
> So your design assumption you're changing other code to work around in that
> instance is the policy can only be replaced once rather than having a "finalize"
> option when it's set, making it immutable from then on.
>
> Rob
I agree it would be better to have a finalize option. Outside of my
idea, it seems the current setup would make it so while developing an
IMA policy you need to keep rebooting to test your changes?
I'd suggest having a knob that starts out unrestricted, and can be
one-way changed to append-only or immutable. This seems like a good idea
even if you decide the embedded image is too much trouble or unworkable
for other reasons.
^ permalink raw reply
* Re: [PATCH RFC v2 0/4] mm/ksm: add option to automerge VMAs
From: Michal Hocko @ 2019-05-14 14:41 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: linux-kernel, Kirill Tkhai, Vlastimil Babka, Matthew Wilcox,
Pavel Tatashin, Timofey Titovets, Aaron Tomlin, Grzegorz Halat,
linux-mm, linux-api
In-Reply-To: <20190514131654.25463-1-oleksandr@redhat.com>
[This is adding a new user visible interface so you should be CCing
linux-api mailing list. Also CC Hugh for KSM in general. Done now]
On Tue 14-05-19 15:16:50, Oleksandr Natalenko wrote:
> By default, KSM works only on memory that is marked by madvise(). And the
> only way to get around that is to either:
>
> * use LD_PRELOAD; or
> * patch the kernel with something like UKSM or PKSM.
>
> Instead, lets implement a sysfs knob, which allows marking VMAs as
> mergeable. This can be used manually on some task in question or by some
> small userspace helper daemon.
>
> The knob is named "force_madvise", and it is write-only. It accepts a PID
> to act on. To mark the VMAs as mergeable, use:
>
> # echo PID > /sys/kernel/mm/ksm/force_madvise
>
> To unmerge all the VMAs, use the same approach, prepending the PID with
> the "minus" sign:
>
> # echo -PID > /sys/kernel/mm/ksm/force_madvise
>
> This patchset is based on earlier Timofey's submission [1], but it doesn't
> use dedicated kthread to walk through the list of tasks/VMAs. Instead,
> it is up to userspace to traverse all the tasks in /proc if needed.
>
> The previous suggestion [2] was based on amending do_anonymous_page()
> handler to implement fully automatic mode, but this approach was
> incorrect due to improper locking and not desired due to excessive
> complexity.
>
> The current approach just implements minimal interface and leaves the
> decision on how and when to act to userspace.
Please make sure to describe a usecase that warrants adding a new
interface we have to maintain for ever.
>
> Thanks.
>
> [1] https://lore.kernel.org/patchwork/patch/1012142/
> [2] http://lkml.iu.edu/hypermail/linux/kernel/1905.1/02417.html
>
> Oleksandr Natalenko (4):
> mm/ksm: introduce ksm_enter() helper
> mm/ksm: introduce ksm_leave() helper
> mm/ksm: introduce force_madvise knob
> mm/ksm: add force merging/unmerging documentation
>
> Documentation/admin-guide/mm/ksm.rst | 11 ++
> mm/ksm.c | 160 +++++++++++++++++++++------
> 2 files changed, 137 insertions(+), 34 deletions(-)
>
> --
> 2.21.0
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-14 11:52 UTC (permalink / raw)
To: Rob Landley, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <49965ffd-dd57-ffe5-4a2f-73cdfb387848@landley.net>
On 5/14/2019 8:06 AM, Rob Landley wrote:
> On 5/13/19 7:47 AM, Roberto Sassu wrote:
>> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>>> Wouldn't the below work even before enforcing signatures on external
>>>>> initramfs:
>>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>>> parsing/setting. This will be verified as part of the kernel image
>>>>> signature, so no new code required.
>>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>>> prevents overwriting the embedded /init even if the external initramfs
>>>>> is unverified.
>>>>
>>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>>> verify /init in the embedded initial ram disk.
>>>
>>> So you made broken infrastructure that's causing you problems. Sounds
>>> unfortunate.
>>
>> The idea is to be able to verify anything that is accessed, as soon as
>> rootfs is available, without distinction between embedded or external
>> initial ram disk.
>
> If /init is in the internal one and you can't overwrite files with an external
> one, all your init has to be is something that applies the xattrs, enables your
> paranoia mode, and then execs something else.
Shouldn't file metadata be handled by the same code that extracts the
content? Instead, file content is extracted by the kernel, and we are
adding another step to the boot process, to execute a new binary with a
link to libc.
From the perspective of a remote verifier that checks the software
running on the system, would it be easier to check less than 150 lines
of code, or a CPIO image containing a binary + libc?
> Heck, I do that sort of set up in shell scripts all the time. Running the shell
> script as PID 1 and then having it exec the "real init" binary at the end:
>
> https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205
>
> If your first init binary is in the initramfs statically linked into the kernel
> image, and the cpio code is doing open(O_EXCL), then it's as verified as any
> other kernel code and runs "securely" until it decides to run something else.
>
>> Also, requiring an embedded initramfs for xattrs would be an issue for
>> systems that use it for other purposes.
>
> I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
> that will probably never go upstream because I'm a hobbyist and dealing with the
> linux-kernel clique is the opposite of fun. I'm only in this conversation
> because I was cc'd.)
>
> You can totally use initramfs for lots of purposes simultaneously.
Yes, I agree. However, adding an initramfs to initialize another
initramfs when you can simply extract file content and metadata with the
same parser, this for me it is difficult to justify.
>>>> The only reason why
>>>> opening .xattr-list works is that IMA is not yet initialized
>>>> (late_initcall vs rootfs_initcall).
>>>
>>> Launching init before enabling ima is bad because... you didn't think of it?
>>
>> No, because /init can potentially compromise the integrity of the
>> system.
>
> Which isn't a problem if it was statically linked in the kernel, or if your
> external cpio.gz was signed. You want a signed binary but don't want the
> signature _in_ the binary...
It is not just for binaries. How you would deal with arbitrary file
formats?
>>>> Allowing a kernel with integrity enforcement to parse the CPIO image
>>>> without verifying it first is the weak point.
>>>
>>> If you don't verify the CPIO image then in theory it could have anything in it,
>>> yes. You seem to believe that signing individual files is more secure than
>>> signing the archive. This is certainly a point of view.
>>
>> As I wrote above, signing the CPIO image would be more secure, if this
>> option is available. However, a disadvantage would be that you have to
>> sign the CPIO image every time a file changes.
>
> Which is why there's a cpio in the kernel and an external cpio loaded via the
> old initrd mechanism and BOTH files wind up in the cpio and there's a way to
> make it O_EXCL so it can't overwrite, and then the /init binary inside the
> kernel's cpio can do any other weird verification you need to do before anything
> else gets a chance to run so why are you having ring 0 kernel code read a file
> out of the filesystem and act upon it?
The CPIO parser already invokes many system calls.
> (Heck, you can mv /newinit /init before the exec /init so the file isn't on the
> system anymore by the time the other stuff gets to run...)
>
>>>> However, extracted files
>>>> are not used, and before they are used they are verified. At the time
>>>> they are verified, they (included /init) must already have a signature
>>>> or otherwise access would be denied.
>>>
>>> You build infrastructure that works a certain way, the rest of the system
>>> doesn't fit your assumptions, so you need to change the rest of the system to
>>> fit your assumptions.
>>
>> Requiring file metadata to make decisions seems reasonable. Also
>> mandatory access controls do that. The objective of this patch set is to
>> have uniform behavior regardless of the filesystem used.
>
> If it's in the file's contents you get uniform behavior regardless of the
> filesystem used. And "mandatory access controls do that" is basically restating
> what _I_ said in the paragraph above.
As I said, that does not work with arbitrary file formats.
> The "infrastructure you have that works a certain way" is called "mandatory
> access controls". Good to know. Your patch changes the rest of the system to
> match the assumptions of the new code, because changing those assumptions
> appears literally unthinkable.
All I want to do is to have the same behavior as if there is no initial
ram disk. And given that inode-based MACs read the labels from xattrs,
the assumption that the system provides xattrs even in the inital ram
disk seems reasonable.
>>>> This scheme relies on the ability of the kernel to not be corrupted in
>>>> the event it parses a malformed CPIO image.
>>>
>>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
>>> extraction code. You can fill up all physical memory with initramfs and lock the
>>> system hard, though.
>>>
>>> It still only parses them at boot time before launching PID 1, right? So you
>>> have a local physical exploit and you're trying to prevent people from working
>>> around your Xbox copy protection without a mod chip?
>>
>> What do you mean exactly?
>
> That you're not remotely the first person to do this?
>
> You're attempting to prevent anyone from running third party code on your system
> without buying a license from you first. You're creating a system with no user
> serviceable parts, that only runs authorized software from the Apple Store or
> other walled garden. No sideloading allowed.
This is one use case. The main purpose of IMA is to preserve the
integrity of the Trusted Computing Base (TCB, the critical part of the
system), or to detect integrity violations without enforcement. This is
done by ensuring that the software comes from the vendor. Applications
owned by users are allowed to run, as the Discrectionary Access Control
(DAC) prevents attacks to the TCB. I'm working on a more advanced scheme
that relies on MAC.
> Which is your choice, sure. But why do you need new infrastructure to do it?
> People have already _done_ this. They're just by nature proprietary and don't
> like sharing with the group when not forced by lawyers, so they come up with
> ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
> ever, for any reason).
>
>>>> Mimi suggested to use
>>>> digital signatures to prevent this issue, but it cannot be used in all
>>>> scenarios, since conventional systems generate the initial ram disk
>>>> locally.
>>>
>>> So you use a proprietary init binary you can't rebuild from source, and put it
>>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
>>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
>>> clearly keeping the kernel up to date is less important to security...)
>>
>> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
>> as the image would be rejected by the boot loader if the signature is
>> invalid.
>
> So you have _more_ assumptions tripping you up. Great. So add a signature in a
> format your bootloader doesn't recognize, since it's the kernel that should
> verify it, not your bootloader?
>
> It sounds like your problem is bureaucratic, not technical.
The boot loader verifies the CPIO image, when this is possible. The
kernel verifies individual files when the CPIO image is not signed.
If a remote verifier wants to verify the measurement of the CPIO image,
and he only has reference digests for each file, he has to build the
CPIO image with files reference digests were calculated from, and in the
same way it was done by the system target of the evaluation.
>>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>>> in-band in the file, but you need xattrs for some reason?
>>
>> Appending just the signature would be possible. It won't work if you
>> have multiple metadata for the same file.
>
> Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
> strings? How is this a hard problem?
>
>> Also appending the signature alone won't solve the parsing issue. Still,
>> the kernel has to parse something that could be malformed.
>
> Your new in-band signaling file you're making xattrs from could be malformed,
> one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
> on for 12 megabytes...
ksys_lsetxattr() checks the limits.
Roberto
> Rob
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [LTP] [EXT] Re: [PATCH v9 00/24] ILP32 for ARM64
From: Cyril Hrubis @ 2019-05-14 10:43 UTC (permalink / raw)
To: Yuri Norov
Cc: Andreas Schwab, Yury Norov, ltp@lists.linux.it,
linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, Prasun Kapoor,
Alexander Graf, Bamvor Zhangjian, Geert Uytterhoeven, Dave Martin,
Adam Borowski, Manuel Montezelo
In-Reply-To: <MN2PR18MB30865B950D85C6463EB0E1D4CB0F0@MN2PR18MB3086.namprd18.prod.outlook.com>
Hi!
> > There is a problem with the stack size accounting during execve when
> > there is no stack limit:
> >
> > $ ulimit -s
> > 8192
> > $ ./hello.ilp32
> > Hello World!
> > $ ulimit -s unlimited
> > $ ./hello.ilp32
> > Segmentation fault
> > $ strace ./hello.ilp32
> > execve("./hello.ilp32", ["./hello.ilp32"], 0xfffff10548f0 /* 77 vars */) = -1 ENOMEM (Cannot allocate memory)
> > +++ killed by SIGSEGV +++
> > Segmentation fault (core dumped)
> >
> > Andreas.
>
> Thanks Andreas, I will take a look. Do we have such test in LTP?
We do have a test that we can run a binary with very small stack size
i.e. 512kB but there does not seem to be anything that would catch this
specific problem.
Can you please open an issue and describe how to reproduce the problem
at our github tracker:
https://github.com/linux-test-project/ltp/issues
Then we can create testcase based on that reproducer later on.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-14 6:06 UTC (permalink / raw)
To: Mimi Zohar, Arvind Sankar
Cc: Arvind Sankar, Roberto Sassu, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <1557785351.4969.94.camel@linux.ibm.com>
On 5/13/19 5:09 PM, Mimi Zohar wrote:
>> Ok, but wouldn't my idea still work? Leave the default compiled-in
>> policy set to not appraise initramfs. The embedded /init sets all the
>> xattrs, changes the policy to appraise tmpfs, and then exec's the real
>> init? Then everything except the embedded /init and the file with the
>> xattrs will be appraised, and the embedded /init was verified as part of
>> the kernel image signature. The only additional kernel change needed
>> then is to add a config option to the kernel to disallow overwriting the
>> embedded initramfs (or at least the embedded /init).
>
> Yes and no. The current IMA design allows a builtin policy to be
> specified on the boot command line ("ima_policy="), so that it exists
> from boot, and allows it to be replaced once with a custom policy.
> After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
> additional rules may be appended. As your embedded /init solution
> already replaces the builtin policy, the IMA policy couldn't currently
> be replaced a second time with a custom policy based on LSM labels.
So your design assumption you're changing other code to work around in that
instance is the policy can only be replaced once rather than having a "finalize"
option when it's set, making it immutable from then on.
Rob
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-14 6:06 UTC (permalink / raw)
To: Roberto Sassu, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <f7bc547c-61f4-1a17-735c-7e8df97d7965@huawei.com>
On 5/13/19 7:47 AM, Roberto Sassu wrote:
> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>> Wouldn't the below work even before enforcing signatures on external
>>>> initramfs:
>>>> 1. Create an embedded initramfs with an /init that does the xattr
>>>> parsing/setting. This will be verified as part of the kernel image
>>>> signature, so no new code required.
>>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>>> prevents overwriting the embedded /init even if the external initramfs
>>>> is unverified.
>>>
>>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>>> verify /init in the embedded initial ram disk.
>>
>> So you made broken infrastructure that's causing you problems. Sounds
>> unfortunate.
>
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.
If /init is in the internal one and you can't overwrite files with an external
one, all your init has to be is something that applies the xattrs, enables your
paranoia mode, and then execs something else.
Heck, I do that sort of set up in shell scripts all the time. Running the shell
script as PID 1 and then having it exec the "real init" binary at the end:
https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205
If your first init binary is in the initramfs statically linked into the kernel
image, and the cpio code is doing open(O_EXCL), then it's as verified as any
other kernel code and runs "securely" until it decides to run something else.
> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.
I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
that will probably never go upstream because I'm a hobbyist and dealing with the
linux-kernel clique is the opposite of fun. I'm only in this conversation
because I was cc'd.)
You can totally use initramfs for lots of purposes simultaneously.
>>> The only reason why
>>> opening .xattr-list works is that IMA is not yet initialized
>>> (late_initcall vs rootfs_initcall).
>>
>> Launching init before enabling ima is bad because... you didn't think of it?
>
> No, because /init can potentially compromise the integrity of the
> system.
Which isn't a problem if it was statically linked in the kernel, or if your
external cpio.gz was signed. You want a signed binary but don't want the
signature _in_ the binary...
>>> Allowing a kernel with integrity enforcement to parse the CPIO image
>>> without verifying it first is the weak point.
>>
>> If you don't verify the CPIO image then in theory it could have anything in it,
>> yes. You seem to believe that signing individual files is more secure than
>> signing the archive. This is certainly a point of view.
>
> As I wrote above, signing the CPIO image would be more secure, if this
> option is available. However, a disadvantage would be that you have to
> sign the CPIO image every time a file changes.
Which is why there's a cpio in the kernel and an external cpio loaded via the
old initrd mechanism and BOTH files wind up in the cpio and there's a way to
make it O_EXCL so it can't overwrite, and then the /init binary inside the
kernel's cpio can do any other weird verification you need to do before anything
else gets a chance to run so why are you having ring 0 kernel code read a file
out of the filesystem and act upon it?
(Heck, you can mv /newinit /init before the exec /init so the file isn't on the
system anymore by the time the other stuff gets to run...)
>>> However, extracted files
>>> are not used, and before they are used they are verified. At the time
>>> they are verified, they (included /init) must already have a signature
>>> or otherwise access would be denied.
>>
>> You build infrastructure that works a certain way, the rest of the system
>> doesn't fit your assumptions, so you need to change the rest of the system to
>> fit your assumptions.
>
> Requiring file metadata to make decisions seems reasonable. Also
> mandatory access controls do that. The objective of this patch set is to
> have uniform behavior regardless of the filesystem used.
If it's in the file's contents you get uniform behavior regardless of the
filesystem used. And "mandatory access controls do that" is basically restating
what _I_ said in the paragraph above.
The "infrastructure you have that works a certain way" is called "mandatory
access controls". Good to know. Your patch changes the rest of the system to
match the assumptions of the new code, because changing those assumptions
appears literally unthinkable.
>>> This scheme relies on the ability of the kernel to not be corrupted in
>>> the event it parses a malformed CPIO image.
>>
>> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
>> extraction code. You can fill up all physical memory with initramfs and lock the
>> system hard, though.
>>
>> It still only parses them at boot time before launching PID 1, right? So you
>> have a local physical exploit and you're trying to prevent people from working
>> around your Xbox copy protection without a mod chip?
>
> What do you mean exactly?
That you're not remotely the first person to do this?
You're attempting to prevent anyone from running third party code on your system
without buying a license from you first. You're creating a system with no user
serviceable parts, that only runs authorized software from the Apple Store or
other walled garden. No sideloading allowed.
Which is your choice, sure. But why do you need new infrastructure to do it?
People have already _done_ this. They're just by nature proprietary and don't
like sharing with the group when not forced by lawyers, so they come up with
ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
ever, for any reason).
>>> Mimi suggested to use
>>> digital signatures to prevent this issue, but it cannot be used in all
>>> scenarios, since conventional systems generate the initial ram disk
>>> locally.
>>
>> So you use a proprietary init binary you can't rebuild from source, and put it
>> in a cpio where /dev/urandom is a file with known contents? Clearly, not
>> exploitable at all. (And we update the initramfs.cpio but not the kernel because
>> clearly keeping the kernel up to date is less important to security...)
>
> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> as the image would be rejected by the boot loader if the signature is
> invalid.
So you have _more_ assumptions tripping you up. Great. So add a signature in a
format your bootloader doesn't recognize, since it's the kernel that should
verify it, not your bootloader?
It sounds like your problem is bureaucratic, not technical.
>> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
>> in-band in the file, but you need xattrs for some reason?
>
> Appending just the signature would be possible. It won't work if you
> have multiple metadata for the same file.
Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
strings? How is this a hard problem?
> Also appending the signature alone won't solve the parsing issue. Still,
> the kernel has to parse something that could be malformed.
Your new in-band signaling file you're making xattrs from could be malformed,
one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
on for 12 megabytes...
Rob
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-13 22:09 UTC (permalink / raw)
To: Arvind Sankar
Cc: Arvind Sankar, Roberto Sassu, Rob Landley, linux-kernel,
linux-api, linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190513184744.GA12386@rani.riverdale.lan>
On Mon, 2019-05-13 at 14:47 -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:36:24PM -0400, Mimi Zohar wrote:
> >
> > > > How does this work today then? Is it actually the case that initramfs
> > > > just cannot be used on an IMA-enabled system, or it can but it leaves
> > > > the initramfs unverified and we're trying to fix that? I had assumed the
> > > > latter.
> > > Oooh, it's done not by starting IMA appraisal later, but by loading a
> > > default policy to ignore initramfs?
> >
> > Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
> > for finer grained policies to be defined. This patch set would allow
> > a builtin IMA appraise policy to be defined which includes tmpfs.
Clarification: finer grain IMA policy rules are normally defined in
terms of LSM labels. The LSMs need to enabled, before writing IMA
policy rules in terms of the LSM labels.
> >
> Ok, but wouldn't my idea still work? Leave the default compiled-in
> policy set to not appraise initramfs. The embedded /init sets all the
> xattrs, changes the policy to appraise tmpfs, and then exec's the real
> init? Then everything except the embedded /init and the file with the
> xattrs will be appraised, and the embedded /init was verified as part of
> the kernel image signature. The only additional kernel change needed
> then is to add a config option to the kernel to disallow overwriting the
> embedded initramfs (or at least the embedded /init).
Yes and no. The current IMA design allows a builtin policy to be
specified on the boot command line ("ima_policy="), so that it exists
from boot, and allows it to be replaced once with a custom policy.
After that, assuming that CONFIG_IMA_WRITE_POLICY is configured,
additional rules may be appended. As your embedded /init solution
already replaces the builtin policy, the IMA policy couldn't currently
be replaced a second time with a custom policy based on LSM labels.
Mimi
^ permalink raw reply
* Re: [EXT] Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yuri Norov @ 2019-05-13 20:16 UTC (permalink / raw)
To: Andreas Schwab, Yury Norov, ltp@lists.linux.it
Cc: linux-doc@vger.kernel.org, Szabolcs Nagy, Catalin Marinas,
Heiko Carstens, Yury Norov, Philipp Tomsich, Joseph Myers,
linux-arch@vger.kernel.org, Steve Ellcey, Prasun Kapoor,
Alexander Graf, Bamvor Zhangjian, Geert Uytterhoeven, Dave Martin,
Adam Borowski, Manuel Montezelo, James Hogan, Chris Metcalf,
Arnd Bergmann, Andrew Pinski, Lin Yongting
In-Reply-To: <mvmtvdyoi33.fsf@suse.de>
+ ltp@lists.linux.it
> There is a problem with the stack size accounting during execve when
> there is no stack limit:
>
> $ ulimit -s
> 8192
> $ ./hello.ilp32
> Hello World!
> $ ulimit -s unlimited
> $ ./hello.ilp32
> Segmentation fault
> $ strace ./hello.ilp32
> execve("./hello.ilp32", ["./hello.ilp32"], 0xfffff10548f0 /* 77 vars */) = -1 ENOMEM (Cannot allocate memory)
> +++ killed by SIGSEGV +++
> Segmentation fault (core dumped)
>
> Andreas.
Thanks Andreas, I will take a look. Do we have such test in LTP?
Yury
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-13 18:47 UTC (permalink / raw)
To: Mimi Zohar
Cc: Arvind Sankar, Roberto Sassu, Rob Landley, linux-kernel,
linux-api, linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <1557772584.4969.62.camel@linux.ibm.com>
On Mon, May 13, 2019 at 02:36:24PM -0400, Mimi Zohar wrote:
>
> > > How does this work today then? Is it actually the case that initramfs
> > > just cannot be used on an IMA-enabled system, or it can but it leaves
> > > the initramfs unverified and we're trying to fix that? I had assumed the
> > > latter.
> > Oooh, it's done not by starting IMA appraisal later, but by loading a
> > default policy to ignore initramfs?
>
> Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
> for finer grained policies to be defined. This patch set would allow
> a builtin IMA appraise policy to be defined which includes tmpfs.
>
> Mimi
>
Ok, but wouldn't my idea still work? Leave the default compiled-in
policy set to not appraise initramfs. The embedded /init sets all the
xattrs, changes the policy to appraise tmpfs, and then exec's the real
init? Then everything except the embedded /init and the file with the
xattrs will be appraised, and the embedded /init was verified as part of
the kernel image signature. The only additional kernel change needed
then is to add a config option to the kernel to disallow overwriting the
embedded initramfs (or at least the embedded /init).
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-13 18:36 UTC (permalink / raw)
To: Arvind Sankar
Cc: Roberto Sassu, Rob Landley, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190513175250.GC69717@rani.riverdale.lan>
> > How does this work today then? Is it actually the case that initramfs
> > just cannot be used on an IMA-enabled system, or it can but it leaves
> > the initramfs unverified and we're trying to fix that? I had assumed the
> > latter.
> Oooh, it's done not by starting IMA appraisal later, but by loading a
> default policy to ignore initramfs?
Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
for finer grained policies to be defined. This patch set would allow
a builtin IMA appraise policy to be defined which includes tmpfs.
Mimi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox