Re: [RFC PATCH v12 3/4] Linux Random Number Generator

[Pavel Machek]

Hi!

On Tue 2017-07-18 21:51:33, Theodore Ts'o wrote:
> On Tue, Jul 18, 2017 at 09:00:10PM -0400, Sandy Harris wrote:
> > The only really good solution I know of is to find a way to provide a
> > chunk of randomness early in the boot process. John Denker has a good
> > discussion of doing this by modifying the kernel image & Ted talks of
> > doing it via the boot loader. Neither looks remarkably easy. Other
> > approaches like making the kernel read a seed file or passing a
> > parameter on the kernel command line have been suggested but, if I
> > recall right, rejected.
> 
> It's actually not that _hard_ to modify the boot loader.  It's not
> finicky work like, say, adding support for metadata checksums or xattr
> deduplication to ext4.  It's actually mostly plumbing.  It's just that
> we haven't found a lot of people willing to do it as paid work, and
> the hobbyists haven't been interested.

Modifying the boot loader sources is not hard, right.

Deploying the modified boot loader is another story; these are
bootloaders -- they normally don't need updating, so they are often
not easy to update, or maybe updating them is risky.

Anyway, if you want to pay for some bootloader modifications... I'm
working for a company that can help :-). (Sometimes I use
pa...@denx.de address.)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 0/3] Introduce AMD Secure Processor device

[Pavel Machek]

On Thu 2017-06-22 06:42:01, Brijesh Singh wrote:
> CCP device (drivers/crypto/ccp/ccp.ko) is part of AMD Secure Processor,
> which is not dedicated solely to crypto. The AMD Secure Processor includes
> CCP and PSP (Platform Secure Processor) devices.
> 
> This patch series adds a framework that allows functional component of the
> AMD Secure Processor to be initialized and handled appropriately. The series
> does not makes any logic modification into CCP - it refactors the code to
> integerate CCP into AMD secure processor framework.

Ok, so this is just preparation. When finished, what services will it provide
to Linux userland?

Re: [PATCH v6 0/5] /dev/random - a new approach

[Pavel Machek]

Hi!

> > From my point of view, it would make sense to factor time from RTC and
> > mac addresses into the initial hash. Situation in the paper was so bad
> > some devices had _completely identical_ keys. We should be able to do
> > better than that.
> 
> We fixed that **years** ago.  In fact, the authors shared with me an
> early look at that paper and I implemented add_device_entropy() over
> the July 4th weekend back in 2012.  So we are indeed mixing in MAC
> addresses and the hardware clock (if it is initialized that early).
> In fact that was one of the first things that I did.  Note that this

Ok, thanks.

> > BTW... 128 interrupts... that's 1.3 seconds, right? Would it make
> > sense to wait two seconds if urandom use is attempted before it is
> > ready?
> 
> That really depends on the system.  We can't assume that people are
> using systems with a 100Hz clock interrupt.  More often than not
> people are using tickless kernels these days.  That's actually the
> problem with changing /dev/urandom to block until things are
> initialized.

Ok, let me check:

config HZ_PERIODIC
config NO_HZ_IDLE
config NO_HZ_FULL

in HZ_PERIODIC, there should be no problem.

NO_HZ_IDLE... should not be a problem either. We can easily make sure
that cpu's are not idle, something like 

 while (not_enough_entropy())
   schedule()

NO_HZ_FULL first, help text seems to imply that timer ticks still
happen when cpu is in kernel, and second, there is always one CPU that
handles timer ticks. So we are still ok.

So I believe we should add the wait to urandom. One second delay in
rare cases sounds better than alternatives.

Best regards,
Pavel
PS: Are there systems where the timer interrupt is the only source of time?
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/5] /dev/random - a new approach

[Pavel Machek]

Hi!

> As far as whether or not you can gather enough entropy at boot time,
> what we're really talking about how how much entropy we want to assume
> can be gathered from interrupt timings, since what you do in your code
> is not all that different from what the current random driver is
> doing.  So it's pretty easy to turn a knob and say, "hey presto, we
> can get all of the entropy we need before userspace starts!"  But
> justifying this is much harder, and using statistical tests isn't
> really sufficient as far as I'm concerned.

Actually.. I'm starting to believe that getting enough entropy before
userspace starts is more important than pretty much anything else.

We only "need" 64-bits of entropy, AFAICT. If it passes statistical
tests, I'd use it... for initial bringup.

We can switch to more conservative estimates when system is fully
running. But IMO it is very important to get _some_ randomness at the
begining...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] crypto: sha256-mb - cleanup a || vs | typo

[Pavel Machek]

On Fri 2016-07-08 10:19:26, Linus Torvalds wrote:
> [ rare comment rant. I think I'll do this once, and then ignore the 
> discussion ]
> 
> On Fri, Jul 8, 2016 at 9:45 AM, Herbert Xu  
> wrote:
> >
> > Nack.  As I said the commenting style in the crypto API is the
> > same as the network stack.  So unless we decide to change both
> > please stick to the current style.
> 
> Can we please get rid of the brain-damaged stupid networking comment
> syntax style, PLEASE?

Please? :-). Having different comment styles between networking and
the rest is confusing.

And yes, this uglyness is documented for net/ and drivers/net/, but
not for crypto/, so at the very least Documentation/CodingStyle should
be updated.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] random: add backtracking protection to the CRNG

[Pavel Machek]

On Mon 2016-06-13 11:48:39, Theodore Ts'o wrote:
> Signed-off-by: Theodore Ts'o 
> ---
>  drivers/char/random.c | 54 
> ++-
>  1 file changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index d640865..963a6a9 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -436,7 +436,8 @@ static int crng_init = 0;
>  #define crng_ready() (likely(crng_init > 0))
>  static void _extract_crng(struct crng_state *crng,
> __u8 out[CHACHA20_BLOCK_SIZE]);
> -static void extract_crng(__u8 out[CHACHA20_BLOCK_SIZE]);
> +static void _crng_backtrack_protect(struct crng_state *crng,
> + __u8 tmp[CHACHA20_BLOCK_SIZE], int used);
>  static void process_random_ready_list(void);

You can do u8 and u32 in the code, we know we are in kernel.

> +/*
> + * Use the leftover bytes from the CRNG block output (if there is
> + * enough) to mutate the CRNG key to provide backtracking protection.
> + */
> +static void _crng_backtrack_protect(struct crng_state *crng,
> + __u8 tmp[CHACHA20_BLOCK_SIZE], int used)
> +{
> + unsigned long   flags;
> + __u32   *s, *d;
> + int i;
> +
> + used = round_up(used, sizeof(__u32));
> + if (used + CHACHA20_KEY_SIZE > CHACHA20_BLOCK_SIZE) {
> + extract_crng(tmp);
> + used = 0;
> + }
> + spin_lock_irqsave(>lock, flags);
> + s = (__u32 *) [used];
> + d = >state[4];
> + for (i=0; i < 8; i++)
> + *d++ ^= *s++;
> + spin_unlock_irqrestore(>lock, flags);
> +}

You are basically trying to turn CRNG into one way hash function here,
right? Do you have any explanation that it has the required
properties?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] random: replace non-blocking pool with a Chacha20-based CRNG

[Pavel Machek]

Hi!

> Yes, I understand the argument that the networking stack is now
> requiring the crypto layer --- but not all IOT devices may necessarily
> require the IP stack (they might be using some alternate wireless
> communications stack) and I'd much rather not make things worse.
> 
> 
> The final thing is that it's not at all clear that the accelerated
> implementation is all that important anyway.  Consider the following
> two results using the unaccelerated ChaCha20:
> 
> % dd if=/dev/urandom bs=4M count=32 of=/dev/null
> 32+0 records in
> 32+0 records out
> 134217728 bytes (134 MB, 128 MiB) copied, 1.18647 s, 113 MB/s
> 
> % dd if=/dev/urandom bs=32 count=4194304 of=/dev/null
> 4194304+0 records in
> 4194304+0 records out
> 134217728 bytes (134 MB, 128 MiB) copied, 7.08294 s, 18.9 MB/s
> 
> So in both cases, we are reading 128M from the CRNG.  In the first
> case, we see the sort of speed we would get if we were using the CRNG
> for some illegitimate, such as "dd if=/dev/urandom of=/dev/sdX bs=4M"
> (because they were too lazy to type "apt-get install nwipe").
> 
> In the second case, we see the use of /dev/urandom in a much more
> reasonable, proper, real-world use case for /de/urandom, which is some
> userspace process needing a 256 bit session key for a TLS connection,
> or some such.  In this case, we see that the other overheads of
> providing the anti-backtracking protection, system call overhead,
> etc., completely dominate the speed of the core crypto primitive.
> 
> So even if the AVX optimized is 100% faster than the generic version,
> it would change the time needed to create a 256 byte session key from
> 1.68 microseconds to 1.55 microseconds.  And this is ignoring the

Ok, so lets say I'm writing some TLS server, and I know that traffic
is currently heavy because it was heavy in last 5 minutes. Would it
make sense for me to request 128M of randomness from /dev/urandom, and
then use that internally, to avoid the syscall overhead?

Ok, maybe 128M is a bit much because by requesting that much in single
request i'd turn urandom into PRNG, but perhaps 1MB block makes sense?

And I guess even requesting 128M would make sense, as kernel can
select best crypto implementation for CRNG, and I'd prefer to avoid
that code in my application as it is hardware-specific...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/7] /dev/random - a new approach

