[prev in list] [next in list] [prev in thread] [next in thread] 

List:       opensc-commits
Subject:    [Opensc-commits] [OpenSC/libp11] 9f1bb3: Simplify pkcs11_try_pkey_rsa_sign() to not hold lo...
From:       Timo Teräs via Opensc-commits <opensc-commits () lists ! sourceforg
Date:       2021-04-11 20:31:11
Message-ID: OpenSC/libp11/push/refs/heads/master/13f616-5125b7 () github ! com
[Download RAW message or body]

  Branch: refs/heads/master
  Home:   https://github.com/OpenSC/libp11
  Commit: 9f1bb3803980a149acab4e8a54894ed5ff343236
      https://github.com/OpenSC/libp11/commit/9f1bb3803980a149acab4e8a54894ed5ff343236
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/p11_pkey.c

  Log Message:
  -----------
  Simplify pkcs11_try_pkey_rsa_sign() to not hold lock over calls

The long term lock keeping was originally added in commit e81e3355
"NULL sig support #178" to support querying the size of the signature
with sig=NULL. However, this commit was immediately followed up
by 7a1fca41 "EVP_PKEY_FLAG_AUTOARGLEN for EVP_PKEY_meth_new()" which
refers to same issue too.

The EVP_PKEY_FLAG_AUTOARGLEN makes OpenSSL core handle sig=NULL
case before calling the algorithm specific sign function. Thus we
never get the sig=NULL call in the current code. Thus the original
hack is unneeded.

This effectively reverts e81e3355 and adds an error handling if
sig=NULL would happen.


  Commit: 586cd12d14f2cbda451c56094d6a4f5dc87ab03c
      https://github.com/OpenSC/libp11/commit/586cd12d14f2cbda451c56094d6a4f5dc87ab03c
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/p11_load.c
    M src/p11_pkey.c

  Log Message:
  -----------
  Simplify pkcs11_try_pkey_rsa_decrypt() to not hold lock over calls

This was originally added in commit 8356d568 "Add support for RSA-OAEP
and RSA-PKCS encryption for PIV and HSM tokens" which just cloned the
same pattern from pkcs11_try_pkey_rsa_sign(). Remove it as unneeded
for the same reason: OpenSSL core handles the special case for us.


  Commit: f0c2ac1ef24753b0ef9624a6a5324f619c93537d
      https://github.com/OpenSC/libp11/commit/f0c2ac1ef24753b0ef9624a6a5324f619c93537d
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/p11_load.c
    M src/p11_pkey.c

  Log Message:
  -----------
  Simplify pkcs11_try_pkey_ec_sign() to not hold lock over calls

This was added in 592b71ae "Add EC signing through EVP api" and it
just seems to have followed the pattern set by pkcs11_try_pkey_rsa_sign().

In fact here the code never worked correctly, because the *siglen
is tested early with "if (*siglen < (size_t)ECDSA_size(eckey))" which
breaks the size inquiry. Often *siglen would be uninitialized, or
initialized to zero causing failure.

This adds the proper code used by OpenSSL core to do the size
inquiry, and removes the stateful handling of lock.


  Commit: 2cb52a2f5a763d4f5be2c1eeb3e48674819e5db4
      https://github.com/OpenSC/libp11/commit/2cb52a2f5a763d4f5be2c1eeb3e48674819e5db4
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/p11_atfork.c

  Log Message:
  -----------
  Remove legacy cruft from atfork code

This removes the __sun cruft which is never used. The #pragma
init(lib_init) makes a function named "lib_init" an initializer,
but we don't have such function. This is likely copy-paste cruft
from where this code was taken from.

Remove also the usage of "inline" and related checks. They add
no value in .c file as the defined functions are used. Normally
"inline" is used in header inline function definitions to remove
compiler warning of unused function (as the function might not
be used in all the C files including the header).


  Commit: 5df3b1bdb8ac98fce5d04293a79ad78d863f228e
      https://github.com/OpenSC/libp11/commit/5df3b1bdb8ac98fce5d04293a79ad78d863f228e
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M configure.ac
    A m4/ax_pthread.m4
    M src/p11_atfork.c

  Log Message:
  -----------
  Use pthread_atfork instead of __register_atfork

Modify build system to detect pthreads, and use pthread_atfork
for fork handling if available. It is conforming to POSIX.1-2001
and available widely instead of the non-standard __register_atfork.

This is especially useful on musl c-library which does not ship
the non-standard variant. Using the atfork callbacks is prefered
as the last resort alternative adds a syscall, getpid(), to many
fast path places.


  Commit: 079b9cf3a0badfd350f847a914a17ed11cd59423
      https://github.com/OpenSC/libp11/commit/079b9cf3a0badfd350f847a914a17ed11cd59423
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/p11_atfork.c

  Log Message:
  -----------
  Refactor atfork logic to elide locking on fast path

The atfork infrastructure is modified so that on Windows it
optimizes practically to nothing.

Otherwise the forkid is kept in a global variable, which is
updated exactly once per check round (to minimize syscalls).

A fast path to avoid locking is added. When the atfork check
is done, it is guaranteed that the P11_forkid does not change
(it can change only for the after-fork child process).

Only if a fork is detected, the locks are taken. Each object's
forkid is again checked in each of the check_*_int calls, so they
get updated only once even if the child is multithreaded.

