| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
The stock assert() implementation turns out to be problematic in the
stm32 environment, due to the lack of an output device, which makes
"assert(foo)" equivalent to "if (!foo) abort()", leading to silent
hangs.
We probably ought to reimplement assert() to do something more useful,
but, for now, avoid using it for "impossible" conditions which we do
seem to be triggering anyway, like the occasional point-not-on-curve
errors we get for points we ourselves have picked when testing
multiple ECDSA clients in parallel. This should never happen, and we
need to figure out what's causing it, but hanging the HSM when it
happens does not help very much.
assert() is somewhat problematic in an embedded environment in any
case, since anything that can go wrong really should have some kind of
recovery action, but in some of the low-probability cases it's far
from obvious what sane recovery action we could possibly take.
|
|
|
|
|
|
libhal and PKCS #11 have slightly different models of private keys: in
libhal, a "private key" object is really a keypair, while in PKCS #11
a private key really is a naked private key. This was a deliberate
design decision in libhal, both for simplicity and to better support
user interfaces other than PKCS #11, so we'd rather not change it.
This difference doesn't matter very much for RSA keys in PKCS #11,
where the private key components are a superset of the public key
components anyway, but the PKCS #11 template for ECDSA private keys
doesn't allow setting public key components with C_CreateObject().
Fortunately, computing the public components of an ECDSA key pair from
the private key is straightforward, so we just do that when needed.
|
|
|
|
|
|
|
|
Merge Paul's review comments.
|
|
|
|
Log exceptions immediately when failing a test; doesn't replace
backtrace at end of test run, but since a full test run can take a
while it's useful to know what failed closer to when it happened.
More conditionals to skip tests which require external Python crypto
packages when those packages aren't installed.
|
|
|
|
|
|
|
|
|
|
Probably reached the point of diminishing returns for trying to get
probing to work better. Best option, where practical, is to avoid
probing completely; when necessary, best run it once then avoid
repeating it. cryptech_muxd will probe if requested, but probing is
never going to be reliable. Dedicated VID:PID would be much better.
|
|
|
|
|
|
|
|
Internal probe code mostly works, probably about as well as
cryptech_probe ever did, but almost certainly needs timeouts around
its I/O calls, and perhaps additional error handling. At the moment,
it works except when it doesn't, just like cryptech_probe.
Some day we'll get away from this kludge, but not today.
|
|
|
|
Renamed multiplexer to cryptech_muxd, since it now handles both RPC and CTY.
Added new program cryptech_console to act as client for CTY multiplexer.
Might want to add console logging capability eventually, not today.
Probably want to incorporate UART probing (what cryptech_probe does
now) eventually, also not today.
|
|
|
|
This is a work in progress, so far it only works with libhal.py
client, haven't adapted libhal C client code for this yet.
General idea is to let PySerial and Tornado handle all the pesky work
of dealing with Unix serial ports, PF_LOCAL connection management,
select() vs epoll() vs kqueue() vs ... I/O management, etcetera.
We could do this with just PySerial and the Python standard libraries,
but using Tornado allows us to do it in a single process, without
threading, and also leaves the door open for consolidating other HSM
management functions (eg, console access) into the same single daemon
process.
For the moment we're using SLIP framing over a SOCK_STREAM connection,
which is a bit silly but avoids the problem of OSX not supporting
SOCK_SEQPACKET. In the long run we're going to want a real channel
security protocol here, so don't sweat this too much right now.
|
|
|
|
|
|
|
|
Fixed handling of deletion actions: code was still using the
zero-length attribute convention instead of HAL_PKEY_ATTRIBUTE_NIL.
Track existing attributes more closely while copying data from old
chunks to new ones in the slow path: the old algorithm had a few
dangerous corner cases which could have resulted in the wrong values
being written to the new chunks.
Single-block-update fast path now under compile-time conditional; in
the long run, we probably want this enabled, but it's disabled for now
in order to force use and testing of the slow path.
This function probably needs to be broken up into a collection of
smaller inline functions for readability.
|
|
PKCS #11 supports zero-length attributes (eg, CKA_LABEL) so hack of
using zero length attribute as NIL value won't work, instead we use a
slightly more portable version of the hack PKCS #11 uses (PKCS #11
stuffs -1 into a CK_ULONG, we stuff 0xFFFFFFFF into a uint32_t).
ks_attribute.c code was trying too hard and tripping over its own
socks. Instead of trying to maintain attributes[] in place during
modification, we now perform the minimum necessary change then re-scan
the block. This is (very slightly) slower but more robust, both
because the scan code has better error checking and because it's the
scan code that we want to be sure is happy before committing a change.
Rename hal_rpc_pkey_attribute_t to hal_pkey_attribute_t.
|
|
|
|
|
|
Calling hal_rpc_pkey_get_attributes() with attribute_buffer_len = 0
now changes the return behavior so that it reports the lengths of
attributes listed in the query, with a length of zero for attributes
not present at all. This is mostly to support C_GetAttributeValue()
in PKCS #11, but we also use it to make the Python interface a bit
kinder to the user.
|
|
Wiping the keystore flash requires reinitializing the keystore, but we
don't want to allocate new static memory when we do this.
|
|
The current pkey access control rules are a bit complex, because they
need to support the somewhat complex rules required by PKCS #11. This
is fine, as far as it goes, but a strict interpretation leaves
HAL_USER_NORMAL as the only user able to see many keys. This is
confusing when using the CLI, to put it mildly.
HAL_USER_WHEEL is intended for exactly this sort of thing: it's a user
ID which, by definition, can never appear in an RPC call from PKCS
to see the same keys that HAL_USER_NORMAL would.
HAL_USER_SO remains restricted per the PKCS #11 rules.
|
|
hal_rpc_pkey_list() was a simplistic solution that worked when the
keystore only supported a handful of keys and we needed a quick
temporary solution in time for a workshop. It doesn't handle large
numbers of keys well, and while we could fix that, all of its
functionality is now available via more robust API functions, so
simplifying the API by deleting it seems best.
Since this change required mucking with dispatch vectors yet again, it
converts them to use C99 "designated initializer" syntax.
|
|
|
|
This check made sense when attribute deletion was a separate
operation, but now that it has been folded into set_attributes(), this
check was worse than useless.
|
|
|
|
pkey attribute API is now just set_attributes() and get_attributes().
|
|
Passes minimal unit-testing and the same minimal tests report that it
does deliver the desired performance speed-up. More testing and much
cleanup still needed.
Attribute API not quite stable yet, we're probably going to want to
remove all the singleton attribute operations from the RPC protocol,
and it turns out that ks_delete_attributes() has enough code in common
with ks_set_attributes() that it makes more sense to handle the former
as a special case of the latter.
|
|
This is not yet complete, only the ks_volatile driver supports it,
ks_flash will be a bit more complicated and isn't written yet.
At the moment, this adds a complete duplicate set of
{set,get,delete}_attributes() functions in parallel to the earlier
{set,get,delete}_attribute() functions. We will almost certainly want
to get rid of the duplicates, probably (but not necessarily) the
entire single-attribute suite. At the moment, though, we want both
sets so we can compare execution speeds of the two sets of functions.
|
|
|
|
|
|
Result of running all these timing tests is about what theory would
have predicted: read time does not vary significantly between RAM and
flash, but write time sure does.
|
|
|
|
|
|
|
|
Watching the pinwheel makes it clear that the painfully slow execution
of test_attribute_bloat_flash_many() isn't a single hidously long
delay anywhere, rather it's a long steady stream of slow operations
and it's the cumulative time that's hurting us. Most likely this is
entirely dominated by flash write time, and suggests that it may be
worth the additional API and implementation complexity to handle
setting a complete set of attributes in a single operation, so that we
only have to pay the flash write toll once.
Will probably require further testing before we can make an informed
decision.
|