[Pavel Machek]

Hi!

> > On Sun 2016-06-19 17:58:41, Stephan Mueller wrote:
> > > Hi Herbert, Ted,
> > > 
> > > The following patch set provides a different approach to /dev/random which
> > > I call Linux Random Number Generator (LRNG) to collect entropy within the
> > > Linux kernel. The main improvements compared to the legacy /dev/random is
> > > to provide sufficient entropy during boot time as well as in virtual
> > > environments and when using SSDs. A secondary design goal is to limit the
> > > impact of the entropy collection on massive parallel systems and also
> > > allow the use accelerated cryptographic primitives. Also, all steps of
> > > the entropic data processing are testable. Finally massive performance
> > > improvements are visible at /dev/urandom and get_random_bytes.
> > 
> > Dunno. It is very similar to existing rng, AFAICT. And at the very
> > least, constants in existing RNG could be tuned to provide "entropy at
> > the boot time".
> 
> The key differences and thus my main concerns I have with the current design 
> are the following items. If we would change them, it is an intrusive change. 
> As of now, I have not seen that intrusive changes were accepted. This led me 
> to develop a competing algorithm.

Well, intrusive changes are hard to do, but replacing whole subsystems
is even harder -- and rightly so.

> - There was a debate around my approach assuming one bit of entropy per 
> received IRQ. I am really wondering about that discussion when there is a 
> much 
> bigger "forcast" problem with the legacy /dev/random: how can we credit HIDs 
> up to 11 bits of entropy when the user (a potential adversary) triggers these 
> events? I am sure I would be shot down with such an approach if I would 
> deliver that with a new implementation.

Well, if you can demonstrate 11 bits is too much, go ahead... I'm sure
that is rather easy to adjust.

But I believe that 1) user is not normally an adversary and 2) the
best thing for him to do would still be "pressing nothing". It will be
hard to press keys (etc) with great enough precision...

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 0/7] /dev/random - a new approach

[Pavel Machek]

On Sun 2016-06-19 17:58:41, Stephan Mueller wrote:
> Hi Herbert, Ted,
> 
> The following patch set provides a different approach to /dev/random which
> I call Linux Random Number Generator (LRNG) to collect entropy within the 
> Linux
> kernel. The main improvements compared to the legacy /dev/random is to provide
> sufficient entropy during boot time as well as in virtual environments and 
> when
> using SSDs. A secondary design goal is to limit the impact of the entropy
> collection on massive parallel systems and also allow the use accelerated
> cryptographic primitives. Also, all steps of the entropic data processing are
> testable. Finally massive performance improvements are visible at /dev/urandom
> and get_random_bytes.

Dunno. It is very similar to existing rng, AFAICT. And at the very
least, constants in existing RNG could be tuned to provide "entropy at
the boot time".

So IMO this should be re-done as tweaks to existing design, not as
completely new RNG.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: random(4) changes

[Pavel Machek]

Hi!

> > > > When dropping the add_disk_randomness function in the legacy
> > > > /dev/random, I
> > > > would assume that without changes to add_input_randomness and
> > > > add_interrupt_randomness, we become even more entropy-starved.
> > > 
> > > Sure, but your system isn't doing anything magical here.  The main
> > > difference is that you assume you can get almost a full bit of entropy
> > > out of each interrupt timing, where I'm much more conservative and
> > > assume we can only get 1/64th of a bit out of each interrupt timing.
> > 
> > Maybe 1/64th of a bit is a bit too conservative? I guess we really
> > have more than one bit of entropy on any system with timestamp
> > counter
> > 
> > Making it 1/2 of bit (or something) should be very easy way to improve
> > entropy early during boot...
> 
> I can easily settle on 1/2 bit here. The LRNG currently uses 0.9 bits which 
> are based on measurements plus a safety margin. But I see no issue to even 
> lower it further to, say, 1/2.

No, you don't need to change anything. But maybe mainline rng should
change.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: random(4) changes

[Pavel Machek]

Hi1

> > When dropping the add_disk_randomness function in the legacy /dev/random, I 
> > would assume that without changes to add_input_randomness and 
> > add_interrupt_randomness, we become even more entropy-starved.
> 
> Sure, but your system isn't doing anything magical here.  The main
> difference is that you assume you can get almost a full bit of entropy
> out of each interrupt timing, where I'm much more conservative and
> assume we can only get 1/64th of a bit out of each interrupt timing.

Maybe 1/64th of a bit is a bit too conservative? I guess we really
have more than one bit of entropy on any system with timestamp
counter

Making it 1/2 of bit (or something) should be very easy way to improve
entropy early during boot...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 0/6] /dev/random - a new approach

[Pavel Machek]

Hi!

> > So you are relying on high-resolution timestamps. Ok. then you do kind
> > of the check on the timestamps... ok, why not. But then you mix in the
> > data regardless, saying that "they are not dependent" and thus can't
> > hurt.
> > 
> > But you already know they _are_ dependent, that's what your stuck test
> > told you:
> 
> The stuck test says that there is a pattern, but not that the pattern shows a 
> dependency.
...
> > Now. I could imagine cases where interrupts are correlated... like
> > some hardware may generate two interrupts for each event or something
> > like that...
> 
> But I see what you are referring to and I think you have a valid point in a 
> worst case assessment.
> 
> Thus, any stuck value should not be mixed into the pool.

Thanks.

> /* This RNG does not work if no high-resolution timer is available */
> BUG_ON(!random_get_entropy() && !random_get_entropy());

Heh, does this cause BUG() with 2^-64 probability? :-).

> If there is no high-resolution timer, the LRNG will not produce good entropic 
> random numbers. The current kernel code implements high-resolution timers for 
> all but the following architectures where neither random_get_entropy nor 
> get_cycles are implemented:

Ok, what about stuff like Intel 486 (no RDTSC)?

> Thus, for all large-scale architectures, the LRNG would be applicable.
> 
> Please note that also the legacy /dev/random will have hard time to obtain 
> entropy for these environments. The majority of the entropy comes
> from high-

Understood.

> Though, the patch I offer leaves the legacy /dev/random in peace for those 
> architectures to not touch the status quo.

Well -- that's the major problem -- right? Makes it tricky to tell
what changed, and we had two RNGs to maintain.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 0/6] /dev/random - a new approach

[Pavel Machek]

Hi!

> Please find in [1] the full design discussion covering qualitative assessments
> of the entropy collection and entropy flow. Furthermore, a full
> testing of the

I don't get it.

# The
# idea is that only after obtaining LRNG_POOL_SIZE_BITS healthy bits,
# the
#entropy pool is completely changed with new bits. Yet, the stuck bit
# is not
# discarded as it may still contain some entropy. Hence, it is simply
# XORed
# with the previous bit as the XOR operation maintains the entropy since
# the previous time stamp and the current time stamp are not dependent
# on each other.

So you are relying on high-resolution timestamps. Ok. then you do kind
of the check on the timestamps... ok, why not. But then you mix in the
data regardless, saying that "they are not dependent" and thus can't
hurt.

But you already know they _are_ dependent, that's what your stuck test
told you:

# Thus, the stuck test
# ensures that:
# (a) variations exist in the time deltas,
# (b) variations of time deltas do not have a simple repeating pattern,
# and
# (c) variations do not have a linearly changing patterns (e.g. 1 - 2 -
# 4 - 7
# - 11 - 16).


Now. I could imagine cases where interrupts are correlated... like
some hardware may generate two interrupts for each event or something
like that...

What goes on if high resolution timer is not available?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] modsign: Fix GPL/OpenSSL licence incompatibility

[Pavel Machek]

On Tue 2015-09-15 16:03:36, David Howells wrote:
> From: David Woodhouse 
> 
> The GPL does not permit us to link against the OpenSSL library. Use
> LGPL for sign-file and extract-file instead.

Actually GPL does permit you to link to "system libraries", and it looks like 
OpenSSL is pretty much system library these days.

(But no problem with LGPL).


> 
> Reported-by: Julian Andres Klode 
> Signed-off-by: David Woodhouse 
> Signed-off-by: David Howells 
> Reviewed-by: Julian Andres Klode 
> ---
> 
>  scripts/extract-cert.c |   10 +-
>  scripts/sign-file.c|   13 -
>  2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/extract-cert.c b/scripts/extract-cert.c
> index 10d23ca9f617..6ce5945a0b89 100644
> --- a/scripts/extract-cert.c
> +++ b/scripts/extract-cert.c
> @@ -1,15 +1,15 @@
>  /* Extract X.509 certificate in DER form from PKCS#11 or PEM.
>   *
> - * Copyright © 2014 Red Hat, Inc. All Rights Reserved.
> - * Copyright © 2015 Intel Corporation.
> + * Copyright © 2014-2015 Red Hat, Inc. All Rights Reserved.
> + * Copyright © 2015  Intel Corporation.
>   *
>   * Authors: David Howells 
>   *  David Woodhouse 
>   *
>   * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public Licence
> - * as published by the Free Software Foundation; either version
> - * 2 of the Licence, or (at your option) any later version.
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1
> + * of the licence, or (at your option) any later version.
>   */
>  #define _GNU_SOURCE
>  #include 
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index 058bba3103e2..c3899ca4811c 100755
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -1,12 +1,15 @@
>  /* Sign a module file using the given key.
>   *
> - * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
> - * Written by David Howells (dhowe...@redhat.com)
> + * Copyright © 2014-2015 Red Hat, Inc. All Rights Reserved.
> + * Copyright © 2015  Intel Corporation.
> + *
> + * Authors: David Howells 
> + *  David Woodhouse 
>   *
>   * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public Licence
> - * as published by the Free Software Foundation; either version
> - * 2 of the Licence, or (at your option) any later version.
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1
> + * of the licence, or (at your option) any later version.
>   */
>  #define _GNU_SOURCE
>  #include 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] random: add random_initialized command line param

