Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Eric,


The solution to the "too many system calls" problem is trivial: just do SHA-512
in userspace.  It's just math; you don't need a system call, any more than you
would call sys_add(1, 1) to compute 1 + 1.  The CPU instructions that can
accelerate SHA-512, such as AVX and ARM CE, are available in userspace too; and
there are tons of libraries already available that implement it for you.  Your
argument isn't fundamentally different from saying that sys_leftpad() (if we had
the extraordinary misfortune of it actually existing) is too slow, so we should
add a Javascript interpreter to the kernel.


So lets recap.  The Kernel crypto framework is something that:
a) (some, many?) people are totally happy with, it does everything that 
they want

b) is peer reviewed by the best programmers in the world
c) responds / fixes vulnerabilities almost instantly
d) automatically picks the best software optimized version of a given 
crypto algorithm for us

e) automagically uses hardware optimization if the system supports it
f) API compatibility is essentially guaranteed forever
g) Maybe not the most performant in the world, but to many users this 
doesn't matter.


So your response to those users is to please stop using what works well 
and start adding random crypto code from the internet into their 
project?  Something that likely won't do a, b, c, d, e or f above just 
because *oh gosh* we might find and have to fix some bugs in the kernel? 
 Have you actually thought through how that sounds?


What you call laziness I call 'common sense' and 'good security 
practice.'  Does using the kernel make sense for everyone? No.  But for 
some it does.  So if there's a legitimate way to make things better, can 
we not discuss them civilly?




Also note that in the rare cases where the kernel actually does do very long
running calculations for whatever reason, kernel developers pretty regularly
screw it up by forgetting to insert explicit preemption points (cond_resched()),
or (slightly less bad) making it noninterruptible.  I had to "fix" one of these,
where someone for whatever reason added a keyctl() operation that does
Diffie-Hellman key exchange in software.  In !CONFIG_PREEMPT kernels any
unprivileged user could call it to lock up all CPUs for 20+ seconds, meaning
that no other processes can be scheduled on them.  This isn't a problem at all
in userspace.


And this is exactly why people should _want_ to use the kernel crypto 
framework.  Because people like you exist and fix such issues.  So 
again, kudos :)


Regards,
-Denis

Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Ted,


I'm not really here to criticize or judge the past.  AF_ALG exists now. It
is being used.  Can we just make it better?  Or are we going to whinge at
every user that tries to use (and improve) kernel features that (some)
people disagree with because it can 'compromise' kernel security?


Another point of view is that it was arguably a mistake, and we
shouldn't make it worse.


Fair enough.  I'm just voicing the opposite point of view.  Namely that 
you have created something nice, and useful.  Even if it turned out not 
quite like you thought it would be in hindsight.





Also, if speed isn't a worry, why not just a single software-only
implementation of SHA1, and be done with it?  It's what I did in
e2fsprogs for e4crypt.


If things were that simple, we would definitely not be having this exchange.
Lets just say we use just about every feature that crypto subsystem provides
in some way.


What I'm saying here is if you need to code PBPDF2 in user-space, it
_really_ isn't hard.  I've done it.  It's less than 7k of source code
to implement SHA512:

https://ghit.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/tree/lib/ext2fs/sha256.c

and then less than 50 lines of code to implement PBPDF2 (I didn't even
bother putting in a library such as libext2fs):

https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/tree/misc/e4crypt.c#n405

This is all you would need to do if we don't put PBPDF2 in the kernel.
Is it really that onerous?


No.  And in fact if you read upthread you will notice that I provided a 
link to our implementation of both PBKDF1 & 2 and they're as small as 
you say.  They do everything we need.  So I'm right there with you.


But, PBKDF uses like 4K iterations (for WiFi passphrase -> key 
conversion for example) to arrive at its solution.  So you have 
implementations hammering the kernel with system calls.


So we can whinge at these implementations for 'being lazy', wring our 
hands, say how everything was just a big mistake.  Or maybe we can do 
something so that the kernel isn't hammered needlessly...


