Age | Commit message (Collapse) | Author |
|
|
|
Support for variable-length keystore objects significantly complicates
the keystore implementation, including serious some serious code bloat
and a complex recovery algorithm to deal with crashes or loss of power
at exactly the wrong time. Perhaps we don't really need this?
So this is an experiment to see whether we can replace variable-length
keystore objects with fixed-length, perhaps with a compile time option
to let us make the fixed object length be 8192 bytes instead of 4096
bytes when needed to hold things like large RSA keys.
First pass on this is just throwing away nearly 1,000 lines of
excessively complex code. The result probably won't even compile yet,
but it's already significantly easier to read.
|
|
|
|
|
|
|
|
|
|
Turns out there are a couple of known minor bugs in PyCrypto's ASN.1
decoder, simple dumb things that never could have worked. Debian's
packaging includes a patch for these bugs, but for some reason the
patch is marked as not needing to be sent upstream, dunno why. So
these methods work fine, but only on Debian. Feh.
Simplest approach is to work around the bugs on all platforms,
particularly given that this is just unit test support code.
|
|
|
|
|
|
|
|
|
|
|
|
Consistent user complaints about HSM login taking too long.
Underlying issue has both superficial and fundamental causes.
Superficial: Our PBKDF2 implementation is slow. We could almost
certainly make it faster by taking advantage of partial
pre-calculation (see notes in code) and by reenabling use of FPGA hash
cores when when checking passwords (which mgiht require linking the
bootloader against a separate libhal build to avoid chicken-and-egg
problem of needing FPGA to log into console to configure FPGA).
Fundamental: The PBKDF2 iteration counts we used to use (10,000
minimum, 20,000 default) are in line with current NIST
recommendations. The new, faster values (1,000 and 2,000,
respectively) are not, or, rather, they're in line with what NIST
recommended a decade ago. Well, OK, maybe the Coretex M4 is so slow
that it's living in the past, but still. The fundamental issue is
that anybody who can capture the encoded PIN can mount an offline
dictionary attack on it, so we'd like to make that expensive.
But the users are unhappy with the current behavior, so this change
falls back to the ancient technique of adding a delay (currently five
seconds, configurable at compile time) after a bad PIN, which makes it
painful to use the login function as an oracle but does nothing about
the offline dictionary attack problem.
Feh.
Note that users can still choose a higher iteration count, by setting
the iteration count via the console. It's just not the default out of
the box anymore.
|
|
|
|
|
|
I doubt this change will have any noticable effect, but it's another
theoretical race condition, might as well eliminate it.
|
|
Static code analysis (Doxygen call graph) detected a low-probability
race condition which could have triggered a deadlock on the keystore
mutex if the mkmif code returns with an error like HAL_ERROR_CORE_BUSY
when we're trying to fetch the KEK.
This is a knock-on effect of the awful kludge of backing up the KEK in
the keystore flash as an alternative to powering the MKM with a
battery as called for in the design. This code path should not exist
at all, but, for now, we avoid the deadlock by making it the caller's
responsibility to grab the keystore mutex before looking up the KEK.
|
|
|
|
|
|
|
|
success, but reduces the failure rate on a busy server.
|
|
|
|
conditions.
This manifested as hal_aes_keyunwrap() returning HAL_ERROR_CORE_BUSY, but
getting reported as HAL_OK, which led to HAL_ERROR_ASN1_PARSE_FAILED
when trying to parse the not-unwrapped der.
|
|
We're using non-blocking I/O in any case, might as well take advantage
of it to keep console output a little smoother.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
pkey_open() now looks in both keystores rather than requiring the user
to know. The chance of collision with randomly-generated UUID is low
enough that we really ought to be able to present a single namespace.
So now we do.
pkey_match() now takes a couple of extra arguments which allow a
single search to cover both keystores, as well as matching for
specific key flags. The former interface was pretty much useless for
anything involving flags, and required the user to issue a separate
call for each keystore.
User wheel is now exempt from the per-session key lookup constraints,
Whether this is a good idea or not is an interesting question, but the
whole PKCS #11 derived per-session key thing is weird to begin with,
and having keystore listings on the console deliberately ignore
session keys was just too confusing.
|
|
|
|
|
|
Enforce minimum PKCS #1.5 padding length when decrypting KEK.
Use public interface to hal_pkey_load() rather than calling the
internal function directly, so we go through all the normal error
checks.
|
|
Now that we use PKCS #8 format for private keys, all key formats we
use include ASN.1 AlgorithmIdentifier field describing the key, so
specifying key type and curve as arguments to hal_rpc_pkey_load() is
neither necessary nor particularly useful.
|
|
|
|
|
|
Borrowing an idea from PyCrypto, we substitute CSPRNG output for the
value of a decrypted KEK if the PKCS #1.5 type 02 block format check
fails. Done properly, this should be very close to constant-time, and
should make it harder to use hal_rpc_pkey_import() as an oracle.
|
|
In the long run this code will fork for two different purposes:
a) A user backup script, which need not handle ASN.1 or crypto, thus
can (and should) be really simple; and
b) Unit test code for the key export and import functions.
So far, hal_rpc_pkey_export() appears to be working, at least under
light testing (result of unpacking everything looks like a key,
anyway, haven't tested the unpacked key yet).
Still need to write test code for hal_rpc_pkey_import().
|
|
Among other things, it turns out that this works better if one
remembers to write the RPC server dispatch code as well as the client
code, doh.
|
|
|
|
|
|
Still missing Python script to drive backup process, and need to do
something about setting the EXPORTABLE key flag for this to be useful.
|
|
Handle AlgorithmIdentifier.parameters as in SubjectPublicKeyInfo: the
field is OPTIONAL, but it's usually set to NULL if no OID is present.
I have a vague memory that this is fallout from a specification error
years ago in which the OPTIONAL was accidently left out. Whatever.
|
|
|