[Pavel Machek]

On Tue 2015-06-23 23:47:33, Stephan Mueller wrote:
 Am Dienstag, 23. Juni 2015, 22:44:11 schrieb Pavel Machek:
 
 Hi Pavel,
 
  On Mon 2015-05-18 18:25:25, Stephan Mueller wrote:
   Make the threshold at which the output entropy pools are considered to
   be initialized configurable via a kernel command line option. The
   current integer value of 128 bits is a good default value. However, some
   user groups may want to use different values. For example, the SOGIS
   group now requires 125 bits at least (BSI, the participant at that group
   used to require 100 bits). NIST moved from 80 bits to 112 bits starting
   with 2014.
   
   It is therefore to be expected that in the future, this threshold may
   increase for different user groups.
  
  Speaking of random and kernel parameters... should we add random=hex
  digits parameter to pass entropy from bootloader to the kernel?
 
 Everybody can see that string. How do you think that way of providing entropy 
 is protected?

Only local users, not remote attackers. And yes, we should probably
censor the command line for this use.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] random: add random_initialized command line param

[Pavel Machek]

On Mon 2015-05-18 18:25:25, Stephan Mueller wrote:
 Make the threshold at which the output entropy pools are considered to
 be initialized configurable via a kernel command line option. The
 current integer value of 128 bits is a good default value. However, some
 user groups may want to use different values. For example, the SOGIS
 group now requires 125 bits at least (BSI, the participant at that group
 used to require 100 bits). NIST moved from 80 bits to 112 bits starting
 with 2014.
 
 It is therefore to be expected that in the future, this threshold may
 increase for different user groups.

Speaking of random and kernel parameters... should we add random=hex
digits parameter to pass entropy from bootloader to the kernel?

u-boot-SPL does DRAM calibration, for example, and I guess that may
provide some rather hard to guess values...

[I initialy misread parameter below as random_initize= and thought
it does exactly this...]
Pavel

 + random_initialized= [KNL] Set the threshold in bits at which the
 + Linux random number generator considers an output
 + entropy pool initialized.
 + Format: int (must be = 112 and = size of output
 +entropy pool in bits)
 + Default: 128
 +
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] crypto: omap-sham: Check for return value from pm_runtime_get_sync

[Pavel Machek]

On Sun 2015-03-08 11:01:01, Pali Rohár wrote:
 Function pm_runtime_get_sync could fail and we need to check return
 value to prevent kernel crash.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/10] crypto: omap-sham: Check for return value from pm_runtime_get_sync

[Pavel Machek]

On Thu 2015-02-26 14:49:54, Pali Rohár wrote:
 Function pm_runtime_get_sync could fail and we need to check return
 value to prevent kernel crash.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/10] crypto: omap-sham: Add support for omap3 devices

[Pavel Machek]

On Thu 2015-02-26 14:49:53, Pali Rohár wrote:
 omap3 support is same as omap2, just with different IO address (specified in 
 DT)
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

 @@ -1792,6 +1792,10 @@ static const struct of_device_id omap_sham_of_match[] 
 = {
   .data   = omap_sham_pdata_omap2,
   },
   {
 + .compatible = ti,omap3-sham,
 + .data   = omap_sham_pdata_omap2,
 + },
 + {
   .compatible = ti,omap4-sham,
   .data   = omap_sham_pdata_omap4,
   },

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/10] ARM: OMAP3: Fix crypto support for HS devices

[Pavel Machek]

On Thu 2015-02-26 14:49:52, Pali Rohár wrote:
 Register crypto hwmod links only if they are not disabled in DT.
 If DT information is missing, enable them only for GP devices.
 
 Before this patch crypto hwmod links were always disabled for all HS devices
 and it was not possible to use omap-aes and omap-sham linux drivers.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/10] ARM: dts: omap3 hs: Remove timer12

[Pavel Machek]

On Thu 2015-02-26 14:49:55, Pali Rohár wrote:
 Device timer12 is automatically disabled on all HS devices via DTS property
 ti,timer-secure in file omap3.dtsi so it can be safely removed. We do not
 need to disable it on another place.

Dunno. Theoretically, Linux is not the only user of dts... so even if
linux handles ti,timer-secure as don't use it, that may not be right
for bootloaders etc.

 diff --git a/arch/arm/boot/dts/omap34xx-hs.dtsi 
 b/arch/arm/boot/dts/omap34xx-hs.dtsi
 index 1ff6264..d4be302 100644
 --- a/arch/arm/boot/dts/omap34xx-hs.dtsi
 +++ b/arch/arm/boot/dts/omap34xx-hs.dtsi
 @@ -10,7 +10,3 @@
  sham {
   status = disabled;
  };
 -
 -timer12 {
 - status = disabled;
 -};
 diff --git a/arch/arm/boot/dts/omap36xx-hs.dtsi 
 b/arch/arm/boot/dts/omap36xx-hs.dtsi
 index 2c7febb..e93ff8d 100644
 --- a/arch/arm/boot/dts/omap36xx-hs.dtsi
 +++ b/arch/arm/boot/dts/omap36xx-hs.dtsi
 @@ -10,7 +10,3 @@
  sham {
   status = disabled;
  };
 -
 -timer12 {
 - status = disabled;
 -};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] ARM: dts: n9/n950: Enable omap crypto support

[Pavel Machek]

On Thu 2015-02-26 14:49:57, Pali Rohár wrote:
 Harmattan system on Nokia N9 and N950 devices uses omap crypto support.
 Bootloader on those devices is known that it enables HW crypto support.
 This patch just include omap36xx.dtsi directly, so aes and sham is enabled.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
 +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
 @@ -8,7 +8,7 @@
   * published by the Free Software Foundation.
   */
  
 -#include omap36xx-hs.dtsi
 +#include omap36xx.dtsi
  
  / {
   cpus {

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] ARM: dts: Remove files omap34xx-hs.dtsi and omap36xx-hs.dtsi

[Pavel Machek]

On Thu 2015-02-26 14:50:00, Pali Rohár wrote:
 These files are not used by any DTS file anymore.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel  Machek pa...@ucw.cz

 ---
  arch/arm/boot/dts/omap34xx-hs.dtsi |   12 
  arch/arm/boot/dts/omap36xx-hs.dtsi |   12 
  2 files changed, 24 deletions(-)
  delete mode 100644 arch/arm/boot/dts/omap34xx-hs.dtsi
  delete mode 100644 arch/arm/boot/dts/omap36xx-hs.dtsi
 
 diff --git a/arch/arm/boot/dts/omap34xx-hs.dtsi 
 b/arch/arm/boot/dts/omap34xx-hs.dtsi
 deleted file mode 100644
 index d4be302..000
 --- a/arch/arm/boot/dts/omap34xx-hs.dtsi
 +++ /dev/null
 @@ -1,12 +0,0 @@
 -/* Disabled modules for secure omaps */
 -
 -#include omap34xx.dtsi
 -
 -/* Secure omaps have some devices inaccessible depending on the firmware */
 -aes {
 - status = disabled;
 -};
 -
 -sham {
 - status = disabled;
 -};
 diff --git a/arch/arm/boot/dts/omap36xx-hs.dtsi 
 b/arch/arm/boot/dts/omap36xx-hs.dtsi
 deleted file mode 100644
 index e93ff8d..000
 --- a/arch/arm/boot/dts/omap36xx-hs.dtsi
 +++ /dev/null
 @@ -1,12 +0,0 @@
 -/* Disabled modules for secure omaps */
 -
 -#include omap36xx.dtsi
 -
 -/* Secure omaps have some devices inaccessible depending on the firmware */
 -aes {
 - status = disabled;
 -};
 -
 -sham {
 - status = disabled;
 -};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/10] ARM: dts: omap3: Add missing dmas for crypto

[Pavel Machek]

On Thu 2015-02-26 14:49:56, Pali Rohár wrote:
 This patch adds missing dma DTS definitions for omap aes and sham drivers.
 Without it kernel drivers do not work.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel  Machek pa...@ucw.cz

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] ARM: dts: n900: Enable omap sham and include directly omap34xx.dtsi

[Pavel Machek]

