RalphTro / epcis-event-hash-generator

ALGORITHM and SOFTWARE PROTOTYPE to uniquely identify/validate the integrity of any EPCIS event through a common, syntax-agnostic approach based on hashing. Takes an EPCIS Document (formatted in either XML or JSON-LD) and returns the corresponding hash value(s).
MIT License
8 stars 4 forks source link

Disregard errorDeclaration element content when calculating the pre-hash string #53

Closed RalphTro closed 1 year ago

RalphTro commented 3 years ago

As discussed in https://github.com/gs1/EPCIS/issues/226#issuecomment-814054029 and in the EPCIS standard 2.0 draft, the eventID of an error declaration event needs to be identical to the original one.

Thus, depending on the outcome of the EPCIS group discussion, I think we can ignore all data as part of the error declaration event, i.e. not concatenate them to the pre-hash string.

The only downside I see is that, in case of an error declararion event, the event hash id is not a true fingerprint of an EPCIS event anymore, i.e. declarationTime, error reason and user extensions could be tampered with.

@Echsecutor : let's discuss this when we have our next call on this!

Echsecutor commented 3 years ago

I would be in favour of keeping the error declaration. As you said, in this case there is no need to calculate an ID anyway, but having a unique hash could still be useful for other purposes (e.g. notarisation).

:+1: for letś discuss ;)

dakbhavesh commented 3 years ago

Hi @RalphTro and @Echsecutor, I think we will need a separate routine or a flag in the existing routine to decide whether to disregard or consider the errorDeclaration element. If errorDeclaration event is missing eventID field then EPCIS 2.0 implementation can request for event hash to match the corresponding erroneous event. If one wants to generate hash for notarization purposes then it will also be possible.

What do you think? Is there any work in progress in regard to this issue?

RalphTro commented 3 years ago

That sounds to be a reasonable suggestion, @dakbhavesh! For the moment, I think it is sufficient to have this documented as an issue. There are anyway a couple of further tweaks necessary to bring our prototype algorithm in line with the current state of discussion (e.g. additional properties in sensorReport such as exception, handling <extension> tags for EPCIS 1.x events vs. 2.0 events (for which you are currently kindly writing the XSLT), non-prefixed CBV vocabulary elements in JSON/JSON-LD, etc.). I suggest to wait at least until the two drafts are motioned to public ommunity review (so that we have a suffiently stable basis), then formulate all outstanding ToDos as issues, and work on them afterwards. How does that sound?

dakbhavesh commented 3 years ago

Dear @RalphTro, It completely makes sense to wait until EPCIS 2.0 draft is motioned for public community review. Thanks!

RalphTro commented 3 years ago

Another way out/option in addition to (a) remove it or (b) to implement a separate routine my consist in flagging/marking errorDeclaration in the property order of this algorithm as a distinction to what is specified in the CBV.

We just needed to add some explanation that for an error declaration event, users must not calculate an event hash ID (just take the eventID value of the original (i.e. erroneous event) - BUT IF users want to notarise such an event, they of course should take the content of the errorDeclaration element into consideration.

IMO, that would be a feasable approach as in the EPCIS 2.0 standard, the major purpose of this is to uniqely identify a given event - use cases such as notarisation go beyond that.

RalphTro commented 1 year ago

Update Still open + TBD. Right now, the canonical property order, in contrast to what is specified in the CBV standard, still accommodates the errorDeclaration element:

...
 ('eventTimeZoneOffset', None),
    ('errorDeclaration',
     [
         ('declarationTime', None),
         ('reason', None),
         ('correctiveEventIDs', [('correctiveEventID', None)])
     ]),
    ('parentID', None),
...
RalphTro commented 1 year ago

TBD. Altogether disregard. Not add to user extension. Add some language, also in the CBV standard!

RalphTro commented 1 year ago

Agreement:

RalphTro commented 1 year ago

Note that we already have examples accommodating an errorDeclaration field

RalphTro commented 1 year ago

Thanks to @dakbhavesh 's fix (see PR exclude-errordeclaration-in-hashcalculation), we can close this issue.