re: modstat and kaslr

2018-01-06 Thread matthew green
Maxime Villard writes:
> Hi,
> Here is a patch [1] that hides the addresses of the kernel modules when
> 'modstat -k' is entered by an unprivileged user. The current behavior is
> preserved for root.
> 
> The addresses currently leaked cannot be used to reconstruct the layout of
> the kernel, since the module VAs are embedded in bootspace.boot, whose 
> location
> is independent from that of each of the remaining kernel segments.
> 
> But it's still good not to leak such information, to limit the surface for ROP
> and a few other things, and this, also in the non-kaslr case. Ok?
> 
> [1] http://m00nbsd.net/garbage/module/modstat.diff

seems reasonable and needed with kaslr.

i wonder if this is something that should be hidden if security.curtain
is set, or something else with a higher hardening mode than normal,
rather than generally, or on systems without kaslr.  a higher hardened
mode should hide them from root too, i guess.


.mrg.


Re: modstat and kaslr

2018-01-01 Thread Maxime Villard

Le 01/01/2018 à 02:17, John Nemeth a écrit :

On Dec 31,  5:11pm, Maxime Villard wrote:
}
} Here is a patch [1] that hides the addresses of the kernel modules when
} 'modstat -k' is entered by an unprivileged user. The current behavior is
} preserved for root.
}
} The addresses currently leaked cannot be used to reconstruct the layout of
} the kernel, since the module VAs are embedded in bootspace.boot, whose 
location
} is independent from that of each of the remaining kernel segments.
}
} But it's still good not to leak such information, to limit the surface for ROP
} and a few other things, and this, also in the non-kaslr case. Ok?
}
} [1] http://m00nbsd.net/garbage/module/modstat.diff

@@ -150,10 +159,13 @@
strlcpy(ms->ms_required, mi->mi_required,
sizeof(ms->ms_required));
}
-   if (mod->mod_kobj != NULL) {
+   if (mod->mod_kobj != NULL && stataddr) {
kobj_stat(mod->mod_kobj, , );
ms->ms_addr = addr;
ms->ms_size = size;
+   } else {
+   ms->ms_addr = 0;
+   ms->ms_size = 0;
}
ms->ms_class = mi->mi_class;
ms->ms_refcnt = -1;

  I don't see why you added the part where you set ms_addr and
ms_size to 0 given that the memory was kmem_zalloc'ed and thus we
know that it is already 0?


Yes it's already zero, but I still added a branch to explicitly show that it
is zero. But that's indeed useless, and I've now removed it.


  Also, given the reason for preventing information leaks, I
would also make sure that the address isn't given out even for root
when secure_level has been elevated.


Actually, that's already the case. See secmodel_securelevel.c, there's

299 case KAUTH_SYSTEM_MODULE:
300 if (securelevel > 0)
301 result = KAUTH_RESULT_DENY;
302 break;

Maxime


Re: modstat and kaslr

2017-12-31 Thread John Nemeth
On Dec 31,  5:11pm, Maxime Villard wrote:
}
} Here is a patch [1] that hides the addresses of the kernel modules when
} 'modstat -k' is entered by an unprivileged user. The current behavior is
} preserved for root.
} 
} The addresses currently leaked cannot be used to reconstruct the layout of
} the kernel, since the module VAs are embedded in bootspace.boot, whose 
location
} is independent from that of each of the remaining kernel segments.
} 
} But it's still good not to leak such information, to limit the surface for ROP
} and a few other things, and this, also in the non-kaslr case. Ok?
} 
} [1] http://m00nbsd.net/garbage/module/modstat.diff

@@ -150,10 +159,13 @@
strlcpy(ms->ms_required, mi->mi_required,
sizeof(ms->ms_required));
}
-   if (mod->mod_kobj != NULL) {
+   if (mod->mod_kobj != NULL && stataddr) {
kobj_stat(mod->mod_kobj, , );
ms->ms_addr = addr;
ms->ms_size = size;
+   } else {
+   ms->ms_addr = 0;
+   ms->ms_size = 0;
}
ms->ms_class = mi->mi_class;
ms->ms_refcnt = -1;

 I don't see why you added the part where you set ms_addr and
ms_size to 0 given that the memory was kmem_zalloc'ed and thus we
know that it is already 0?

 Also, given the reason for preventing information leaks, I
would also make sure that the address isn't given out even for root
when secure_level has been elevated.

}-- End of excerpt from Maxime Villard


Re: modstat and kaslr

2017-12-31 Thread Christos Zoulas
In article ,
Maxime Villard   wrote:
>Hi,
>Here is a patch [1] that hides the addresses of the kernel modules when
>'modstat -k' is entered by an unprivileged user. The current behavior is
>preserved for root.
>
>The addresses currently leaked cannot be used to reconstruct the layout of
>the kernel, since the module VAs are embedded in bootspace.boot, whose location
>is independent from that of each of the remaining kernel segments.
>
>But it's still good not to leak such information, to limit the surface for ROP
>and a few other things, and this, also in the non-kaslr case. Ok?
>
>[1] http://m00nbsd.net/garbage/module/modstat.diff

That looks fine (I presume root can still see them)

christos