On Thu 2015-02-26 14:49:58, Pali Rohár wrote:
 This patch moves content of file omap34xx-hs.dtsi into omap3-n900.dts and 
 enable
 omap sham support (omap HW support for SHA + MD5). After testing both omap 
 hwmod
 and omap-sham.ko drivers it looks like signed Nokia X-Loader enable L3 
 firewall
 for omap sham. There is no kernel crash with both official bootloader and 
 crypto
 enable bootloader. So we can safely enable sham code.
 

 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel  Machek pa...@ucw.cz


 +/*
 + * Default secure signed bootloader (Nokia X-Loader) does not enable L3 
 firewall
 + * for omap AES HW crypto support. When linux kernel try to access memory of 
 AES

  When Linux Kernel tries...

 + * blocks then kernel receive Unhandled fault: external abort on 
 non-linefetch
, the kernel receives

 + * and crash. Until somebody fix omap-aes.c and omap_hwmod_3xxx_data.c code 
 (no

  and crashes. Until somebody fixes
  
 + * crash anymore) omap AES support will be disabled for all Nokia N900 
 devices.
 + * There is unofficial version of bootloader which enables AES in L3 
 firewall
  
firewall,
  
 + * but it is not widely used and to prevent kernel crash rather AES is 
 disabled.
 + * There is also no runtime detection code if AES is disabled in L3 
 firewall...
 + */
 +aes {
 + status = disabled;
 +};
 +
  / {
   model = Nokia N900;
   compatible = nokia,omap3-n900, ti,omap3430, ti,omap3;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/10] ARM: dts: omap3-tao3530: Include directly omap34xx.dtsi

[Pavel Machek]

On Thu 2015-02-26 14:49:59, Pali Rohár wrote:
 This patch just move content of file omap34xx-hs.dtsi into omap3-tao3530.dts.
 There is no code change, patch is just preparation for removing -hs file.
 
 Signed-off-by: Pali Rohár pali.ro...@gmail.com