Regards,
-Denis

Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Ted,

On 05/24/2018 06:25 PM, Theodore Y. Ts'o wrote:

On Thu, May 24, 2018 at 05:08:41PM -0500, Denis Kenzior wrote:

Actually for the use case we have, speed is something pretty low on the
priority list; not having to deal with userspace crypto library dependencies
was a goal in and of itself.  Each one has its own issues and you can never
support just one.  Using AF_ALG has been rather... liberating.


Which is probably why Eric used the word, "laziness".  You might use a
different word, but the decisoin was one that was more driven by
convenience than kernel security


Err, this is a bit uncalled for.

But seriously, how is it a fault of the 'random person on the mailing 
list' that AF_ALG exists and is being used for its (seemingly intended) 
purpose?


I'm not really here to criticize or judge the past.  AF_ALG exists now. 
It is being used.  Can we just make it better?  Or are we going to 
whinge at every user that tries to use (and improve) kernel features 
that (some) people disagree with because it can 'compromise' kernel 
security?




Also, if speed isn't a worry, why not just a single software-only
implementation of SHA1, and be done with it?  It's what I did in
e2fsprogs for e4crypt.


If things were that simple, we would definitely not be having this 
exchange.  Lets just say we use just about every feature that crypto 
subsystem provides in some way.


Regards,
-Denis

Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Ted,

On 05/24/2018 04:05 PM, Theodore Y. Ts'o wrote:

Has anyone actually done the experiment and verified that it was in
fact a win to use AF_ALG on at least _some_ platform?  What about the
common cast for most platforms, even those that had some kind of
hardware accleration that could only be accessed by the kernel?

Actually for the use case we have, speed is something pretty low on the 
priority list; not having to deal with userspace crypto library 
dependencies was a goal in and of itself.  Each one has its own issues 
and you can never support just one.  Using AF_ALG has been rather... 
liberating.


Regards,
-Denis

Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Eric,


No, we don't add random code to the kernel just because people are lazy.  IMO it
was a mistake that AF_ALG allows access to software crypto implementations by
default (as opposed to just hardware crypto devices), but it's not an excuse to


I understand, but my point is, AF_ALG is out there now and hand-wringing 
about how it is a bad idea is sort of late / misplaced...



add random other stuff to the kernel.  The kernel runs in a privileged context
under special constraints, e.g. non-preemptible in some configurations, and any
bug can crash or lock up the system, leak data, or even allow elevation of
privilege.  We're already dealing with hundreds of bugs in the kernel found by
fuzzing [1], many of which no one feels very responsible for fixing.  In fact


I can only speak for myself here, but yes, this is understood.  And I 
still disagree with your assessment.



about 20 bugs were reported in AF_ALG as soon as definitions for AF_ALG were
added to syzkaller; at least a couple were very likely exploitable to gain


And I saw your contributions fly by to fix a lot of these issues.  Given 
that our project depends on and heavily uses AF_ALG, this is really much 
appreciated!



arbitrary kernel code execution.  The last thing we need is adding even more
code to the kernel just because people are too lazy to write userspace code.  Do
we need sys_leftpad() [2] next?


Well, I'm not sure where the laziness comment is coming from.  We have 
at least two user-space implementations that implement PBKDF on top of 
AF_ALG.  But a typical invocation of PBKDF runs a couple of thousand 
iterations.  That is a lot of system call overhead.  Would it not be 
better to fix things to be more efficient rather than worry about how 
'mistakes were made'?


And yes, people can always try to take things too far (as you point out 
in [2]), but I would argue this isn't such a bad one.


Regards,
-Denis

Re: PBKDF2 support in the linux kernel

[Denis Kenzior]

Hi Stephan,

On 05/24/2018 12:57 AM, Stephan Mueller wrote:

Am Donnerstag, 24. Mai 2018, 04:45:00 CEST schrieb Eric Biggers:

Hi Eric,



