fdo-rs / fido-device-onboard-rs

An implementation of the FIDO Device Onboard (FDO) spec written in Rust.
BSD 3-Clause "New" or "Revised" License
56 stars 31 forks source link

Wrong serialization of error messages. #620

Closed mmartinv closed 4 months ago

mmartinv commented 5 months ago

It looks like the error messages are not being serialized correctly.

From the conformance server I can see the following error message as the response for a bad formatted request (in hex format):

a56a6572726f725f636f646518647570726576696f75735f6d6573736167655f74797065146c6572726f725f737472696e67784553657269616c697a6174696f6e206572726f723a20696e76616c696420747970653a20696e7465676572206030602c206578706563746564207374727563742048656c6c6f6f6572726f725f74696d657374616d70f66a6572726f725f757569641b55413d158cba1b93

The conformance server fails parsing the above data with:

cbor: cannot unmarshal map into Go value of type fdoshared.FdoError (cannot decode CBOR map to struct with toarray option)

Using the playground site (http://cbor.me ) to decode the content by pasting it in the Bytes text box (on the right side) and clicking on the green arrow above that text box (convert to diagnostic notation), the following is shown:

According to the FIDO specification the error message definition seems to be an array of elements:

ErrorMessage = [
    EMErrorCode: uint16,   ;; Error code
    EMPrevMsgID: uint8,    ;; Message ID of the previous message
    EMErrorStr:  tstr,     ;; Error string
    EMErrorTs:   timestamp,;; UTC timestamp
    EMErrorCID: correlationId ;; Unique id associated with this request
]
timestamp = null / UTCStr / UTCInt / TIME_T
UTCStr = #6.0(tstr)
UTCInt = #6.1(uint)
TIMET  = #6.1(uint)
correlationId = uint

Therefore, the conformance server error would make sense as we are formatting the Error as a CBOR map instead of an CBOR array.

nullr0ute commented 5 months ago

It would be worth while looking at git history here, I seem to remember there was previous back and forth around cbor serialisation, could also be a bug/change in rust dependencies.