Acked-by: Pavel Machek pa...@ucw.cz

 --- a/arch/arm/boot/dts/omap3-tao3530.dtsi
 +++ b/arch/arm/boot/dts/omap3-tao3530.dtsi
 @@ -8,7 +8,16 @@
   */
  /dts-v1/;
  
 -#include omap34xx-hs.dtsi
 +#include omap34xx.dtsi
 +
 +/* Secure omaps have some devices inaccessible depending on the firmware */
 +aes {
 + status = disabled;
 +};
 +
 +sham {
 + status = disabled;
 +};
  
  / {
   cpus {

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Nokia N900: omap aes is broken

[Pavel Machek]

On Tue 2015-02-24 09:37:34, Tony Lindgren wrote:
 * Pali Rohár pali.ro...@gmail.com [150224 09:42]:
  On Tuesday 24 February 2015 18:25:12 Tony Lindgren wrote:
   * Pali Rohár pali.ro...@gmail.com [150218 16:03]:
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3938,8 +3938,9 @@ int __init omap3xxx_hwmod_init(void)

if (r  0)

return r;

-   /* Register GP-only hwmod links. */
-   if (h_gp  omap_type() == OMAP2_DEVICE_TYPE_GP) {
+// /* Register GP-only hwmod links. */
+// if (h_gp  omap_type() == OMAP2_DEVICE_TYPE_GP) {
+   if (h_gp) {

r = omap_hwmod_register_links(h_gp);
if (r  0)

return r;

aes hwmod is defined in GP-only hwmod...
   
   Doesn't this depend on the bootloader version of n900 to work?
   
   Regards,
   
   Tony
  
  Ok, it looks like second patch (omap_hwmod_3xxx_data.c) needs 
  that aes-enabled bootloader.
 
 OK we need some runtime detection somehow for what's enabled..
  
  But first patch (omap3.dtsi) is needed for proper definitions. 
  Otherwise omap-aes driver will never work on DT systems.
 
 Yeah that one makes sense to me, I guess you'll do a proper
 fix for that one.

If this depends on bootloader... would command line parameter be right
solution here?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -v4] random: introduce getrandom(2) system call

[Pavel Machek]

On Thu 2014-07-31 10:06:37, Bernd Petrovitsch wrote:
 On Don, 2014-07-31 at 00:18 +0200, Pavel Machek wrote:
  On Wed 2014-07-30 16:40:52, Bernd Petrovitsch wrote:
   On Mit, 2014-07-30 at 07:56 -0600, Bob Beck wrote:
Pavel. I have bit 'ol enterprise daemon running with established file
descriptors serving thousands of connections
which periodically require entropy.  Now I run out of descriptors. I
can't establish new connections. but I should
now halt all the other ones that require entropy?  I should raise
SIGKILL on my process serving these thousands
of connetions?  I don't think so.
   
   If that long-running daemon periodically needs something from a device,
   one would better keep the fd for that open the whole time. Saves some
   CPU cycles and latency too BTW.
  
  Agreed.
  
  On the other hand, keeping a fd open is quite tricky for a
  library. But better solution might be to make that easier.
 
 Yes, in a (full-fledged, standalone) library seems at least tricky (also
 referring to some off-list mails here: think about fork() - which could
 be inside system() or popen() or similar). 
 
 But as part of the *application* (where one has control over fork()
 etc.), this should be somewhat less risky. Yes, that doesn't really help
 libssl;-)

...

 Hehe, we (Unix!) have (had) gettimeofday(), time() and similar sys-calls
 since ages and no one proposed to make devices for them and get rid of
 the system-calls.

Well, but we do open(/dev/sda), read(1); not read_from_sda(...). And
cat /dev/urandom  file is useful operation. So it is not like
getentropy() can replace /dev/*random.

  open( , O_IM_A_LIBRARY_GIVE_ME_ONE_OF_THREE_RESERVED_FDS) might be one
  solution. Actually, one reserved fd should be enough.
 
 Well, this can also be DoSed and the proposal aims to make that
 impossible (and where does this reserved count against? process-limits,
 kernel-wide limit?).

Process limit. DoS should not be an issue here. Remember, we are doing
this to help libraries.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -v4] random: introduce getrandom(2) system call

[Pavel Machek]

Hi!

 The rationale of this system call is to provide resiliance against
 file descriptor exhaustion attacks, where the attacker consumes all
 available file descriptors, forcing the use of the fallback code where
 /dev/[u]random is not available.  Since the fallback code is often not
 well-tested, it is better to eliminate this potential failure mode
 entirely.

I'm not sure I understand the rationale; if someone can eat all your
file descriptors, he can make you stop working. So you can just stop
working when you can't open /dev/urandom, no?

Fallback code is probably very bad idea to use...

 The other feature provided by this new system call is the ability to
 request randomness from the /dev/urandom entropy pool, but to block
 until at least 128 bits of entropy has been accumulated in the
 /dev/urandom entropy pool.  Historically, the emphasis in the
 /dev/urandom development has been to ensure that urandom pool is
 initialized as quickly as possible after system boot, and preferably
 before the init scripts start execution.

Sounds like ioctl() for /dev/urandom for this behaviour would be nice?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] random: introduce getrandom(2) system call

[Pavel Machek]

On Wed 2014-07-23 14:10:16, Hannes Frederic Sowa wrote:
 
 
 On Wed, Jul 23, 2014, at 13:52, George Spelvin wrote:
  I keep wishing for a more general solution.  For example, some way to
  have a spare extra fd that could be accessed with a special O_NOFAIL
  flag.
  
  That would allow any number of library functions to not fail, such as
  logging from nasty corner cases.
  
  But you'd have to provide one per thread, and block non-fatal signals
  while it was open, so you don't get reentrancy problems.  Ick.
  
  
  This overly-specialized system call (and worse yet, a blocking
  system call that you can't put into a poll() loop) just feels ugly
  to me.  Is it *absolutely* necessary?
 
 One point that often came up besides fd exhaustion is missing
 /dev/u?random device nodes in chroot environments.

From the maillist discussion, it seems you sometimes _want_
/dev/random not to be present.

For example you want to trace exactly the same path through malware
every time.

  For example, how about simply making getentropy() a library function that
  aborts if it can't open /dev/urandom?  If you're suffering fd exhaustion,
  you're being DoSed already.
 
 Maybe applications want to mitigate fd exhaustion.

Dunno. Will we add special read_passwd() syscall that reads just
/etc/passwd, to allow uid-name resolution without available FDs?

I like the library function suggestion, this should not need a new
syscall.

And btw -- compatibility with getentropy() is _not_ going to be easy,
if they have different blocking / partial read / signals policy.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -v4] random: introduce getrandom(2) system call

[Pavel Machek]

On Wed 2014-07-30 16:40:52, Bernd Petrovitsch wrote:
 On Mit, 2014-07-30 at 07:56 -0600, Bob Beck wrote:
  Pavel. I have bit 'ol enterprise daemon running with established file
  descriptors serving thousands of connections
  which periodically require entropy.  Now I run out of descriptors. I
  can't establish new connections. but I should
  now halt all the other ones that require entropy?  I should raise
  SIGKILL on my process serving these thousands
  of connetions?  I don't think so.
 
 If that long-running daemon periodically needs something from a device,
 one would better keep the fd for that open the whole time. Saves some
 CPU cycles and latency too BTW.

Agreed.

On the other hand, keeping a fd open is quite tricky for a
library. But better solution might be to make that easier.

open( , O_IM_A_LIBRARY_GIVE_ME_ONE_OF_THREE_RESERVED_FDS) might be one
solution. Actually, one reserved fd should be enough.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Add in-kernel /dev/random equivalent

[Pavel Machek]

On Mon 2014-05-12 00:36:01, Stephan Mueller wrote:
 Hi,
 
 as discussed in thread [1], an in-kernel equivalent to the blocking 
 /dev/random device behavior is suggested. This in-kernel blocking access to 
 the RNG can be used to seed deterministic random number generators with 
 random 
 numbers that are directly backed by the underlying noise sources.
 
 The following patch implements this concept by adding a third output pool, 
 the 
 kernel_pool that is only accessible inside the kernel. That kernel_pool feeds 
 the blocking operation API calls to obtain random numbers.
 
 Please find a test kernel module attached. You can unload it even while the 
 collection process is ongoing.
 
 [1] https://lkml.org/lkml/2014/4/27/174
 
 Stephan Mueller (2):
   Addition of kernel_pool
   Asynchronous and syncronous API for accessing kernel_pool
 
  drivers/char/random.c  | 163 
 +
  include/linux/random.h |  16 +
  2 files changed, 166 insertions(+), 13 deletions(-)

 * Copyright (c) 2014
 *
 * Permission is hereby granted, free of charge, to any person
 *obtaining a copy
 * of this software and associated documentation files (the
 *Software), to deal
 * in the Software without restriction, including without limitation
 *the rights
 * to use, copy, modify, merge, publish, distribute, sublicense,
 *and/or sell
 * copies of the Software, and to permit persons to whom the Software
 *is
 * furnished to do so, subject to the following conditions:

...but module_license says this is GPL...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: x86/sha1: fix coverity CID 1195603

[Pavel Machek]

Hi!

 Most likely yes but I wanted to keep sha1_ssse3_mod_init consistent
 with sha256_ssse3_mod_init/sha512_ssse3_mod_init functions.


   Reported-by: scan-ad...@coverity.com
   Signed-off-by: Milos Vyletel milos.vyle...@gmail.com
   ---
arch/x86/crypto/sha1_ssse3_glue.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
  
   diff --git a/arch/x86/crypto/sha1_ssse3_glue.c
  b/arch/x86/crypto/sha1_ssse3_glue.c
   index 74d16ef..5352196 100644
   --- a/arch/x86/crypto/sha1_ssse3_glue.c
   +++ b/arch/x86/crypto/sha1_ssse3_glue.c
   @@ -235,31 +235,33 @@ static bool __init avx2_usable(void)
  
static int __init sha1_ssse3_mod_init(void)
{
   - char *algo_name;
   -
 
  Would simple algo_name = NULL be enough to fix this?

Dunno. I'd say that single-line change that is obviously right is
preferable to rewrite inspired by some other function.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

Hi!

 BTW: MFENCE is not guaranteed to flush the instruction pipeline; you
 need CPUID for that.
 
 I was not aware of that. Can I simply call the CPUID instruction to read 
 it or do I have to do something special?

Simply call CPUID (warning, it clobbers some registers.).
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

Hi!

 Of course, some of the state in the CPU may not be unknown to the
 attacker, if it is derived by external events that are not visible to
 the attacker, such as a network interrupt.  But if that's the case,
 why not measure network interrupts directly?  We're much less likely
 to overestimate the amount of entropy we can extract the system in
 that case.

Actually, I believe Stephan is up to something here.

We _can't_ measure network interrupts directly, because we do not have
TSC. (And TSC-less machines are the ones that are problematic, right?)

Extracting entropy from the CPU will allow us to pick up entropy from
network packets (and timer interrupt jitter) even on machines that
lack TSC. And that counts like very cool feature.

(And yes, we could just increment variable to get tsc emulation in
idle loop, and then extract entropy from that. But we would not be
able to enter low power states at that point, and it would not work
when cpu is busy computing.)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

Hi!

 I plugged that idea into my current Jitter RNG processing and disabled
 the other jitter measurements to get a clear, isolated picture.
 
 The result is also a white noise! And it is even quite fast.
 
 After doing some more research on this approach, I have to admit that 
 the output not good (i.e. white noise) in all situations. Therefore, I 
 dropped that (for now).

Is there chance to extract at least some entropy from it? (Can you
post the code you used for testing?) Because in this case we know
where the entropy comes from, which is important for Ted.

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

Hi!

 Another friend of mine mentioned that he assumes the rise and fall times 
 of transistors varies very slightly and could be the main reason for the 
 jitter. I do not think that this is really the case, because our gates 
 that form the CPU instructions comprise of many transistors. The 
 combined raise/fall jitter should cancel each other out.

Plus, there's clock that should make sure that this jitter does not
matter.

 There should be way to extract entropy more directly from various
 oscillator effects, no?
 
 I am working a different way of measuring such oscillator effects by 
 using the high-resolution timer of the CPU and measure it with a 
 Jiffies-based snapshotting window. So, here I would combine two timers 
 that are differently generated. If their frequencies would be relative 
 prime to each other, we would measure a direct oscillator effect.

I guess main problem is machines that do not have high-resolution
timer on the CPU (rdtsc). They do not get enough entropy during boot,
and the hell breaks loose.

But they usually _do_ have RTC or other clock, not driven by CPU
oscilator. Good.

What about just

while (!enough_entropy) {
  cur_time = read_rtc();
  simulated_tsc = 0;
  while (cur_time == read_rtc())
simulated_tsc++;
  gain_entropy_from(simulated_tsc)
}

(Where enough_entropy should be something like 128 bits).

This should work, we know why it works (drift between rtc and cpu
clock) and it does _not_ need rdtsc-style fast source.

Disadvantage is that it burns cpu, but, well, you only need 128
bits. Asuming the rtc used has 100Hz resolution, enough entropy should
be collected in under 2 seconds. That's acceptable adition to time it
takes generating ssh keys on slow cpu.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

Hi!

 sense of where the unpredictability might be coming from, and whether
 the unpredictability is coming from something which is fundamentally
 arising from something which is chaotic or quantum effect, or just
 because we don't have a good way of modelling the behavior of the
 L1/L2 cache (for example) and that is spoofing your entropy estimator.
 
 Please note: if the jitter really comes from the oscillator effect of 
 the RAM clock vs. the CPU clock (which I suspect), we will not be able 
 to alter the jitter software wise.

Well... if it is really oscillator effect, there should be _no_
entropy when running with L1/L2 enabled (because DRAM will not be
accessed at all at that case).

There should be way to extract entropy more directly from various
oscillator effects, no?

For DRAM, just perform timing, have entropy. Plus we could for example
measure PIT vs. other timer sources... (but I suspect that on PCs we
already have enough entropy...)
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPU Jitter RNG: inclusion into kernel crypto API and /dev/random

[Pavel Machek]

On Sat 2013-11-02 12:01:12, Pavel Machek wrote:
 Hi!
 
  sense of where the unpredictability might be coming from, and whether
  the unpredictability is coming from something which is fundamentally
  arising from something which is chaotic or quantum effect, or just
  because we don't have a good way of modelling the behavior of the
  L1/L2 cache (for example) and that is spoofing your entropy estimator.
  
  Please note: if the jitter really comes from the oscillator effect of 
  the RAM clock vs. the CPU clock (which I suspect), we will not be able 
  to alter the jitter software wise.
 
 Well... if it is really oscillator effect, there should be _no_
 entropy when running with L1/L2 enabled (because DRAM will not be
 accessed at all at that case).
 
 There should be way to extract entropy more directly from various
 oscillator effects, no?

Actually... bogomips calibrating loop already has jitter, measured
from difference between CPU clock and system clock... Could that be
used as entropy source? As a bonus, we should have it on most
platforms...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

[Pavel Machek]

Hi!

  On Sun 2013-09-15 08:56:59, Lee, Chun-Yi wrote:
   This patch introduced SNAPSHOT_SIG_HASH config for user to select which
   hash algorithm will be used during signature generation of snapshot.
  
  This series is big enough already... and who is going to test it?
 
 The hash config not just for testing, it's relate to the performance and
 secure between different hash algorithms.

I'm not saying it is for testing. I'm saying that selection makes
testing harder.

 There have person raised in LPC say he don't like SHA algorithm.

Well, I don't like the config option.

  There's no need to make hash configurable. Just select one that works.
 
 SHA1 has good performance, and SHA512 has better security, which one you
 like it?

Use SHA1. It is completely adequate for what you are trying to do.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

[Pavel Machek]

Hi!

 For the symmetric key solution, I will try HMAC (Hash Message
 Authentication Code). It's already used in networking, hope the
 performance is not too bad to a big image.

Kernel already supports crc32 of the hibernation image, you may want
to take a look how that is done.

Maybe you want to replace crc32 with cryptographics hash (sha1?) and
then use only hash for more crypto? That way speed of whatever crypto
you do should not be an issue.

Actually...

Is not it as simple as storing hash of hibernation image into NVRAM
and then verifying the hash matches the value in NVRAM on next
startup? No encryption needed. 

And that may even be useful for non-secure-boot people, as it ensures
you boot right image after resume, boot it just once, etc...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

[Pavel Machek]

On Sun 2013-09-15 08:56:59, Lee, Chun-Yi wrote:
 This patch introduced SNAPSHOT_SIG_HASH config for user to select which
 hash algorithm will be used during signature generation of snapshot.

This series is big enough already... and who is going to test it?
There's no need to make hash configurable. Just select one that works.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot

[Pavel Machek]

Hi!

  - Bootloader store the public key to EFI boottime variable by itself
  - Bootloader put The private key to S4SignKey EFI variable for forward 
   to
kernel.
  
  Is the UEFI NVRAM really suited for such regular updates?
  
 
 Yes, Matthew raised this concern at before. I modified patch to load
 private key in efi stub kernel, before ExitBootServices(), that means we
 don't need generate key-pair at every system boot. So, the above
 procedure of efi bootloader will only run one time. 
 
 User can enable SNAPSHOT_REGEN_KEYS kernel config to notify efi
 booloader regenerate key-pair for every S4 to improve security if he
 want. So, the key-pair re-generate procedure will only launched when S4
 resume, not every system boot.

How many writes can UEFI NVRAM survive? (Is it NOR?)

every S4 resume may be approximately every boot for some users...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot

[Pavel Machek]

   @@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, 
   efi_system_table_t *_table,

 setup_efi_pci(boot_params);

   +#ifdef CONFIG_SNAPSHOT_VERIFICATION
   + setup_s4_keys(boot_params);
   +#endif
   +
  
  Move ifdef inside the function?
 
 OK, I will define a dummy function for non-verification situation.

IIRC you can just put the #ifdef inside the function body. 
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/18] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

[Pavel Machek]

On Tue 2013-08-27 18:22:17, joeyli wrote:
 於 日，2013-08-25 於 18:43 +0200，Pavel Machek 提到：
  On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
   This patch introduced SNAPSHOT_SIG_HASH config for user to select which
   hash algorithm will be used during signature generation of snapshot.
   
   v2:
   Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before
   declare pkey_hash().
   
   Reviewed-by: Jiri Kosina jkos...@suse.cz
   Signed-off-by: Lee, Chun-Yi j...@suse.com
   ---
kernel/power/Kconfig|   46 
   ++
kernel/power/snapshot.c |   27 ++-
2 files changed, 68 insertions(+), 5 deletions(-)
   
   diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
   index b592d88..79b34fa 100644
   --- a/kernel/power/Kconfig
   +++ b/kernel/power/Kconfig
   @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION
   dependent on UEFI environment. EFI bootloader should generate the
   key-pair.

   +choice
   + prompt Which hash algorithm should snapshot be signed with?
   +depends on SNAPSHOT_VERIFICATION
   +help
   +  This determines which sort of hashing algorithm will be used 
   during
   +  signature generation of snapshot. This algorithm _must_ be 
   built into
   +   the kernel directly so that signature verification can take place.
   +   It is not possible to load a signed snapshot containing the algorithm
   +   to check the signature on that module.
  
  Like if 1000 ifdefs you already added to the code are not enough, you
  make some new ones?
  Pavel
  
 
 This SNAPSHOT_SIG_HASH kernel config is to select which SHA algorithms
 used for generate digest of snapshot. The configuration will captured by
 a const char* in code:
 
 +static const char *snapshot_hash = CONFIG_SNAPSHOT_SIG_HASH;
 +
 +static int pkey_hash(void)
 
 So, there doesn't have any ifdef block derived from this new config.

I'd say select one hash function, and use it. There's no need to make
it configurable.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot

[Pavel Machek]

On Tue 2013-08-27 14:01:42, Manfred Hollstein wrote:
 On Tue, 27 Aug 2013, 13:29:43 +0200, Pavel Machek wrote:
 @@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, 
 efi_system_table_t *_table,
  
   setup_efi_pci(boot_params);
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + setup_s4_keys(boot_params);
 +#endif
 +

Move ifdef inside the function?
   
   OK, I will define a dummy function for non-verification situation.
  
  IIRC you can just put the #ifdef inside the function body. 
 
 This is certainly not to be invoked on a frequent basis (and therefore
 not on a hot path), but from a more general angle, wouldn't this leave
 a(nother) plain jsr... rts sequence without any effect other than
 burning a few cycles? If the whole function call can be disabled
 (ignored) in a certain configuration, it shouldn't call at all, should
 it?

gcc should be able to deal with optimizing that out.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/18] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

[Pavel Machek]

On Thu 2013-08-22 19:01:41, Lee, Chun-Yi wrote:
 Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
 first step of signature generation operation
 (RSASSA-PKCS1-v1_5-SIGN).

Is this your own code, or did you copy it from somewhere?

 + if (!T)
 + goto error_T;
 +
 + memcpy(T, RSA_ASN1_templates[hash_algo].data, 
 RSA_ASN1_templates[hash_algo].size);
 + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks-digest, 
 pks-digest_size);
 +
 + /* 3) check If emLen  tLen + 11, output intended encoded message 
 length too short */
 + if (emLen  tLen + 11) {
 + ret = EINVAL;
 + goto error_emLen;
 + }

Normal kernel calling convention is 0 / -EINVAL.

 + memcpy(EM + 2, PS, emLen - tLen - 3);
 + EM[2 + emLen - tLen - 3] = 0x00;
 + memcpy(EM + 2 + emLen - tLen - 3 + 1, T, tLen);

ThisDoesNotLookLikeKernelCode, NoCamelCase, please.

 + *_EM = EM;

And we don't usually use _ prefix like this.


 --- a/include/crypto/public_key.h
 +++ b/include/crypto/public_key.h
 @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
  struct public_key_signature {
   u8 *digest;
   u8 digest_size; /* Number of bytes in digest */
 + u8 *S;  /* signature S of length k octets */

u8 *signature?

 + size_t k;   /* length k of signature S */

u8 *signature_length.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

[Pavel Machek]

On Thu 2013-08-22 19:01:42, Lee, Chun-Yi wrote:
 Due to RSA_I2OSP is not only used by signature verification path but also used
 in signature generation path. So, separate the length checking of octet string
 because it's not for generate 0x00 0x01 leading string when used in signature
 generation.
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com

 +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
 +{
 + unsigned x_size;
 + unsigned X_size;
 + u8 *X = NULL;

Is this kernel code or entry into obfuscated C code contest? This is not funny.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/18] asymmetric keys: support parsing PKCS #8 private key information

[Pavel Machek]

On Thu 2013-08-22 19:01:45, Lee, Chun-Yi wrote:
 Add ASN.1 files and parser to support parsing PKCS #8 noncompressed private
 key information. It's better than direct parsing pure private key because
 PKCS #8 has a privateKeyAlgorithm to indicate the algorithm of private
 key, e.g. RSA from PKCS #1
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com


 +#include crypto/public_key.h
 +
 +struct pkcs8_info {
 + enum pkey_algo privkey_algo:8;  /* Private key algorithm */

Are you sure this is well-defined?

 +struct private_key_algorithm *pkcs8_private_key_algorithms[PKEY_ALGO__LAST] 
 = {
 + [PKEY_ALGO_DSA] = NULL,
 +#if defined(CONFIG_PUBLIC_KEY_ALGO_RSA) || \
 + defined(CONFIG_PUBLIC_KEY_ALGO_RSA_MODULE)
 + [PKEY_ALGO_RSA] = RSA_private_key_algorithm,
 +#endif
 +};

  pkey_algo
  privkey_algo
  private_key_algorithm

...you use all variants.

Having symbols with __ inside them is interesting. I'd not do it.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/18] asymmetric keys: explicitly add the leading zero byte to encoded message

[Pavel Machek]

On Thu 2013-08-22 19:01:46, Lee, Chun-Yi wrote:
 Per PKCS1 spec, the EMSA-PKCS1-v1_5 encoded message is leading by 0x00 0x01 in
 its first 2 bytes. The leading zero byte is suppressed by MPI so we pass a
 pointer to the _preceding_ byte to RSA_verify() in original code, but it has
 risk for the byte is not zero because it's not in EM buffer's scope, neither
 RSA_verify() nor mpi_get_buffer() didn't take care the leading byte.
 
 To avoid the risk, that's better we explicitly add the leading zero byte to EM
 for pass to RSA_verify(). This patch allocate a _EM buffer to capture the
 result from RSA_I2OSP(), then set the first byte to zero in EM and copy the
 remaining bytes from _EM.
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com

 - ret = RSA_verify(H, EM - 1, k, sig-digest_size,
 + EM = kmalloc(k, GFP_KERNEL);
 + memset(EM, 0, 1);
 + memcpy(EM + 1, _EM, k-1);
 + kfree(_EM);

Spot a crash waiting to happen.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/18] Secure boot: Add new capability

[Pavel Machek]

On Thu 2013-08-22 19:01:47, Lee, Chun-Yi wrote:
 From: Matthew Garrett m...@redhat.com
 
 Secure boot adds certain policy requirements, including that root must not
 be able to do anything that could cause the kernel to execute arbitrary code.
 The simplest way to handle this would seem to be to add a new capability
 and gate various functionality on that. We'll then strip it from the initial
 capability set if required.

There was some discussion about this before, right? And I don't think
conclusion was it was acceptable...?

 Signed-off-by: Matthew Garrett m...@redhat.com
 Acked-by: Lee, Chun-Yi j...@suse.com
 Signed-off-by: Lee, Chun-Yi j...@suse.com
 ---
  include/uapi/linux/capability.h |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)
 
 diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
 index ba478fa..7109e65 100644
 --- a/include/uapi/linux/capability.h
 +++ b/include/uapi/linux/capability.h
 @@ -343,7 +343,11 @@ struct vfs_cap_data {
  
  #define CAP_BLOCK_SUSPEND36
  
 -#define CAP_LAST_CAP CAP_BLOCK_SUSPEND
 +/* Allow things that trivially permit root to modify the running kernel */
 +
 +#define CAP_COMPROMISE_KERNEL  37
 +
 +#define CAP_LAST_CAP CAP_COMPROMISE_KERNEL
  
  #define cap_valid(x) ((x) = 0  (x) = CAP_LAST_CAP)
  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/18] Secure boot: Add a dummy kernel parameter that will switch on Secure Boot mode

[Pavel Machek]

You may want to check subject. If it does something, it is not dummy.

 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -2784,6 +2784,13 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
   Note: increases power consumption, thus should only be
   enabled if running jitter sensitive (HPC/RT) workloads.
  
 + secureboot_enable=
 + [KNL] Enables an emulated UEFI Secure Boot mode.  This
 + locks down various aspects of the kernel guarded by the
 + CAP_COMPROMISE_KERNEL capability.  This includes things
 + like /dev/mem, IO port access, and other areas.  It can
 + be used on non-UEFI machines for testing purposes.
 +
   security=   [SECURITY] Choose a security module to enable at boot.
   If this boot parameter is not specified, only the first
   security module asking for security registration will be
 diff --git a/kernel/cred.c b/kernel/cred.c
 index e0573a4..c3f4e3e 100644
 --- a/kernel/cred.c
 +++ b/kernel/cred.c
 @@ -565,6 +565,23 @@ void __init cred_init(void)
0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
  }
  
 +void __init secureboot_enable()
 +{
 + pr_info(Secure boot enabled\n);
 + cap_lower((init_cred)-cap_bset, CAP_COMPROMISE_KERNEL);
 + cap_lower((init_cred)-cap_permitted, CAP_COMPROMISE_KERNEL);
 +}

OTOH you don't implement CAP_COMPROMISE_KERNEL, so it is dummy after
all. But CAP_COMPROMISE_KERNEL is infeasible to implement, right?
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/18] efi: Enable secure boot lockdown automatically when enabled in firmware

[Pavel Machek]

On Thu 2013-08-22 19:01:49, Lee, Chun-Yi wrote:
 From: Matthew Garrett m...@redhat.com
 
 The firmware has a set of flags that indicate whether secure boot is enabled
 and enforcing. Use them to indicate whether the kernel should lock itself
 down.  We also indicate the machine is in secure boot mode by adding the
 EFI_SECURE_BOOT bit for use with efi_enabled.

 + status = efi_call_phys5(sys_table-runtime-get_variable,
 + LSecureBoot, var_guid, NULL, datasize, sb);

What is this L... thing?
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot

[Pavel Machek]

On Thu 2013-08-22 19:01:50, Lee, Chun-Yi wrote:
 Introduced a hibernate_key.c file to query the key pair from EFI variables
 and maintain key pair for check signature of S4 snapshot image. We
 loaded the private key when snapshot image stored success.
 
 This patch introduced 2 EFI variables for store the key to sign S4 image and
 verify signature when S4 wake up. The names and GUID are:
   S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21
   S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21
 
 S4SignKey is used by EFI bootloader to pass the RSA private key that packaged
 by PKCS#8 format, kernel will read and parser it when system boot and reload
 it when S4 resume. EFI bootloader need gnerate a new private key when every
 time system boot.
 
 S4WakeKey is used to pass the RSA public key that packaged by X.509
 certificate, kernel will read and parser it for check the signature of
 S4 snapshot image when S4 resume.
 
 The follow-up patch will remove S4SignKey and S4WakeKey after load them
 to kernel for avoid anyone can access it through efivarfs.
 
 v3:
 - Load S4 sign key before ExitBootServices.
   Load private key before ExitBootServices() then bootloader doesn't need
   generate key-pair for each booting:
+ Add setup_s4_keys() to eboot.c to load S4 sign key before 
 ExitBootServices.
+ Reserve the memory block of sign key data blob in efi.c
 - In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign
   key before check hibernate image. It makes sure the new sign key will be
   transfer to resume target kernel.
 - Set depends on EFI_STUB in Kconfig
 
 v2:
 Add CONFIG_SNAPSHOT_VERIFICATION for build of hibernate_keys.c depend on
 Kconfig.
 
 Cc: Matthew Garrett mj...@srcf.ucam.org
 Cc: Takashi Iwai ti...@suse.de
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com


 --- a/arch/x86/boot/compressed/eboot.c
 +++ b/arch/x86/boot/compressed/eboot.c
 @@ -368,6 +368,91 @@ free_handle:
   return status;
  }
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 +static efi_status_t setup_s4_keys(struct boot_params *params)
 +{
 + struct setup_data *data;
 + unsigned long datasize;
 + u32 attr;
 + struct efi_s4_key *s4key;
 + efi_status_t status;
 +
 + data = (struct setup_data *)(unsigned long)params-hdr.setup_data;

A bit too many casts.

 @@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, 
 efi_system_table_t *_table,
  
   setup_efi_pci(boot_params);
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + setup_s4_keys(boot_params);
 +#endif
 +

Move ifdef inside the function?

 @@ -729,6 +792,11 @@ void __init efi_init(void)
  
   set_bit(EFI_SYSTEM_TABLES, x86_efi_facility);
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + /* keep s4 key from setup_data */
 + efi_reserve_s4_skey_data();
 +#endif
 +

Here too.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/18] Hibernate: generate and verify signature of snapshot