This already improves performance measurably, but is even more
important to avoid lock contention after session pooling support
is added.


  Commit: d28387ae41b171def20b9e2bbaa20b29df67e727
      https://github.com/OpenSC/libp11/commit/d28387ae41b171def20b9e2bbaa20b29df67e727
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/libp11.h
    M src/p11_atfork.c
    M src/p11_ckr.c
    M src/p11_front.c
    M src/p11_slot.c

  Log Message:
  -----------
  Remove complexity from slot reinitialization after fork

After fork the context handler will reset the PKCS#11 module by
calling C_Initialize. After this all handles from the module should
be considered invalidated.

This combines the session opening and logging after fork to single
function, and removes the relogin flag from functions where it's
not needed. The new pkcs11_reload_slot() properly update the state
so the normal functions operate as expected.

This also fixes a memory leak after fork: if the slot was in
logged-in state, a session was leaked from check_slot_fork_int because:
 1. the "if loggedIn" clears state, and calls pkcs11_relogin() which
    also implicitly opens a session
 2. the next "if haveSession" block fires also, and clears state,
    and calls pkcs11_reopen_session. This function will explicitly
    call C_OpenSession overwriting and leaking the session from step #1


  Commit: 5ffebc4b52bc88237733c9f94fd32a9de64e4377
      https://github.com/OpenSC/libp11/commit/5ffebc4b52bc88237733c9f94fd32a9de64e4377
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/libp11.h
    M src/p11_atfork.c
    M src/p11_cert.c
    M src/p11_ckr.c

  Log Message:
  -----------
  Implement atfork handling for certificates

This removes a search operation from pkcs11_remove_certificate()
and simplifies it greatly. This makes also the handling of certificates
similar to other objects, as only this was lacking the fork handling.

Also fixed the memory leaks on error paths of pkcs11_remove_certificate()
in case the certificate search failed. The equivalent code in
pkcs11_reload_certificate() was refactored to always free allocated
resources.


  Commit: 4bd92da60a670e3c25d9cb69fca3363f47a225c7
      https://github.com/OpenSC/libp11/commit/4bd92da60a670e3c25d9cb69fca3363f47a225c7
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M README.md
    M make.rules.mak
    M src/libp11-int.h
    M src/libp11.h
    M src/p11_attr.c
    M src/p11_cert.c
    M src/p11_ckr.c
    M src/p11_ec.c
    M src/p11_key.c
    M src/p11_pkey.c
    A src/p11_pthread.h
    M src/p11_rsa.c
    M src/p11_slot.c

  Log Message:
  -----------
  Refactor internal code to use session pooling

This gets rid of locking in the crypto operation paths, and allows
concurrent use of multiple threads by making sure that each session
is not used by any two threads.

Additionally this fixes cases where it was possible that the PKCS#11
was called from different threads with the one per-slot session.

The default session pool size is selected as 16, but it is dynamically
shrunk if the maximum supported session count is reached. Further
knobs to tune the session pool size can be added later.

The pool is implemented with mutex+condition mechanism, and thus
a simple wrapper to implement pthreads API on Windows Vista and
later is added. This bumps the Windows requirement to a bit higher,
but considering Vista is already EOL, this should be acceptable.

A FIFO style queue was chosen to support even load-balancing
between sessions. This is helps pkcs#11 libraries which in turn
are load-balancing sessions to different units in a cluster.


  Commit: dcfc9cf7ecbaf4eaacb3ccdd8a9b74a69be93f79
      https://github.com/OpenSC/libp11/commit/dcfc9cf7ecbaf4eaacb3ccdd8a9b74a69be93f79
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/p11_atfork.c
    M src/p11_load.c

  Log Message:
  -----------
  Replace the context rwlock with a mutex

The only remaining user for this is the after-fork code, so rename
the lock also as fork_lock.


  Commit: aaccad6e116acf0109a9a3f473704379070dad53
      https://github.com/OpenSC/libp11/commit/aaccad6e116acf0109a9a3f473704379070dad53
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/eng_back.c
    M src/libp11-int.h
    M src/p11_misc.c

  Log Message:
  -----------
  Convert and fix engine context locking

Optimize also ctx_init_libp11 to elide locking if the one time
intialization is completed already.

Basic locking to key and certificate loading is added, because
the underlying functions they use are not thread safe.

Also remove the old compat functions which are no longer needed.

add simple locking


  Commit: 5125b7834e2b14c47e915faa8231f07ac67bcf61
      https://github.com/OpenSC/libp11/commit/5125b7834e2b14c47e915faa8231f07ac67bcf61
  Author: Timo Teräs <timo.teras@iki.fi>
  Date:   2021-04-11 (Sun, 11 Apr 2021)

  Changed paths:
    M src/libp11-int.h
    M src/p11_attr.c
    M src/p11_cert.c
    M src/p11_ec.c
    M src/p11_key.c
    M src/p11_rsa.c

  Log Message:
  -----------
  Refactor attribute getting to include session handle

This avoids one thread to get two sessions from the pool, and
speeds up operation as locking is not needed to get the attribute.


Compare: https://github.com/OpenSC/libp11/compare/13f61606fabb...5125b7834e2b


_______________________________________________
Opensc-commits mailing list
Opensc-commits@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensc-commits

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic