Implement large blobs #332
Conversation
msirringhaus
force-pushed
the
largeblobs
branch
from
7fe68dd to
30a9f76
Compare
msirringhaus
force-pushed
the
largeblobs
branch
3 times, most recently
from
3599d70 to
77572e2
Compare
jschanck
left a comment
jschanck
left a comment
There was a problem hiding this comment.
Sorry for the wait on this one. Just a few comments below.
| where | ||
| Dev: FidoDevice, | ||
| { | ||
| if input.is_empty() { |
There was a problem hiding this comment.
This should also error out if the input is length 1, otherwise we'll panic on &input[1..] below.
src/ctap2/commands/large_blobs.rs
Outdated
|
|
||
| let byte_len = large_blob.len() as u64; | ||
| let large_blob_array: Vec<LargeBlobArrayElement> = | ||
| from_slice(large_blob).unwrap(); |
There was a problem hiding this comment.
We don't want to panic if the authenticator returns malformed data.
src/ctap2/commands/large_blobs.rs
Outdated
| .get_authenticator_info() | ||
| .and_then(|i| i.max_msg_size) | ||
| .unwrap_or(1024) | ||
| - 64; |
There was a problem hiding this comment.
Is there a guarantee that max_msg_size is greater than 64? We would need to error out if it is.
There was a problem hiding this comment.
I interpreted the spec as "it has to be 1024 or bigger", but the way it is worded does indeed allow for smaller max_msg_sizes. (Plus, we may just as well guard against non-compliant tokens)
Likewise, because some authenticators are memory constrained, the maximum message size supported by an authenticator MAY be limited. By default, authenticators MUST support messages of at least 1024 bytes. Authenticators MAY declare a different maximum message size supported using the maxMsgSize authenticatorGetInfo result parameter.
I'll add a check and error out if it is smaller or equal to 64
src/ctap2/commands/large_blobs.rs
Outdated
| .get_authenticator_info() | ||
| .and_then(|i| i.max_msg_size) | ||
| .unwrap_or(1024) | ||
| - 64; |
There was a problem hiding this comment.
As above, we need a consistency check here.
msirringhaus
force-pushed
the
largeblobs
branch
from
77572e2 to
ba0539b
Compare
|
Well, this was perfect timing if there ever was one. After all this time, |
2 participants
Implement basic writing and reading of large blob arrays associated with credentials.
Not yet supported: Stand-alone functions for updating (although that would be just reading, then writing), and Garbage collection.
Since we probably won't use this in Firefox right away, I have moved all operations that require more dependencies outside of the core and into the examples. In theory, we could all of that internally, but then we'd need AEAD-GCM and DEFLATE in the core.