[Pavel Machek]

On Thu 2013-08-22 19:01:51, Lee, Chun-Yi wrote:
 This patch add the code for generate/verify signature of snapshot, it
 put the signature to snapshot header. This approach can support both
 on userspace hibernate and in-kernel hibernate.
 
 v2:
 - Due to loaded S4 sign key before ExitBootServices, we need forward key from
   boot kernel to resume target kernel. So this patch add a empty page in
   snapshot image, then we keep the pfn of this empty page in snapshot header.
   When system resume from hibernate, we fill new sign key to this empty page
   space after snapshot image checked pass. This mechanism let boot kernel can
   forward new sign key to resume target kernel but don't need write new 
 private
   key to any other storage, e.g. swap.
 
 Cc: Matthew Garrett mj...@srcf.ucam.org
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com
 ---
  kernel/power/power.h|6 +
  kernel/power/snapshot.c |  280 +-
  kernel/power/swap.c |   14 +++
  kernel/power/user.c |9 ++
  4 files changed, 302 insertions(+), 7 deletions(-)
 
 diff --git a/kernel/power/power.h b/kernel/power/power.h
 index 69a81d8..84e0b06 100644
 --- a/kernel/power/power.h
 +++ b/kernel/power/power.h
 @@ -3,6 +3,9 @@
  #include linux/utsname.h
  #include linux/freezer.h
  
 +/* The maximum length of snapshot signature */
 +#define SIG_LENG 512
 +
  struct swsusp_info {
   struct new_utsname  uts;
   u32 version_code;
 @@ -11,6 +14,8 @@ struct swsusp_info {
   unsigned long   image_pages;
   unsigned long   pages;
   unsigned long   size;
 + unsigned long   skey_data_buf_pfn;
 + u8  signature[SIG_LENG];
  } __attribute__((aligned(PAGE_SIZE)));

SIG_LEN or SIG_LENGTH. Select one.


 +static int
  copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
  {
   struct zone *zone;
 - unsigned long pfn;
 + unsigned long pfn, dst_pfn;
...
 + tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0);
 + if (IS_ERR(tfm)) {
 + pr_err(IS_ERR(tfm): %ld, PTR_ERR(tfm));
 + return PTR_ERR(tfm);
 + }
 +
 + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
 + digest_size = crypto_shash_digestsize(tfm);
 + digest = kzalloc(digest_size + desc_size, GFP_KERNEL);

Are you sure GFP_KERNEL allocation is okay at this phase of
hibernation?

Could the hashing be done at later phase, when writing the image to
disk?

  
 +void **h_buf;

helpfully named.

 + ret = verify_signature(s4_wake_key, pks);
 + if (ret) {
 + pr_err(snapshot S4 signature verification fail: %d\n, ret);
 + goto error_verify;
 + } else
 + pr_info(snapshot S4 signature verification pass!\n);
 +
 + if (pks-rsa.s)
 + mpi_free(pks-rsa.s);
 + kfree(pks);

ret = 0 and fall through?

 + return 0;
 +
 +error_verify:
 + if (pks-rsa.s)
 + mpi_free(pks-rsa.s);
 +error_mpi:
 + kfree(pks);
 + return ret;
 +}


 + ret = crypto_shash_final(desc, digest);
 + if (ret)
 + goto error_shash;
 +
 + ret = snapshot_verify_signature(digest, digest_size);
 + if (ret)
 + goto error_verify;
 +
 + kfree(h_buf);
 + kfree(digest);
 + crypto_free_shash(tfm);
 + return 0;

