Re: secmodel_securelevel(9) and machdep.svs.enabled
26.04.2018, 13:03, "Thor Lancelot Simon" :If that is true it's a serious regression. Are you talking about thepathological case where "options INSECURE" is set?ThorMy reasoning was based entirely upon reading secmodel_securelevel(9) and kmem(4) man pages. I don’t know whether it accurately reflects the reality.Alex
Re: secmodel_securelevel(9) and machdep.svs.enabled
On Wed, Apr 25, 2018 at 09:43:18PM +0100, Alexander Nasonov wrote: > > Thinking a bit more about this, I don't think my patch will prevent > data leakage from the kernel because /dev/mem and /dev/kmem are > readable at all securelevels. It can only prevent leakage in some If that is true it's a serious regression. Are you talking about the pathological case where "options INSECURE" is set? Thor
Re: secmodel_securelevel(9) and machdep.svs.enabled
Alexander Nasonov wrote: > Thinking a bit more about this, I don't think my patch will prevent > data leakage from the kernel because /dev/mem and /dev/kmem are > readable at all securelevels. There is an important distrinction, though. Code in sys/dev/mm.c can be changed to scramble sensitive pages (e.g. cgd(4) keys) while meltdown is a wild beast and it's nearly impossible to control. -- Alex
Re: secmodel_securelevel(9) and machdep.svs.enabled
Maxime Villard wrote: > Yes, it's fine. I've never taken care of securelevel, but your change > can't be incorrect. Perhaps I would use just KAUTH_MACHDEP_SVS instead > of KAUTH_MACHDEP_SVS_DISABLE, in case another operation gets added in > the future, but that doesn't matter. I don't think securelevel should care about details of SVS. If you want to introduce levels of SVS, KAUTH_MACHDEP_SVS_DISABLE can still be used to prevent lowering (instead of disabling SVS completely). Perhaps the name can be changed to KAUTH_MACHDEP_SVS_DEGRADE or something similar but it's not that important. Thinking a bit more about this, I don't think my patch will prevent data leakage from the kernel because /dev/mem and /dev/kmem are readable at all securelevels. It can only prevent leakage in some situations. For example, when root is compromised inside chroot and chroot directory is on a file system mounted with nodev. -- Alex
re: secmodel_securelevel(9) and machdep.svs.enabled
Maxime Villard writes: > Le 25/04/2018 à 19:47, Alexander Nasonov a écrit : > > Alexander Nasonov wrote: > >> Alexander Nasonov wrote: > >>> When securelevel is set, should be lock 1->0 change for > >>> machdep.svs.enabled (and possibly for other sysctls related > >>> to recent security mitigations)? > >> > >> Can I commit the attached patch? (doc update will follow) > > > > If I don't hear any objections, I will commit the patch soon and > > I will request a pullup to netbsd-8. it's the right idea to me. > > Alex > > Yes, it's fine. I've never taken care of securelevel, but your change > can't be incorrect. Perhaps I would use just KAUTH_MACHDEP_SVS instead > of KAUTH_MACHDEP_SVS_DISABLE, in case another operation gets added in > the future, but that doesn't matter. i considered this idea -- plain SVS would have to not include ENABLE, which doesn't seem right. perhaps another generic name that implies !enable would work. .mrg.
Re: secmodel_securelevel(9) and machdep.svs.enabled
Le 25/04/2018 à 19:47, Alexander Nasonov a écrit : Alexander Nasonov wrote: Alexander Nasonov wrote: When securelevel is set, should be lock 1->0 change for machdep.svs.enabled (and possibly for other sysctls related to recent security mitigations)? Can I commit the attached patch? (doc update will follow) If I don't hear any objections, I will commit the patch soon and I will request a pullup to netbsd-8. Alex Yes, it's fine. I've never taken care of securelevel, but your change can't be incorrect. Perhaps I would use just KAUTH_MACHDEP_SVS instead of KAUTH_MACHDEP_SVS_DISABLE, in case another operation gets added in the future, but that doesn't matter. Maxime
Re: secmodel_securelevel(9) and machdep.svs.enabled
Alexander Nasonov wrote: > Alexander Nasonov wrote: > > When securelevel is set, should be lock 1->0 change for > > machdep.svs.enabled (and possibly for other sysctls related > > to recent security mitigations)? > > Can I commit the attached patch? (doc update will follow) If I don't hear any objections, I will commit the patch soon and I will request a pullup to netbsd-8. Alex
Re: secmodel_securelevel(9) and machdep.svs.enabled
Alexander Nasonov wrote:
> When securelevel is set, should be lock 1->0 change for
> machdep.svs.enabled (and possibly for other sysctls related
> to recent security mitigations)?
Can I commit the attached patch? (doc update will follow)
--
Alex
Index: src/sys/sys/kauth.h
===
RCS file: /cvsroot/src/sys/sys/kauth.h,v
retrieving revision 1.75
diff -p -u -u -r1.75 kauth.h
--- src/sys/sys/kauth.h 28 Aug 2017 00:46:07 - 1.75
+++ src/sys/sys/kauth.h 24 Apr 2018 17:59:13 -
@@ -320,7 +320,8 @@ enum {
KAUTH_MACHDEP_NVRAM,
KAUTH_MACHDEP_UNMANAGEDMEM,
KAUTH_MACHDEP_PXG,
- KAUTH_MACHDEP_X86PMC
+ KAUTH_MACHDEP_X86PMC,
+ KAUTH_MACHDEP_SVS_DISABLE
};
/*
Index: src/sys/secmodel/suser/secmodel_suser.c
===
RCS file: /cvsroot/src/sys/secmodel/suser/secmodel_suser.c,v
retrieving revision 1.43
diff -p -u -u -r1.43 secmodel_suser.c
--- src/sys/secmodel/suser/secmodel_suser.c 14 Jun 2017 17:48:41 -
1.43
+++ src/sys/secmodel/suser/secmodel_suser.c 24 Apr 2018 17:59:13 -
@@ -854,6 +854,7 @@ secmodel_suser_machdep_cb(kauth_cred_t c
case KAUTH_MACHDEP_UNMANAGEDMEM:
case KAUTH_MACHDEP_PXG:
case KAUTH_MACHDEP_X86PMC:
+ case KAUTH_MACHDEP_SVS_DISABLE:
if (isroot)
result = KAUTH_RESULT_ALLOW;
break;
Index: src/sys/secmodel/securelevel/secmodel_securelevel.c
===
RCS file: /cvsroot/src/sys/secmodel/securelevel/secmodel_securelevel.c,v
retrieving revision 1.30
diff -p -u -u -r1.30 secmodel_securelevel.c
--- src/sys/secmodel/securelevel/secmodel_securelevel.c 25 Feb 2014 18:30:13
- 1.30
+++ src/sys/secmodel/securelevel/secmodel_securelevel.c 24 Apr 2018 17:59:13
-
@@ -494,6 +494,11 @@ secmodel_securelevel_machdep_cb(kauth_cr
result = KAUTH_RESULT_DENY;
break;
+ case KAUTH_MACHDEP_SVS_DISABLE:
+ if (securelevel > 0)
+ result = KAUTH_RESULT_DENY;
+ break;
+
case KAUTH_MACHDEP_CPU_UCODE_APPLY:
if (securelevel > 1)
result = KAUTH_RESULT_DENY;
Index: src/sys/arch/x86/x86/svs.c
===
RCS file: /cvsroot/src/sys/arch/x86/x86/svs.c,v
retrieving revision 1.17
diff -p -u -u -r1.17 svs.c
--- src/sys/arch/x86/x86/svs.c 30 Mar 2018 19:58:05 - 1.17
+++ src/sys/arch/x86/x86/svs.c 24 Apr 2018 17:59:11 -
@@ -38,6 +38,7 @@ __KERNEL_RCSID(0, "$NetBSD: svs.c,v 1.17
#include
#include
#include
+#include
#include
#include
@@ -737,11 +738,13 @@ sysctl_machdep_svs_enabled(SYSCTLFN_ARGS
error = 0;
else
error = EOPNOTSUPP;
- } else {
- if (svs_enabled)
+ } else if (svs_enabled) {
+ error = kauth_authorize_machdep(kauth_cred_get(),
+ KAUTH_MACHDEP_SVS_DISABLE, NULL, NULL, NULL, NULL);
+ if (!error)
error = svs_disable();
- else
- error = 0;
+ } else {
+ error = 0;
}
return error;
Re: secmodel_securelevel(9) and machdep.svs.enabled
On Thu, 19 Apr 2018, Alexander Nasonov wrote: When securelevel is set, should be lock 1->0 change for machdep.svs.enabled (and possibly for other sysctls related to recent security mitigations)? Possibly. At the very least, we should prevent _disabling" the svs code if securelevel is set. IMHO. +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +--+--++
secmodel_securelevel(9) and machdep.svs.enabled
When securelevel is set, should be lock 1->0 change for machdep.svs.enabled (and possibly for other sysctls related to recent security mitigations)? -- Alex
