hyperledger / fabric-private-chaincode

FPC enables Confidential Chaincode Execution for Hyperledger Fabric using Intel SGX.
Apache License 2.0
159 stars 91 forks source link

refactor shim and chaincode interface #83

Closed mbrandenburger closed 4 years ago

mbrandenburger commented 5 years ago

the methods provided by shim.h should be revisited. Error handling is missing, for instance, getState does not return an error when a integrity violation is detected.

mbrandenburger commented 5 years ago

logging should be revisited as well #88

mbrandenburger commented 5 years ago

we also should refactor the chaincode interface, for instance, returning the arguments already as array instead of forcing the chaincode to unmarshal the args using json.

invoke(const char *args, uint8_t *response, uint32_t max_response_len, uint32_t *actual_response_len, void *ctx);
g2flyer commented 5 years ago

We should also clean up shim.h with internal functions not targeted to chaincode developers (e.g., all the rwset function and types). They should be moved out of the shim to a separate header-file such that shim.h is exactly and only the complete external programming model.

g2flyer commented 5 years ago

If we have unmarshall* functions for inputs in shim.h, shouldn't we have corresponding marshall* functions for results? Seems strange to have only one but not the other ...

g2flyer commented 5 years ago

More long-term, but we probably also want to have some put/get_public_state function in shim.h. We've discussed earlier with the reasoning that this might be a way to allow for broadcasting decisions to the public (such as auction results) and long-term evidence of outcomes even when the enclaves might have "died". As it seems though there is no easy way to extract info from the ledger other than a query to the chaincode, it's maybe less clear how benificial it really would be in practise ..

If we think we want it, we could maybe change the auction example to put the final result as such a public value.

g2flyer commented 5 years ago

Shouldn't shim.h also contain some form of interface definition of functions which the chaincode has to implement, i.e.,


int invoke(const char* args,
    uint8_t* response,
    uint32_t max_response_len,
    uint32_t* actual_response_len,
    void* ctx)
{
```?
g2flyer commented 5 years ago

If we want enable client authentication based on MSP, we might also want to add a corresponding API function.

For sdkization release we could just pass go's shim.GetCreator() as-is (and unverified) to the application. For an eventual implementation, we of course would then also (in the enclave) have to verify the proposal to really get the proper authentication but at least in current version the shim implementation should get the signed proposal, so it could pass it along to the enclave to do the work. For TLCC we already have to deal with all of MSP, so this function shouldn't necessarily add any new code complexity related to that.

If we decide we want this, we also should extend the auction example with such "authenticated" identities rather than have, as now, the bidder_name as an (inherently untrusted) transaction argument. Enhanced correspondingly, it would be probably also best example to use for #129 ?

g2flyer commented 5 years ago

eventually we want to benefit also from fabric private data collection.

They definitely can give a performance boost when dealing with large data as the data doesn't have to go all to the orderer. Enabling this, though, can be done under the cover and does not have to be exposed in the API.

Less clear is whether there are some scenario where we want to expose it to the developer the {Get,Put}PrivateData* which are in the normal Go Shim (https://godoc.org/github.com/hyperledger/fabric/core/chaincode/shim). In their case of course it might be necessary due to some data being shared with chaincodes which are not part of the collection but for us it really shouldn't make much of a security difference (the only difference i can see that we would hide access patterns from the orderers. However, peers are much more likely to be access-pattern-attackers as they have inherently more vested interest as CC participants and also do understand much more about the application (hence been better able to exploit the side-channel). Insofar, it doesn't seem to me worth to expose it, at least until wee have a concrete use-case requiring it?

Maybe iff we add a PutPublic* it might be slightly different as we would then have a somewhat semi-private setting (though we should then probably call it PutPublicPrivate or PutSemiPublic rather than the PutPrivateData ...).

g2flyer commented 5 years ago

Much of above is addressed in PR #137. (Potentially) remaining shim-related issues/additions after PR#137 are as follows (see also the various TODOs in shim.h):

=> investigation of issue #150 should answer that question whether we need them or not

g2flyer commented 4 years ago

Close as remaining issues are delegated to issue #88 and #150