These four lines can be deleted.

 +
 +error_verify:
 +error_shash:
 + kfree(h_buf);
 + kfree(digest);
 +error_digest:
 + crypto_free_shash(tfm);
 + return ret;
 +}
 +
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/18] Hibernate: Avoid S4 sign key data included in snapshot image

[Pavel Machek]

On Thu 2013-08-22 19:01:52, Lee, Chun-Yi wrote:
 This patch add swsusp_page_is_sign_key() method to hibernate_key.c and
 check the page is S4 sign key data when collect saveable page in
 snapshot.c to avoid sign key data included in snapshot image.
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com
 ---
  kernel/power/snapshot.c |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
 index 72e2911..9e4c94b 100644
 --- a/kernel/power/snapshot.c
 +++ b/kernel/power/snapshot.c
 @@ -860,6 +860,9 @@ static struct page *saveable_highmem_page(struct zone 
 *zone, unsigned long pfn)
  
   BUG_ON(!PageHighMem(page));
  
 + if (swsusp_page_is_sign_key(page))
 + return NULL;
 +
   if (swsusp_page_is_forbidden(page) ||  swsusp_page_is_free(page) ||
   PageReserved(page))
   return NULL;

Just do set_page_forbidden() on your page?
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/18] Hibernate: adapt to UEFI secure boot with signature check

