openwallet-foundation-labs / identity-credential

Apache License 2.0
157 stars 80 forks source link

No error code send to verifier during exception while decrypting request #115

Closed vaimut closed 1 year ago

vaimut commented 2 years ago

Expected Behavior

A response with appropriate error code should be send to verifier when an exception occurs while decrypting request.

According to document ISO 18013-5, section 9.1.1.4 Table 20(attached): An error code should be send when there is an exception while processing the request before terminating the session. But in the IC API when an exception occurred while decrypting a request(PresentationHelper->onMessageReceived()) there is no response with error codes send to the verifier. The DataTransport session is closed and an error message is send to the reportError method which gets communicated to the onError callback. When the implementing app tries to send the response with error code the below 2 issues blocks it.

  1. As only the message was sent its not feasible to identify the error code with error message in the onError callback.
  2. As the transport is closed its not possible to send the response even with some error code(hardcoded) In order to solve this a response with appropriate error code should be sent to the verifier before closing the transport.
    Also with this the error callback should be modified in a way to communicate the error with error codes.

Actual Behavior

No response with appropriate error code has been send to verifier during exception while decrypting request

Steps to Reproduce the Problem

There are more than 5 exception handling in PresentationHelper->onMessageReceived() method and all the cases should be covered. One such case is listed below While sending an encrypted device retrieval mdoc request to the mdoc in a SessionEstablishment message make sure the mdoc reader ephemeral public point is not on the curve

  1. Generate an mdoc reader ephemeral key pair, using the curve communicated by the mdoc in the device engagement structure
  2. Add 1 to the value of the y-coordinate of the resulting public point, and verify that the new point is not on the curve. If it is, add 1 again.
  3. Send the request.

Specifications

Screenshot 2022-08-30 at 7 40 39 PM
davidz25 commented 2 years ago

Yeah, I think right now we probably just close the connection with proper session termination on all error paths. Would be nice to fix this... that said... in general an mdoc reader need to handle the case where the mdoc just closes the connection w/o proper session termination ... so I don't think this is very urgent to fix.

davidz25 commented 1 year ago

https://github.com/google/identity-credential/pull/194 fixes this issue

vaimut commented 1 year ago

While testing this bug we observed that the app is crashing. Debugging it we found that the UL suite send an tampered request by changing the Y point in the public key. While IC API evaluates this request crash happens as the point is not in curve. This crash occurs in ensureSessionEncryption -> Util.coseKeyDecode(). As the method ensureSessionEncryption is not called within try block the crash is navigating to app which crash the app. Moving this call inside try catch will fix the crash and will send response with 10.

Tampered request: A26A655265616465724B6579D818584BA401022001215820513BB24EC15E1BFEBE47901B1C562E5245CAB394CF776A924814E6C4325F826D2258201924129F7E649D7116683E69DC1EB920A86765AF49A8DEF9E0EEBE444AF2FF5464646174615901233A221CB16FB7F7D1232811FB703F81785F5196EA62A830F0CD994B410C3A67DB76F59946FD54492051B468D346306279F34EAD3D7B265010E1553B58973D679D23E6AA0FB9A03415054B8DAD6D8DF4106C22B172BA8AC14F2F4636ECC99AD24A9227F6DB18F70C3F45838FFFF28AFB2E50407B1F99FE46E3F61FADC1F13F1312751AAFC9BBBC7DF49B68CFFAF13F9DD12C6DB81D999A68B42AEEF020B95F10560FE4EBCD17AAFCBF5172513C5B82E16887DB8141ACA39E3BAB15A5ED99B9AE0BCE643355AA7947094FB3933EE2C3814C73D2A155A18F9EABFCD00D7EE746E2200EADA565D4F1D7EFD523E4E3EDD98B742A2DA540E387C15745D4BE628216378118B2573DFE042776927C76A42C2EAFAA9B77113166A6309E812308F2BC0948CBB16C88

115_Crash.txt

vaimut commented 1 year ago

Also the library need to send status code of 11 if there are any CBOR encoding issue. As of now only status code of 10 is send(Which is also not working).

 

MicrosoftTeams-image
vaimut commented 1 year ago

try{

ensureSessionEncryption(data);

}catch(IllegalStateExceptione){

mTransport.sendMessage(SessionEncryptionDevice.encodeStatusToReader(

Constants.SESSION_DATA_STATUS_ERROR_SESSION_ENCRYPTION));

mTransport.close();

reportError(newError("ErrordecodingEReaderKeyinSessionEstablishment",e));

return;

}

The above code is not capturing all exception the method throws. Internal logics of method "ensureSessionEncryption()" throws "IllegalArgumentException" as well which is however not cached in catch block. This cause the app to crash still.