Ensure RPC communication doesn't leak server-side errors

Description

When an exception is raised on server side, our RPC communication sometimes reports an error due to the fact that the exception on server side is not whitelisted for serialisation.

This is wrong on several levels:

1. This is leaking information which should remain on the server side.
2. Exceptions shouldn't be serialised, but communicated.

Instead, catch any exceptions raised (directly or through observables), log them, and create an RpcException containing a generic error message e.g., "Something went wrong in the Corda node.".
In case we sometimes use exceptions to signal something specific went wrong to the client side, pass the message (not the stacktrace) to the RpcException just for these special ones (an interface might help).

Example:

Id Flow name Initiator Status
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
cf880868-93cc-49a6-bea5-c30c8e281 Cash Issue corda Result(stx=SignedTransaction(id=4D1946F8923119BEB7B8C2DA2C106BE455C
af048eef-856f-4397-8fe1-434a3b249 Cash Exit corda In progress
Waiting for completion or Ctrl-C ...
Observable completed with an error
flow watch: exception: Class org.hibernate.exception.GenericJDBCException is not annotated or on the whitelist, so cannot be used in serialization
Serialization trace:
exception (net.corda.core.utilities.Try$Failure)
result (net.corda.core.messaging.StateMachineUpdate$Removed)
value (rx.Notification)
Wed Mar 21 13:20:05 GMT 2018>>>

Activity

Show:
Gavin Thomas
March 26, 2018, 4:43 PM

Also check whether this exists in Corda 3.0

Katelyn Baker
April 11, 2018, 10:34 AM

This had been backported to the 3.1 RC. However, much discussion around this and a regression (CORDA-1313) meant we ultimately decided to back it out of that release rather than hold it and await a fix.

It looks like this need a rethink / some more work

Hence the issue is being reopened. I'm assigning it to so he can add his comments on this

Katelyn Baker
April 11, 2018, 10:36 AM

Removing fixed version Corda 3.1 as we reverted that commit

James Brown
May 11, 2018, 3:30 PM

for reference:

https://www.owasp.org/index.php/Improper_Error_Handling

"Improper handling of errors can introduce a variety of security problems for a web site. The most common problem is when detailed internal error messages such as stack traces, database dumps, and error codes are displayed to the user (hacker). These messages reveal implementation details that should never be revealed. Such details can provide hackers important clues on potential flaws in the site and such messages are also disturbing to normal users."

Michele Sollecito
May 15, 2018, 10:16 AM

As per Slack discussion with , and :

  • No stacktraces in any circumstances.

  • Exceptions are let through only if the exception is white-listed (with stacktrace removed anyway).

  • If the exception is not white-listed, a generic exception with a generic error message is sent instead.

  • As part of this, a new ContractVerificationException : IllegalArgumentException will be thrown when verifying contracts (at the moment, they throw IllegalArgumentException).

Assignee

Michele Sollecito

Reporter

Michele Sollecito

Labels

Sprint

None

Epic Link

None

Priority

Medium

Severity

Medium

CVSS Score

None

CVSS Vector

None

Due Date

None

Engineering Teams

None

Fix versions

None

Affects versions

None

Ported to...

None

Story Points / Dev Days

None
Configure