gmane.linux.kernel.lsm

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T13:53:22


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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T12:32:06


">= 255" that is.  "> 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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T12:29:32


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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

Tetsuo Handa — 2012-05-25T12:25:24

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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T12:18:23

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 <= 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, &end, 10);
if (sig_size >= size || (*end != ' ' && *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_ERR(mod_sig);
}

/* Load the module contents into the digest */
ret = verify_sig_add_data(mod_sig, data, mod_size);
if (ret < 0) {
verify_sig_cancel(mod_sig);
return ret;
}

/* Do the actual signature verification */
ret = verify_sig_end(mod_sig, sig, sig_size);
pr_devel("verify-sig : %d\n", ret);
return ret;
}

See:

http://git.kernel.org/?p=linux/kernel/git/dhowells/linux-modsign.git;a=shortlog;h=refs/heads/modsign-rusty

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T11:37:59


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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] lib: remove unused code from MPI library

Kasatkin, Dmitry — 2012-05-25T11:32:02

Hello.

No objections here?

- Dmitry


On Wed, May 9, 2012 at 5:37 PM, Dmitry Kasatkin
<dmitry.kasatkin< at >intel.com> wrote:
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

Kasatkin, Dmitry — 2012-05-25T11:15:40

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< at >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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KEYS: Fix some sparse warnings

James Morris — 2012-05-25T10:53:56


Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-queue

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T09:36:11


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
< at >< at > -35,6 +35,9 < at >< at > 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->key_n > 999)
+return 0; /* Don't overrun descbuf */
+
 sprintf(ctx->descbuf + ctx->dsize, "%d", ctx->key_n++);
 
 key = key_create_or_update(ctx->keyring, "crypto", ctx->descbuf,
< at >< at > -84,6 +87,7 < at >< at > 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 > sizeof(ctx.descbuf) - 4);
 strcpy(ctx.descbuf, descprefix);
 
 return pgp_parse_packets(pgpdata, pgpdatalen, &ctx.pgp);
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-25T09:26:48

Hi Tetsuo,

Thanks once again!

Tetsuo Handa <penguin-kernel< at >I-love.SAKURA.ne.jp> 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
< at >< at > -240,25 +240,22 < at >< at > static int pgp_process_public_key(struct pgp_parse_context *context,
 key->capabilities = pgp_public_key_capabilities[pgp.pubkey_algo] &
 (PKEY_CAN_ENCRYPT | PKEY_CAN_VERIFY);
 
-ret = -ENOMEM;
 for (i = 0; i < algo->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->mpi[i] = mpi_read_from_buffer(data, &remaining);
 if (!key->mpi[i])
-goto cleanup;
+goto cleanup_nomem;
 data += remaining;
 datalen -= remaining;
 }
 
-ret = -EBADMSG;
 if (datalen != 0) {
 pr_debug("excess %zu\n", datalen);
-goto cleanup;
+goto cleanup_badmsg;
 }
 
 ret = pgp_generate_fingerprint(ctx, &pgp, key);
< at >< at > -272,6 +269,11 < at >< at > static int pgp_process_public_key(struct pgp_parse_context *context,
 kleave(" = 0 [use]");
 return 0;
 
+cleanup_nomem:
+ret = -ENOMEM;
+goto cleanup;
+cleanup_badmsg:
+ret = -EBADMSG;
 cleanup:
 pr_devel("cleanup");
 if (key) {
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/3] move the secure_computing call

Andrew Lutomirski — 2012-05-25T00:55:02

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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/3] move the secure_computing call

Andrew Lutomirski — 2012-05-25T00:26:54

Just to clarify: are you suggesting that, for now, the traced behavior
should be:

process -> seccomp -> ptrace -> 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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/3] move the secure_computing call

Andrew Lutomirski — 2012-05-24T23:43:29

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

Re: [RFC PATCH 0/3] move the secure_computing call

Andrew Morton — 2012-05-24T22:00:57

On Thu, 24 May 2012 11:07:58 -0500
Will Drewry <wad< at >chromium.org> 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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET_TRACE

Indan Zupancic — 2012-05-24T20:17:05

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 execve is replaced with a
call to mmap2() to get a read-only shared mapping which is used to avoid
file path modifications and similar races. And when using seccomp all
events are PTRACE_EVENT_SECCOMP ones, and those are exactly the ones
where we need the shared read-only mapping. We could work around your
ad hoc restriction, but the main thing is if seccomp filters is just
about syscall numbers anyway, then why use BPF instead of a bitmask?

The whole point of PTRACE_EVENT_SECCOMP is to delegate to ptrace, it
means "we can't decide in the filter, ask the ptracer". This implies
that the ptracer is trusted, so doing any checks afterwards is a bit
pointless. But if you want to do it anyway, at least do it properly
and re-run the filter after ptrace. To avoid loops you need to allow
the syscall if you get another PTRACE_EVENT_SECCOMP.


As seccomp can generate ptrace events, the only thing that makes sense is
to either do seccomp first and ignore ptrace changes, or rerun filters after
any ptrace changes. But as I said in my other mail, if a process can call
ptrace(), it will pretty much avoid all secomp filters anyway, seccomp
filters which allow ptrace() are pretty much guaranteed to be insecure.
We're talking about a non-seccomped process ptracing a seccomped process,
both with the same UID. I don't think it matters in practice what you do
in this case, from a security point of view.

Greetings,

Indan


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/3] move the secure_computing call

