Add TCN validation to our Cloud Function

[madhavajay]

Our mobile apps generate cryptographic signatures and then upload these via a REST endpoint to our cloud function on firebase. The data is currently being written to the firestore but before we do that we want to verify the signature is correct.

The code is there and its setup so you can run the local firebase / firestore emulator on your machine. https://github.com/covid19risk/covidwatch-cloud-functions

Once the emulators are running you can open postman and make some requests. We are currently using the POST request to write data to the firestore.

in index.ts there is a line that says: // validate crypto

Thats where we need to do the crypto validation.

All of the data needed should be available at that point although you might need to change the formats around, we are receiving base64 and turning it into bytes.

This is the TCN protocol https://github.com/TCNCoalition/TCN

Basically we want to run the code in that Rust implementation to verify that the payload and signature match so that there has been no modification of the submitted data and that it is well formed.

The TCN rust implementation has been wrapped in a node module here, see the branch add-verification-function: https://github.com/covid19risk/tcn-node/tree/add-verification-function

It might not be complete yet so there could be some work to be done there.

I did the cloud function stuff so if you have questions ping me, The guy who did tcn-node is @miridius and you can message him directly on EU time if you want any help there.

[miridius]

Hey @madhavajay I've started working on this but I can't run build or test in the covidwatch-cloud-functions repo. I made an issue in that repo: https://github.com/covid19risk/covidwatch-cloud-functions/issues/3

[inmyth]

Hi. I've also been working on this and this is what I learned so far. I think verifying the digital signature is just matching the report and the decrypted signature. The payload already has everything:

report ← rvk || tck_{j1-1} || le_u16(j1) || le_u16(j2) || memo

{
  "temporary_contact_key_bytes": tck_{j1-1} (prolly used for recalculating tcn and tck)
  "memo_data": memo,
  "memo_type": {0,1,255},
  "start_index": j1,
  "end_index": j2,
  "signature_bytes": sig,
  "report_verification_public_key_bytes": rvk or public key
}

Only public key, sig, and report are needed for verification but the Rust lib already has Report and SignedReport that takes in all these fields and returns the verification result on self. This is where I'm stuck. I cannot figure out how to reconstruct a Report from Javascript side. I'd probably need to read up neon-binding but I would ask @miridius about it.

[miridius]

The add-verification-function branch has now been finished and merged to master (#6), so you can just work with master directly. The current version of the library is also published to npm so you can just npm install tcn-node

@inmyth this library currently uses neon-serde to convert a SignedReport to/from JavaScript objects, because this was the fastest path to a working implementation. Please see the README for an example. The interface is quite un-ergonomic but fixing that is on the todo list, if you have time you could work on that yourself by updating lib/index.js

[inmyth]

Thanks @miridius it works now. The rest of the validation on Node side I can complete. I think this library is great. I would probably just add on README a little bit about how to use validate, like getting R_bytes and s_bytes.

[madhavajay]

@miridius awesome work! @inmyth let me know if you need help with this.

[miridius]

@inmyth happy to hear that :) I didn't do any heavy lifting myself, this is only a wrapper of the TCN crate written in rust: https://docs.rs/crate/tcn/0.4.1

I forked that rust module and added json serialisation/deserialisation of all the internal structs, so that's why you see those not very descriptive field names. I definitely think we should improve the field naming and/or add more information to the README.

R_bytes and s_bytes are the fields from a ed25519-zebra::Signature struct. I don't know their meanings but based on the source of that library it looks like if you have a 64 byte signature then R_bytes are the first 32 and s_bytes are the last 32.

I noticed in the covidwatch-cloud-functions library you have the signature, rvk, etc. all as strings currently, but these fields have binary data. Are you receiving them base64 encoded or in some other format? The tcn code expects that as a Uint8Array so we would need some conversion.

[inmyth]

Yeah I didn't know anything about edDSA format either. Last night I randomly split the array and it worked. The format is base64 encoded but madhava already did the conversion.

@madhavajay thanks, for now it's just specification details. Do we need to validate the content such as rejecting invalid memo_type / allowing specific memo_type or do we just need to validate the crypto ?

@miridius There's one thing I forgot. Maybe we should expose the enum MemoType ?

[madhavajay]

@inmyth thats a good question, I think its totally valid to allow any Memo and Memo type as long as they fit in the spec since they were created to allow additional out of band messaging in the TCN protocol, so as long as the signature verifies then we are good to write to the firebase and if not we should reject with an error message. Are you able to create a PR so I can test it!? :)

[inmyth]

@madhavajay yes so there's a little bit ambiguity about Memo type as the doc moves faster than the code. The code only allows three values but the doc allows the whole range from 0 to 255. I'm working on it, hopefully I can do it today.

[madhavajay]

I would allow anything in the 255 range.

[miridius]

Hi @inmyth, I have added a new issue: https://github.com/covid19risk/tcn-node/issues/7 asking for feedback on how you would like to see this library's function signatures / types to look like so that they would be most convenient. Could you make a comment there?

FYI I would make those changes in a new version (0.4) so as long as you're depending on 0.3 you won't get any breakages to existing code