The response to get OpenSSH to work is non-compliant
(because it's trying to read a 2.1 command set out of a 2.0
device). This will lead to breakage in the 2.0 test bench,
but, it will allow the device to work with OpenSSH.
meh. specs, schmecs.
read_to_end() "guesses" the total amount of data to read by doing
roughly 5 calls per couple hundred bytes: it starts by guessing
it's 32 bytes long, then another 32, then 64, then 128, and then
a final call to make sure there's nothing more.
The read() call is somewhat expensive in that it involves initializing
and sending page-sized memory chunks, and every time it also
descends the entire basis hierarchy, recomputing the overlay.
Probably, this can be improved with some sort of server-side caching
of read results -- if a file is read, it seems very likely that
the next few bytes will be read and a smarter caching implementation
could smooth over some of the expense.
However, a much easier fix for now is to just change the read
call to read exactly the length of the file being presented. Since
we already have the key handle, we can get the attributes and then
do a single read of the exact amount.
Doing this improves the speed of the UX substantially, about 40%
faster for the same operation (at least).
In retrospect forcing apps to manage their suspend in resume
crosses too many abstraction barriers.
Instead, the app should specify the mode of the hardware, but
then the driver should be responsible for restoring that state
on resume, not the app.
perfomance regression on the PDDB with the latest fixes make
the device stop passing compliance when the database grows
to about 40-ish keys. For now, we set the cred limit to 32,
until the performance issues are addressed. (note that yubico
has a limit of 25 credentials, so, 32 is not insanely low).
The behavior with a secret basis is as follows:
- The key stays in the basis in which it was created.
- So, if you created k1 in .System basis, and k2 in Secret basis,
if you edit k1 while Secret basis is open, k1 stays in .System
basis. it does *not* migrate into Secret basis.
I think this is the "least surprising" behavior. There is an
argument that "maybe I want to keep secret notes in an overlay
on a key" but I feel like creating a second shadow copy of a key
with a different set of notes is not the most desired behavior
for this application (NB: it could be desired for others applications,
and that behavior can be programmed with the PDDB API; which is
why the API is a bit explicit at the moment. This flexibility goes
against the goal of "foolproof deniability", unfortunately, but
I think it is too early to know which default to set for the system).
These adjustments ideally make the system less confusing to use in
the face of secret Bases, but the spec calls for some information that
could be a leakage channel (see below).
- items that are assumed to be consistent regardless of the basis
are now forced into the System basis. This reduces potentially confusing
behavior if the system has han unlocked secret basis and things like
PIN hashes are set which appear to be global.
- There is a "global signature counter" which could leak some information
about how many credentials are in the device. However, the FIDO2 spec
enforces some plausible deniability by requiring that the counter increments
by a random value instead of by 1. In the OpenSK implementation, this increments
by a random integer from 1-8. So, if you store dozens of credentials in secret
bases, it may become less deniable due to this global signature counter.
The global signature counter seems optional, so maybe that is a feature to
be stripped out...
this checks to make sure that the system is in a usable state
for the vault to run before allowing it to be launched.
in particular, the PDDB must be initialized, and so must the
system time.
- test entries for passwords at 100 entries. it's...pretty slow
at 300 passwords, about 8 seconds to scan/read-in the database.
once the database is read-in, sorting and viewing is fast. good
news is that heap usage is about 300k for that many entries,
no OOM crashes found. just slow.
- fix an unimplemented()! error
- strip out some debug
originally we stuck it in the CRED_DICT because it forces its
creation -- but there's no reason to *really* do that. We just
set the dictionary to be created if it's not existent in the later
API calls.
By moving this to FIDO_DICT instead of FIDO_CRED_DICT it makes
key access & sorting much cleaner in the UX, because you don't
have this odd key hanging out, and it also means that you can't
have an attack on the system where you try to name a key with
the name held in FIDO_CRED_DICT to try and read it out.