Indan Zupancic — 2012-05-24T19:39:09

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< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/3] move the secure_computing call

Indan Zupancic — 2012-05-24T18:27:43

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.

Greetings,

Indan


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET_TRACE

Will Drewry — 2012-05-24T18:24:37

All this does is assert that the tracer can't change the syscall
number without it skipping the call.  If seccomp returned
SECCOMP_RET_TRACE because the argument to open was O_RDWR, then
everything is fine.


This enforces that the system call that is going to be executed is the
one that triggered SECCOMP_RET_TRACE.  That means seccomp is
delegating the go/no-go decision to the tracer.  I don't understand
your assertion here.  This code doesn't affect the PTRACE_SYSCALL
case.


Do you have a valid case where you want to remap one system call to
another without the ability to also handle the syscall exit path and
do any fixups?  I've mostly just seen skip, allow, update arguments -
not swapping the entire syscall.  That said, it's possible.  you could
do all sorts of weird things with ptrace if you want :)


I don't feel strongly about this RFC, but I don't believe that
expectations are being changed dramatically.

thanks!
will
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/23] Crypto keys and module signing

David Howells — 2012-05-24T14:00:51


It doesn't really need the < at > signs or the MSIZE; they can be dropped.

gpg --batch --no-greeting $(KEYFLAGS) -b $< && \
stat --printf %-5s $<.sig >$< at >.trailer && \
echo -n "This Is A Crypto Signed Module" >>$< at >.trailer && \
cat $< $<.sig $< at >.trailer >$< at >

Further, the signature size field can be reduced to 5 decimal digits easily -
if the signature is larger than 99999 bytes, there's very likely a problem
somewhere.

Oh, and I should use --printf, not -c with stat as the newline char is unneeded.

That then adds 5 bytes to the magic string.  Is that really so bad?

And if you object to generating a foo.ko.trailer file, then:

gpg --batch --no-greeting $(KEYFLAGS) -b $< && \
(cat $< $<.sig && \
 stat --printf %-5s $<.sig && \
 echo -n "This Is A Crypto Signed Module" && \
 ) >$< at >

Note that this scheme does not preclude using multiple signatures as you
desire, but since you have no record of the module length you'd either have to
parse all the signature records first to find the end of the module, or include
each preceding signature and marker in the digest for the next one (which
shouldn't be a problem).  You should still check all signatures anyway and
verify all for which you have the public key.

You could even stick other types of record in with different magic strings
terminating them, provided you include a length.


A chunk of which can be discarded with the above reductions.


Which will likely oops.  You need to decrement i, not increment it, but that's
a minor detail.  You're also subtracting a pointer from a size.

And no, I won't do this.  It's unnecessary and a potentially large overhead.
Say you've got a module that's 7.7M in size (an unstripped, unsigned CIFS
module for example)...  That's nearly eight *million* calls to memcmp() if
there's no signature.  I suspect that's on the order of a tenth of a second or
longer on most machines.

Stripped, CIFS is still on the order of half a meg - which in itself translates
to half a million calls to memcmp() if there's no signature.

Furthermore, the data cache may be of limited utility as it can't do readahead
as you're scanning backwards through the image.  You'd be much better off doing
a memchr() or just open-coding a forward search for 'T', and then doing the
memcmp() at each instance.  Better still, don't do the scan at all.

Doing a SHA digest on some machines will be done with hardware assistance
(s390, for example) - so this scan may take longer than the digest there.


You still haven't justified it.  One of your arguments about rejecting the ELF
parsing version was that it was too big for no useful extra value that I could
justify.  Supporting multiple signatures adds extra size and complexity for no
obvious value.

More importantly, a major problem with (multiple) signatures is that each
signing event has to risk exposing the private key - so you really only want to
sign once unless you cannot avoid it.  Further, in an automated system, the
private key cannot be protected by a password as all the secrets have to be
passed to the signer.

Trying to automatically save a during-build generated private key or trying to
get a private key into the build from within the RHEL and Fedora automated
build systems risks having a key stolen or having someone substitute their own
key - and also makes it more complicated to build a kernel outside of the build
system.

The way I handle the private key in these patches is with transience: A fresh
key is generated during a build from a clean tree and then discarded when the
build tree is deleted.  This key is then a one-off.  If it is stolen or
cracked, it can only affect a single build of the kernel.

Note that we _do_ allow extra public keys to be installed in the kernel as
there's much less risk there, and further they have to be there to permit
signature verification.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Smack: Maintainer Record

Casey Schaufler — 2012-05-24T01:34:52

From: Casey Schaufler <casey< at >schaufler-ca.com>
Subject: [PATCH] Smack: Maintainer Record

Add a maintainer record for the Smack LSM.

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by: Casey Schaufler <casey< at >schaufler-ca.com>

---

 MAINTAINERS |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d7ca204..ccc8a14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
< at >< at > -6133,6 +6133,15 < at >< at > S:Maintained
 F:include/linux/sl?b*.h
 F:mm/sl?b.c
 
+SMACK SECURITY MODULE
+M:Casey Schaufler <casey< at >schaufler-ca.com>
+L:linux-security-module< at >vger.kernel.org
+W:http://schaufler-ca.com
+T:git git://git.gitorious.org/smack-next/kernel.git
+S:Maintained
+F:Documentation/security/Smack.txt
+F:security/smack/
+
 SMC91x ETHERNET DRIVER
 M:Nicolas Pitre <nico< at >fluxnic.net>
 S:Odd Fixes


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html