Flight Log JSON Stringification before signing

[abrbhat]

Right now, json_dumps introduces spaces while stringifying log before signing (https://github.com/iSPIRT/NPNT-Provisional-testing-tool/blob/master/helpers.py#L163).

An example output of such stringification is: '{"PermissionArtefact": "Permission artefact ID of the current flightlog", "previous_log_hash": "hash of the most recent flight log", "LogEntries": [{"Entry_type": "TAKEOFF/ARM", "TimeStamp": 41, "Longitude": 380.000000, "Latitude": 443.5000000, "Altitude": 704.3400, "CRC": 719}, {"Entry_type": "GEOFENCE_BREACH", "TimeStamp": 505, "Longitude": 11, "Latitude": 701, "Altitude": 7, "CRC": 637}, {"Entry_type": "TIME_BREACH", "TimeStamp": 81, "Longitude": 116.5, "Latitude": 996, "Altitude": 560, "CRC": 147}, {"Entry_type": "LAND/DISARM", "TimeStamp": 81, "Longitude": 116.5, "Latitude": 996, "Altitude": 560, "CRC": 147}]}'

The spaces are generally unexpected in stringified JSON and make it difficult validate the signature. We should use compact JSON encoding. Example from https://docs.python.org/2/library/json.html

>>> import json
>>> json.dumps([1,2,3,{'4': 5, '6': 7}], separators=(',',':'))
'[1,2,3,{"4":5,"6":7}]'

@nihalMM @sidd-shetty @sidhantgoel

[abrbhat]

Maybe we should agree upon JSON canonicalization rules since we are stuck with using JSON for now.

[abrbhat]

Another issue is that serializing data results in losing trailing zeros. https://github.com/rjsf-team/react-jsonschema-form/issues/433#issuecomment-279338672 Therefore

{"Entry_type": "TAKEOFF/ARM", "TimeStamp": 41, "Longitude": 380.000000, "Latitude": 443.5000000, "Altitude": 704.3400, "CRC": 719}

will get converted to

{"Entry_type": "TAKEOFF/ARM", "TimeStamp": 41, "Longitude": 380, "Latitude": 443.5, "Altitude": 704.34, "CRC": 719}

which then again causes problems while signing and verifying.

[abrbhat]

For most languages 1 == 1.00 returns true. Checked with javascript, python and java. Therefore, the best option seems to be to use strings for decimal point numbers.

[mnihalm]

The Trailing zeros was fixed in #20

The whitespace bug should be fixed as well.

[mnihalm]

closed by #25

[abrbhat]

20 does not fix the trailing zeroes issue. The issue is that if the field is a number, it should not have trailing zeroes. Otherwise these zeroes will be lost while parsing this JSON wherever it is consumed and the signature will not get validated. Another way to fix this for now is to represent all floating-point numbers as strings.

[mnihalm]

Fields with trailing zeros are currently not lost while parsing the json and validating the signature. can you please help me understand where the issue is? This fixed the issue where if there were any trailing zeros in the json supplied, it is not lost while generating signature. That was an issue some users were facing. If somebody is generating the flight log with trailing zeros and signing the json with the zeroes included, then the tool should account for that. That is being accounted for.

If you are saying that the number should not have trailing zeroes in the first place, and that we use floats, that becomes a spec change and should be respectively addressed in the digital sky spec. If we are to do that, as @sidhantgoel recommends, we should move away from json to a more strictly encoded format for logs.

[abrbhat]

Yeah I am talking about the issue with schema itself. Eventually we should definitely move away from JSON. I am just exploring if we can do small fixes to the JSON schema itself for the short-term since we don't know when we will be able to shift away from JSON. Is that ok? @sidhantgoel @sidd-shetty

[sidhantgoel]

Such data types (that preserves trailing zeroes in a float) do not exist in most programming languages and fixing it in a test tool doesn't serve any purpose other than testing something that other programming languages cannot do. IMO fixing canonicalization rules for floating-point is also not a good idea it will just make it more complicated. Making it string seems to be a good solution to patch current implementations but I think we should move away from JSON and we have to do it anyway and sooner we do it manufacturers will have plenty of time to make it right? what say @abrbhat @mnihalm @sidd-shetty @bugobliterator

[sidhantgoel]

Created #28 to discuss the possibilities of moving to ASN.1 with DER encoding.