[Pavel Machek]

On Thu 2013-08-22 19:01:54, Lee, Chun-Yi wrote:
 In current solution, the snapshot signature check used the RSA key-pair
 that are generated by bootloader(e.g. shim) and pass the key-pair to
 kernel through EFI variables. I choice to binding the snapshot
 signature check mechanism with UEFI secure boot for provide stronger
 protection of hibernate. Current behavior is following:
 
  + UEFI Secure Boot ON, Kernel found key-pair from shim:
Will do the S4 signature check.
 
  + UEFI Secure Boot ON, Kernel didn't find key-pair from shim:
Will lock down S4 function.
 
  + UEFI Secure Boot OFF
Will NOT do the S4 signature check.
Ignore any keys from bootloader.
 
 v2:
 Replace sign_key_data_loaded() by skey_data_available() to check sign key data
 is available for hibernate.
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com
 ---
  kernel/power/hibernate.c |   36 +-
  kernel/power/main.c  |   11 +-
  kernel/power/snapshot.c  |   95 
 ++
  kernel/power/swap.c  |4 +-
  kernel/power/user.c  |   11 +
  5 files changed, 112 insertions(+), 45 deletions(-)
 
 diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
 index c545b15..0f19f3d 100644
 --- a/kernel/power/hibernate.c
 +++ b/kernel/power/hibernate.c
 @@ -29,6 +29,7 @@
  #include linux/ctype.h
  #include linux/genhd.h
  #include linux/key.h
 +#include linux/efi.h
  
  #include power.h
  
 @@ -632,7 +633,14 @@ static void power_down(void)
  int hibernate(void)
  {
   int error;
 - int skey_error;
 +
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + if (!capable(CAP_COMPROMISE_KERNEL)  !skey_data_available()) {
 +#else
 + if (!capable(CAP_COMPROMISE_KERNEL)) {
 +#endif
 + return -EPERM;
 + }
  
   lock_system_sleep();
   /* The snapshot device should not be opened while we're running */
 @@ -799,6 +807,15 @@ static int software_resume(void)
   if (error)
   goto Unlock;
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + if (!capable(CAP_COMPROMISE_KERNEL)  !wkey_data_available()) {
 +#else
 + if (!capable(CAP_COMPROMISE_KERNEL)) {
 +#endif
 + mutex_unlock(pm_mutex);
 + return -EPERM;
 + }
 +
   /* The snapshot device should not be opened while we're running */
   if (!atomic_add_unless(snapshot_device_available, -1, 0)) {
   error = -EBUSY;
 @@ -892,6 +909,15 @@ static ssize_t disk_show(struct kobject *kobj, struct 
 kobj_attribute *attr,
   int i;
   char *start = buf;
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + if (efi_enabled(EFI_SECURE_BOOT)  !skey_data_available()) {
 +#else
 + if (efi_enabled(EFI_SECURE_BOOT)) {
 +#endif
 + buf += sprintf(buf, [%s]\n, disabled);
 + return buf-start;
 + }
 +
   for (i = HIBERNATION_FIRST; i = HIBERNATION_MAX; i++) {
   if (!hibernation_modes[i])
   continue;
 @@ -926,6 +952,14 @@ static ssize_t disk_store(struct kobject *kobj, struct 
 kobj_attribute *attr,
   char *p;
   int mode = HIBERNATION_INVALID;
  
 +#ifdef CONFIG_SNAPSHOT_VERIFICATION
 + if (!capable(CAP_COMPROMISE_KERNEL)  !skey_data_available()) {
 +#else
 + if (!capable(CAP_COMPROMISE_KERNEL)) {
 +#endif
 + return -EPERM;
 + }
 +
   p = memchr(buf, '\n', n);
   len = p ? p - buf : n;
  

You clearly need some helper function.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/18] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

[Pavel Machek]

On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
 This patch introduced SNAPSHOT_SIG_HASH config for user to select which
 hash algorithm will be used during signature generation of snapshot.
 
 v2:
 Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before
 declare pkey_hash().
 
 Reviewed-by: Jiri Kosina jkos...@suse.cz
 Signed-off-by: Lee, Chun-Yi j...@suse.com
 ---
  kernel/power/Kconfig|   46 ++
  kernel/power/snapshot.c |   27 ++-
  2 files changed, 68 insertions(+), 5 deletions(-)
 
 diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
 index b592d88..79b34fa 100644
 --- a/kernel/power/Kconfig
 +++ b/kernel/power/Kconfig
 @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION
 dependent on UEFI environment. EFI bootloader should generate the
 key-pair.
  
 +choice
 + prompt Which hash algorithm should snapshot be signed with?
 +depends on SNAPSHOT_VERIFICATION
 +help
 +  This determines which sort of hashing algorithm will be used during
 +  signature generation of snapshot. This algorithm _must_ be built 
 into
 +   the kernel directly so that signature verification can take place.
 +   It is not possible to load a signed snapshot containing the algorithm
 +   to check the signature on that module.

Like if 1000 ifdefs you already added to the code are not enough, you
make some new ones?
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rfc4106, Intel, AES-NI: Don't leak memory in rfc4106_set_hash_subkey().

[Pavel Machek]

On Mon 2011-02-07 19:24:43, Jesper Juhl wrote:
 On Mon, 7 Feb 2011, tadeusz.st...@intel.com wrote:
 
  From: 
  Date: Sun, 16 Jan 2011 16:41:11 +
  Subject: RE: [PATCH] rfc4106, Intel, AES-NI: Don't leak memory in 
  rfc4106_set_hash_subkey().
  
  Hi Jesper,
  Thanks, but I think there is still a problem here. You don't want to kfree 
  req_data
  when the kmalloc failed. I think it should look as follows.  
  Are you ok with this?
  
 Fine by me.
 
 I was aware of the kfree(NULL) thing, but desided to leave it as is for 
 two reasons - 1) kfree(NULL) is harmless and this is an error path, so the 
 extra function call doesn't matter much. 2) I wanted to preserve 
 deallocations in the reverse order of the allocations. But sure, moving 
 that kfree is a tiny optimization of the error path, so I'm fine with it.

I don't think such optimalization is worth doing... original code is
as good but shorter and less complex...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19] RFC, v2: New /dev/crypto user-space interface

[Pavel Machek]

Hi!

 Motivations for the extensions: governments are asking for more security
 features in the operating systems they procure, which make user-space
 implementations impractical.  A few examples:
 
 * Advanced crypto module for OSPP for Common Criteria requires OS services
   implementing several low-level crypto algorithms (e.g. AES, RSA).  This
   requires the separation of crypto services from the consumer of those
   services. (The threat model is that apps tend to have more vulnerabilities
   than libraries and compromise of the app will lead to the ability to access
   key material.) An user-space library is not separated, options are a) root
   running daemon that does crypto, but this would be slow due to context
   switches, scheduler mismatching and all the IPC overhead and b) use crypto
   that is in the kernel.

Hmm, root daemon seems like a way to go. You already do the switch
into the kernel... and IPC is slow is not good enough reason to put
everything in kernel. Plus, you should be able to get better usage of
multicore with daemon.

Numbers?
Pavel


 * FIPS-140-3 calls out for cryptographic functions to be non-debuggable 
 (ptrace)
   meaning that you cannot get to the key material. The solution is the same as
   above.
 
 * GPOSPP requires auditing for crypto events (so does FIPS-140 level 2 cert).
   To do this you need any crypto to have CAP_AUDIT_WRITE permissions which
   means making everything that links to openssl, libgcrypt, or nss setuid
   root. Making firefox and 400 other applications setuid root is a 
 non-starter.
   So, the solution is again to use crypto in the kernel where auditing needs 
 no
   special permissions.
 
 Other advantages to having kernel crypto available to user space:
 
 * User space will be able to take advantage of kernel drivers for hardware
   crypto accelerators.
 
 * glibc, which in some configurations links to libfreebl3.so for hashes
   necessary for crypt(), will be able to use the kernel implementation; this
   means one less library to load and dynamically link for each such process.
 
 The code is derived from the original cryptodev-linux patch set; most of the
 new implementation was written by Nikos Mavrogiannopoulos
 n.mavrogiannopou...@gmail.com.  Attributions are included in the respective
 source files.
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Using Intel CRC32 instruction to accelerate CRC32c algorithm by new crypto API -V3.

[Pavel Machek]

On Fri 2008-08-08 21:35:30, Herbert Xu wrote:
 Ulrich Drepper [EMAIL PROTECTED] wrote:
 
  I think you want to use
  
   #define SCALE_F sizeof(unsigned long)
 
 Yeah in general that's what we should do.  However, this driver
 is specific to Intel x86 CPUs.

I thought we support intel x86 cpus in both 32 and 64bit modes...?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] MODSIGN: Kernel module signing

[Pavel Machek]

Hi!

 Now, this is not a complete solution by any means: the core kernel is not
 protected, and nor are /dev/mem or /dev/kmem, but it denies (or at least
 controls) one relatively simple attack vector.

Could we fix the /dev/*mem holes, first? They are already used by
malicious modules (aka rootkits...).  Or can selinux already provide
/dev/*mem protection with no way for admin to turn it off?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html