"Not having to rely on any third-party library" is not an excuse to add
random code to the kernel, which runs in a privileged context.  Please do
PBKDF2 in userspace instead.


I second that. Besides, if you really need to rely on the kernel crypto API to
do that because you do not want to add yet another crypto lib, libkcapi has a
PBKDF2 implementation that uses the kernel crypto API via AF_ALG. I.e. the
kernel crypto API is used and yet the PBKDF logic is in user space.

http://www.chronox.de/libkcapi.html



I actually don't see why we _shouldn't_ have PBKDF in the kernel.  We 
already have at least 2 user space libraries that implement it via 
AF_ALG.  ell does this as well:

https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/pkcs5.c

One can argue whether this is a good or bad idea, but the cat is out of 
the bag.


So from a practical perspective, would it not be better to make this an 
explicit kernel API and not have userspace hammer AF_ALG socket a few 
thousand times to do what it wants?


Regards,
-Denis

Re: [RFC PATCH 5/5] KEYS: add KPP ecdh parser

[Denis Kenzior]

Hi Tudor,

On 02/28/2018 10:52 AM, Tudor Ambarus wrote:

The ECDH private keys are expected to be encoded with the ecdh
helpers from kernel.

Use the ecdh helpers to check if the key is valid. If valid,
allocate a tfm and set the private key. There is a one-to-one
binding between the private key and the tfm. The tfm is allocated
once and used as many times as needed.

ECDH keys can be loaded like this:
 # echo -n 
020028000200200024d121ebe5cf2d83f6621b6e43843aa38be086c32019da92505303e1c0eab882
 \


This part looks a bit scary.  Isn't this translating directly to 
kpp_secret data structure (in looks like little-endian order) followed 
curve_id, etc.


If the intent is to extend KPP with regular DH, DH + KDF, etc, then we 
might want to invent a proper format here?  I don't think that a Diffie 
Hellman or ECDH Private Key format was ever invented, similar to how 
PKCS8 is used for RSA.


Inventing an ASN.1 syntax would be logical but somewhat painful as D-H 
is frequently used with plain old random numbers and certificates are 
not stored on disk...



 | xxd -r -p | keyctl padd asymmetric private @s

Signed-off-by: Tudor Ambarus 
---
  crypto/asymmetric_keys/Kconfig  |   8 +++
  crypto/asymmetric_keys/Makefile |   5 ++
  crypto/asymmetric_keys/kpp_parser.c | 124 
  include/crypto/asym_kpp_subtype.h   |   2 +
  4 files changed, 139 insertions(+)
  create mode 100644 crypto/asymmetric_keys/kpp_parser.c


Regards,
-Denis

Re: [RFC PATCH 1/5] KEYS: Provide key type operations for kpp ops

[Denis Kenzior]

   int (*asym_kpp_gen_pubkey)(struct kernel_kpp_params *params, void *out);
+   int (*asym_kpp_compute_ss)(struct kernel_kpp_params *params,
+  const void *in, void *out);
  
  	/* internal fields */

struct list_headlink;   /* link in types list */
diff --git a/include/linux/keyctl.h b/include/linux/keyctl.h
index e89b4a4..8f464ea 100644
--- a/include/linux/keyctl.h
+++ b/include/linux/keyctl.h
@@ -43,4 +43,15 @@ struct kernel_pkey_params {
enum kernel_pkey_operation op : 8;
  };
  
+struct kernel_kpp_query {

+   __u32   supported_ops;  /* Which ops are supported */
+   __u32   max_size;   /* Maximum size of the output buffer */
+};
+
+struct kernel_kpp_params {
+   struct key  *key;
+   __u32   in_len; /* Input data size */
+   __u32   out_len;/* Output buffer size */
+};
+
  #endif /* __LINUX_KEYCTL_H */
diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index d75d208..98b79f8 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -107,4 +107,7 @@ struct keyctl_pkey_params {
__u32   __spare[7];
  };
  
