http://permalink.gmane.org/gmane.linux.kernel.lsm hourly 1 1901-01-01T00:00+00:00 http://gmane.org/img/gmane-25t.png http://gmane.org http://permalink.gmane.org/gmane.linux.kernel.lsm/16505 <pre> Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-queue -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> James Morris 2012-05-26T02:03:33 http://permalink.gmane.org/gmane.linux.kernel.lsm/16504 <pre> Hi Rusty, If you prefer to have userspace extract the module signature and pass it in uargs, here's a tree that will do that: http://git.kernel.org/?p=linux/kernel/git/dhowells/linux-modsign.git;a=shortlog;h=refs/heads/modsign-uarg You can use it with the attached patch to kmod.git. I've passed the signature length in the argument as it permits the kernel to preallocate the buffer it decodes the signature into, but that's not actually necessary. David --- commit e4c9290fc949bc41f942f00a1460d9d90fc1775c Author: David Howells &lt;dhowells&lt; at &gt;redhat.com&gt; Date: Fri May 25 16:35:19 2012 +0100 libkmod: Extract module signature and prepend on module argument list Extract the module signature from the module image, if present, and prepend on the module argument list hexcoded as "modsign=l,x " Where "l" is the length of the signature as a decimal number and "x" is the hex-encoded signature data. Signed-off-by: David Howells &lt;dhowells&lt; at &gt;redhat.com&gt; diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module</pre> David Howells 2012-05-25T15:42:19 http://permalink.gmane.org/gmane.linux.kernel.lsm/16503 <pre> Yes, unfortunately, it is handled differently. Instead of passing a file descriptor, a buffer containing the file data is passed. Without the file descriptor, you also need to pass the file metadata, in this case, the signature as well. Whether the signature is passed as a separate parameter or appended to the file, it still is being passed to the kernel. IMA measures a file and appraises file data integrity based on the IMA measurement/appraisal policy. There are currently 3 hooks: open - ima_file_check(), exec - ima_bprm_check() and mmap - ima_file_mapp(). I don't know the reasons or history for passing the file data content, instead of a file descriptor, but if a file descriptor were passed, then module integrity appraisal could simply be another integrity hook. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Mimi Zohar 2012-05-25T14:40:28 http://permalink.gmane.org/gmane.linux.kernel.lsm/16502 <pre> Surely it's handled differently. The kernel is told by insmod what the signature should be in your scheme rather than going looking for it itself. In such a case, why not include the signature in the module file? It's more efficient on the filesystem, doesn't require xattr support and is easier for things like the initramfs composer to deal with. Btw, am I right in thinking that with IMA, the kernel itself normally goes and finds the signature (if there is one) for a file when it needs to open a file? Do you only check the IMA when exec'ing a file or whenever you open it? David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> David Howells 2012-05-25T13:53:22 http://permalink.gmane.org/gmane.linux.kernel.lsm/16500 <pre> "&gt;= 255" that is. "&gt; 255" is not possible. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> David Howells 2012-05-25T12:32:06 http://permalink.gmane.org/gmane.linux.kernel.lsm/16499 <pre> Hmmm... Good point. I should reduce 999 to 255. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> David Howells 2012-05-25T12:29:32 http://permalink.gmane.org/gmane.linux.kernel.lsm/16498 <pre>Looks OK. David Howells wrote: key_n is currently u8, isn't it? struct preload_pgp_keys_context { struct pgp_parse_context pgp; key_ref_t keyring; char descbuf[20]; u8 key_n; u8 dsize; }; -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Tetsuo Handa 2012-05-25T12:25:24 http://permalink.gmane.org/gmane.linux.kernel.lsm/16497 <pre> I've posted a new version with Tetsuo's comments fixed and module_verify_signature() reduced to: static int module_verify_signature(const void *data, size_t size) { struct crypto_key_verify_context *mod_sig; const char *cp, *sig; char *end; size_t magic_size, sig_size, mod_size; int ret; magic_size = sizeof(modsign_magic) - 1; if (size &lt;= 5 + magic_size) return 1; if (memcmp(data + size - magic_size, modsign_magic, magic_size) != 0) return 1; size -= 5 + magic_size; cp = data + size; sig_size = simple_strtoul(cp, &amp;end, 10); if (sig_size &gt;= size || (*end != ' ' &amp;&amp; *end != 'T')) return -ELIBBAD; mod_size = size - sig_size; sig = data + mod_size; /* Find the crypto key for the module signature * - !!! if this tries to load the required hash algorithm module, * we will deadlock!!! */ mod_sig = verify_sig_begin(modsign_keyring, sig, sig_size); if (IS_ERR(mod_sig)) { pr_err("Couldn't initiate module signature verification: %ld\n", PTR_ERR(mod_sig)); return PTR</pre> David Howells 2012-05-25T12:18:23 http://permalink.gmane.org/gmane.linux.kernel.lsm/16496 <pre> Exactly what I was thinking of, though I wouldn't call it "ima=" as that doesn't seem specific enough. You could also append it to the binary image and have the kernel extract it from there as if the module file had had the signature attached during the build. I don't particularly favour holding the signature in an xattr, though, as that assumes that underlying fs has the capability to hold it and means that the package installation mechanism must know how to fill in xattrs. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> David Howells 2012-05-25T11:37:59 http://permalink.gmane.org/gmane.linux.kernel.lsm/16495 <pre>Hello. No objections here? - Dmitry On Wed, May 9, 2012 at 5:37 PM, Dmitry Kasatkin &lt;dmitry.kasatkin&lt; at &gt;intel.com&gt; wrote: -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Kasatkin, Dmitry 2012-05-25T11:32:02 http://permalink.gmane.org/gmane.linux.kernel.lsm/16494 <pre> Actually it is not for hw. I have implemented reading of signature from security.ima xattr and from .sig file. http://linux-ima.git.sourceforge.net/git/gitweb.cgi?p=linux-ima/module-init-tools.git;a=summary There is no additional syscall. signature is passed is 'ima=' parameter to init_module() like ---------------------------------------------------------- root&lt; at &gt;ubuntu:~# modprobe -v button insmod /lib/modules/3.4.0-rc5-kds+/kernel/drivers/acpi/button.ko ima=016e7cbb4f00005d2b05fc633ee3e8010400ad30f2e50d52456ef4a4f0c540f3c8d9955b7ea125cd2dd0cb41216d1388801427c6bddc1431c82e4e82372e6a2101afdcf0daa4f59e1b3d9581d9f1fd0f003fede88d6679814a0887e056a7ddabf070e96cdf5901201d7a6cd4717af68500bd2af88d078a9f1cfc136f5e2d8d0df710121fbb5658c248714f77bb6879aba7b4 ---------------------------------------------------------- - Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordom</pre> Kasatkin, Dmitry 2012-05-25T11:15:40 http://permalink.gmane.org/gmane.linux.kernel.lsm/16493 <pre> Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-queue </pre> James Morris 2012-05-25T10:53:56 http://permalink.gmane.org/gmane.linux.kernel.lsm/16492 <pre> Probably. I'm going to assume that BUG_ON() is a good enough check for the moment as I assume the prefix will probably be generated at compile time. David --- diff --git a/security/keys/crypto/pgp_preload.c b/security/keys/crypto/pgp_preload.c index 25154e3..ef53902 100644 --- a/security/keys/crypto/pgp_preload.c +++ b/security/keys/crypto/pgp_preload.c &lt; at &gt;&lt; at &gt; -35,6 +35,9 &lt; at &gt;&lt; at &gt; static int __init found_pgp_key(struct pgp_parse_context *context, container_of(context, struct preload_pgp_keys_context, pgp); key_ref_t key; +if (ctx-&gt;key_n &gt; 999) +return 0; /* Don't overrun descbuf */ + sprintf(ctx-&gt;descbuf + ctx-&gt;dsize, "%d", ctx-&gt;key_n++); key = key_create_or_update(ctx-&gt;keyring, "crypto", ctx-&gt;descbuf, &lt; at &gt;&lt; at &gt; -84,6 +87,7 &lt; at &gt;&lt; at &gt; int __init preload_pgp_keys(const u8 *pgpdata, size_t pgpdatalen, ctx.keyring = make_key_ref(keyring, 1); ctx.key_n = 0; ctx.dsize = strlen(descprefix); +BUG_ON(ctx.dsize &gt; sizeof(ctx.descbuf) - 4); strcpy(ctx.descbuf, descprefix); return pgp_parse_packets(pgpdata, pgpd</pre> David Howells 2012-05-25T09:36:11 http://permalink.gmane.org/gmane.linux.kernel.lsm/16491 <pre> Hi Tetsuo, Thanks once again! Tetsuo Handa &lt;penguin-kernel&lt; at &gt;I-love.SAKURA.ne.jp&gt; wrote: How about the attached changes to patch 13? David --- diff --git a/security/keys/crypto/pgp_key_parser.c b/security/keys/crypto/pgp_key_parser.c index 9cfedc2..a6c3189 100644 --- a/security/keys/crypto/pgp_key_parser.c +++ b/security/keys/crypto/pgp_key_parser.c &lt; at &gt;&lt; at &gt; -240,25 +240,22 &lt; at &gt;&lt; at &gt; static int pgp_process_public_key(struct pgp_parse_context *context, key-&gt;capabilities = pgp_public_key_capabilities[pgp.pubkey_algo] &amp; (PKEY_CAN_ENCRYPT | PKEY_CAN_VERIFY); -ret = -ENOMEM; for (i = 0; i &lt; algo-&gt;n_pub_mpi; i++) { unsigned int remaining = datalen; -ret = -EBADMSG; if (remaining == 0) { pr_debug("short %zu mpi %d\n", datalen, i); -goto cleanup; +goto cleanup_badmsg; } key-&gt;mpi[i] = mpi_read_from_buffer(data, &amp;remaining); if (!key-&gt;mpi[i]) -goto cleanup; +goto cleanup_nomem; data += remaining; datalen -= remaining; } -ret = -EBADMSG; if (datalen != 0) { pr_debug("ex</pre> David Howells 2012-05-25T09:26:48 http://permalink.gmane.org/gmane.linux.kernel.lsm/16489 <pre> Hmm. I guess I agree. I'll shut up now :) --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Andrew Lutomirski 2012-05-25T00:55:02 http://permalink.gmane.org/gmane.linux.kernel.lsm/16487 <pre> Just to clarify: are you suggesting that, for now, the traced behavior should be: process -&gt; seccomp -&gt; ptrace -&gt; kernel? If so, I think the man page or something should have a big fat warning that seccomp filters should *never* allow ptrace (even PTRACE_TRACEME) unless they fully understand the issue. In any case, I think that the UML interaction is missing the point. UML will *emulate* the seccomp filter. If it chooses to use host seccomp filters for some business, that's its business. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Andrew Lutomirski 2012-05-25T00:26:54 http://permalink.gmane.org/gmane.linux.kernel.lsm/16485 <pre> IMO the behavior should change. Alternatively, a post-ptrace syscall should have to pass the *tracer's* seccomp filter, but that seems overcomplicated and confusing. OTOH, allowing ptrace in a seccomp filter is asking for trouble anyway </pre> Andrew Lutomirski 2012-05-24T23:43:29 http://permalink.gmane.org/gmane.linux.kernel.lsm/16483 <pre>On Thu, 24 May 2012 11:07:58 -0500 Will Drewry &lt;wad&lt; at &gt;chromium.org&gt; wrote: Perhaps you could repeat those comments in this changelog. Because my take on the above reasoning is "why did you bother writing these patches"! -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Andrew Morton 2012-05-24T22:00:57 http://permalink.gmane.org/gmane.linux.kernel.lsm/16482 <pre> Why wouldn't it be allowed to change the system call number? And try answering that question in a way that doesn't apply to syscall argument values too. No it's not fine, because it's inconsistent and arbitrary. It still gives normal ptracers the ability to bypass seccomp by changing allowed system calls into system calls that would have been denied. So considering ptrace can still be used to execute arbitrary system calls, why add this special case restriction to SECCOMP_RET_TRACE? One case is for system call injection where an arbitrary syscall is hijacked for the jailer's purposes. But that needs the exit path too. Another one is replacing fork() with clone(), but that's only necessary on 2.4. Another one is to let the process call exit() to get rid of it. But it's easy to get the exit notification by resuming with PTRACE_SYSCALL after getting a PTRACE_EVENT_SECCOMP event, so I don't see how it matters if we're interested in the exit path or not. In the jailer the first system call after an exec</pre> Indan Zupancic 2012-05-24T20:17:05 http://permalink.gmane.org/gmane.linux.kernel.lsm/16481 <pre> How? All that seccomp can do is deny system calls and kill the task. What you describe sounds more like ptrace. Seccomp can't change system calls, only prevent them, so I miss how it can change anything for UML in that way. Well, obviously it will use seccomp filters instead of ptrace when possible and do the more complicated stuff via PTRACE_SECCOMP_EVENT when seccomp isn't sufficient. But mainly for performance reasons. "process is traceable" should be "process is able to trace". Greetings, Indan -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo&lt; at &gt;vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html </pre> Indan Zupancic 2012-05-24T19:39:09 http://permalink.gmane.org/gmane.linux.kernel.lsm/16479 <pre> If so, then the seccomp check needs to be redone after any ptrace changes, or we should give up and just do the seccomp check first, instead of possibly looping forever. PTRACE_EVENT_SECCOMP has the same problem. If a seccomp filtered task can do ptrace(), it can easily bypass the seccomp filter by ptracing any task not under the same filter but from the same user. And then it can puppeteer the victim into doing anything it wishes. So pretending seccomp can make a ptracer secure is futile, I think. Perhaps it's better to keep it simple and always do the seccomp test first and ignore ptrace changes, however sad that may seem. Seccomp had the power to stop ptrace(). It didn't, so it shouldn't try to do it afterwards either. It's a bit fuzzy though, only reason why doing seccomp first is more convenient is because seccomp can generate ptrace events. I don't think it will make a difference in practice because ptrace(2) won't be allowed by seccomp filters anyway, so it's a bit of a theoretical problem. Greeti</pre> Indan Zupancic 2012-05-24T18:27:43 Search the mailing list at Gmane query http://search.gmane.org/?group=$group=gmane.linux.kernel.lsm