+#define KEYCTL_SUPPORTS_GEN_PUBKEY	0x0f


This looks fishy, the other KEYCTL_SUPPORTS_ block ends at 0x08.  This 
would imply that KPP supports encryption, decryption, sign, verify.  Do 
you mean 0x10 here?



+#define KEYCTL_SUPPORTS_COMPUTE_SS 0x10
+


And 0x20 here?


  #endif /*  _LINUX_KEYCTL_H */



Otherwise this looks good to me.  Feel free to add:

Reviewed-by: Denis Kenzior <denk...@gmail.com>

Regards,
-Denis

Re: [PATCH v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

[Denis Kenzior]

Hi Stephan,

>>

This brings me to another proposal for read buffer sizing: AF_ALG akcipher
can guarantee that partial reads (where the read buffer is shorter than
the output of the crypto op) will work using the same semantics as
SOCK_DGRAM/SOCK_SEQPACKET. With those sockets, as much data as will fit is
copied in to the read buffer and the remainder is discarded.

I realize there's a performance and memory tradeoff, since the crypto
algorithm needs a sufficiently large output buffer that would have to be
created by AF_ALG akcipher. The user could manage that tradeoff by
providing a larger buffer (typically key_size?) if it wants to avoid
allocating and copying intermediate buffers inside the kernel.


How shall the user know that something got truncated or that the kernel
created memory?



To the former point, recall the signature of recv:
ssize_t recv(int sockfd, void *buf, size_t len, int flags);

Traditionally, userspace apps can know that the buffer provided to recv 
was too small in two ways:


The return value from recv / recvmsg was >= len.

In the case of recvmsg, the MSG_TRUNC flag is set.

To quote man recv:

"All  three calls return the length of the message on successful comple‐
tion.  If a message is too long to fit in the supplied  buffer,  excess
bytes  may  be discarded depending on the type of socket the message is
received from."

and

"MSG_TRUNC (since Linux 2.2)

For   raw   (AF_PACKET),   Internet   datagram   (sinceLinux
2.4.27/2.6.8),  netlink  (since Linux 2.6.22), and UNIX datagram
(since Linux 3.4) sockets: return the real length of the  packet
or datagram, even when it was longer than the passed buffer.
"

Regards,
-Denis
--
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 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Denis Kenzior]

Hi Herbert,

On 06/22/2016 09:20 AM, Herbert Xu wrote:

On Wed, Jun 22, 2016 at 09:19:16AM -0500, Denis Kenzior wrote:

Just to clarify, we use this from userspace.  So we _already_ depend
on this functionality.  Please keep the hash and non-hash versions
of pkcs1pad available.


How can you be depending on this in userspace when we haven't
even exported akcipher to userspace? Colour me confused.



We live on the bleeding edge :)

I realize that these features are not upstream yet, but that doesn't 
mean that we can't influence / see the direction that the kernel is 
taking and act accordingly.


We'd like to have both pkcs1pad + hash, and simple pkcs1pad go upstream. 
 That will make our job in userspace much easier.  Andrew submitted 
pkcs1pad transform to the kernel specifically so we could get rid of 
this logic in our userspace code.  So please consider leaving both 
versions for upstream inclusion.


Regards,
-Denis

--
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 4/8] crypto: rsa-pkcs1pad - Require hash to be present

[Denis Kenzior]

Hi Herbert,

On 06/22/2016 09:02 AM, Herbert Xu wrote:

On Wed, Jun 22, 2016 at 03:20:51PM +0200, Andrzej Zaborowski wrote:


We use pkcs1pad with AF_ALG to implement lightweight TLS.  TLS
versions < 1.2 use a non-standard hash so we'd have to move the PKCS#1
padding back to userspace if this is changed.


When this is submitted for upstream inclusion we can add support
for it.



Just to clarify, we use this from userspace.  So we _already_ depend on 
this functionality.  Please keep the hash and non-hash versions of 
pkcs1pad available.


Regards,
-Denis

--
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