privacybydesign / irmago

IRMA server, client, and tooling. Documentation: http://irma.app/docs
Apache License 2.0
70 stars 48 forks source link

Validation of signature fails after new optional attribute #35

Closed stevenvegt closed 5 years ago

stevenvegt commented 5 years ago

The following signature was valid:

Contract json: ````json "signature": [ { "c": "7IBqXjMivYL6SSr1v/TOpYG+3GXZMuMejsCIAtd6Xp4=", "A": "ljVcut65H/UTeIUHjpXEcENX1rwU0BwvaJj2Z5fTw8i3QKnzCXTsaCskA3OsdjfIU5E0xOhI8rwZnLkbd2wZ1EwrWlL0vBLqh6m8DVfUn02czr2Fx9tWY6grc2trALy0ROqM4U/pp1wBlXV7m4CcslNwFWkvO35BXECic5PHfI2LUU88DnN0jLKOHmeu60TOJqS94PxVs7Bt3A5sC7hOi7dB1DIhJJmIMyxcLSB8IP1fTucIYiK1n0rd7rYG5BgivEHGbvg27AmgrpI3Z3zbnpmycuSLN80gARx8ozIvIIS/pdw5XieTkFbxL/hJTvymbz+LVTX3eKVBQXCmXXBnYw==", "e_response": "Tht2LdtAmJH9kCuzNdW5PfgNqdna/iyy0xMcQVguvrmqhtMGhNE9aAZYL/BZlcFikz0xECRIkZXsLuKHmo7+", "v_response": "BqryxHSYLIax24I3ZT2OSXQjab8zY8I19qIUx2m7cooEjlkFBQlJxR76y48MMNt6CExMxiZaw5ygEK7sQQlMyDv4BcIpCHxn9Iu8ZuHM4DqzDMxNed2iPyqiVjjxf3uDlY/aJ0dYKLe0oYZGQ11KPAV0uz5uGqEQN9Q8kdE7ZzxPyEiUZX9HkJJQ0HB4UkKoz3jbM51DdpW4V9dmxQaO/gSP2sUWfDrUkMnfBuAdImyQ7y4g8ZWYfNNCvdUvaaWuqo27X9e9DlgVVfbWf/tUsL0EemkMW4XtB8ql371qys2HRoNxAm6uAVzZrn1EHzuPSWXLsSluUYO1JS9fEq+5EZSzSqHPuqqmxmKMU2jAe3PYtZJphp0hFxinOARTspParaN3a18Rvw68iC5gdfK6+9qZlYeRSrOfU4IvEKugeOKatcRD46tUbGO+hiYblwSInvzvRYAXx+TT2rXTtTk6ez5wK2zgajzIzSfSXHTvQTjXEd+1gSIFtuvyw8yzRqEEXnVUAyly54uE4DwuedDsQEk=", "a_responses": { "0": "kd6cCLk/IQ52zyVgJu9M9DMYiGwEQhSB6Rj80Asm55LAqsOj0AYb+LGWpf/RiSaz/vJEbfUBbJjZqBGjgbLUTkmzaht9QiXg7Ro=" }, "a_disclosed": { "1": "AwAJ8gAaAABH2jklUts5iBWSlKLhMjvi", "2": "YGBgYGBgYGM=" } } ], "indices": [ [ { "cred": 0, "attr": 2 } ] ], "nonce": "pGW5aB1YNp8cADoDa2KXtQ==", "context": "AQ==", "message": "NL:BehandelaarLogin:v1 Ondergetekende geeft toestemming aan Helder om uit zijn/haar naam het nuts netwerk te bevragen. Deze toestemming is geldig van Wednesday, 03-Apr-19 16:36:06 CEST tot Wednesday, 03-Apr-19 17:36:06 CEST.", "timestamp": { "Time": 1554302366, "ServerUrl": "https://metrics.privacybydesign.foundation/atum", "Sig": { "Alg": "ed25519", "Data": "7lX0cWof1dnkccBZ4QA7vz4W1JOxkPhxhj9ldzN2fuDCywR8Uk+WxPt5PuT8Rl5GO1uchky8BxKfYSi6RCfhAQ==", "PublicKey": "e/nMAJF7nwrvNZRpuJljNpRx+CsT7caaXyn9OX683R8=" } } } ````

But after adding this role attribute: https://github.com/privacybydesign/irma-demo-schememanager/commit/211d4a2ede7b1ca7ea30698e2f7c401a90f07901

The contract validation fails with a INVALID_TIMESTAMP status.

sietseringers commented 5 years ago

The problem is that the timestamp signature signs not only the timestamp, but (1) also the disclosed attributes, and (2) for the undisclosed attributes the fact that they are undisclosed (where (un)disclosed means (not) attached to this signature). So this happens:

This mismatch results in INVALID_TIMESTAMP. The problem is thus that the timestamp verification algorithm does not take into account that at creation time, the 2nd attribute did not yet exist.

This could be fixed in two different ways, of which I'm not yet sure which is best:

  1. keep track of when new attributes were added to credential types in the scheme, so this can be taken into account when verifying the timestamp,
  2. taking the disclosed and undisclosed attributes into account in a wholly different way at creation and verification time of the timestamp, that will automatically be compatible with later extensions of the credential type with more attributes. This seems like a more thorough fix but would not solve the problem for this particular signature, or others coming from the same credential type. Seeing as you're using irma-demo here, and your production credential type does not yet have the extra attribute and thus this problem, can I assume this is not a problem for you?
stevenvegt commented 5 years ago

The second options seems the best one for me too. We do not have any contracts outside test and demo context. I would say that IRMA contracts are currently in their alpha phase and would give priority to elegant future proof solutions even if that means breaking the older test/demo contracts.

sietseringers commented 5 years ago

This is now fixed in version 0.3.0 of the irma binary and the current beta IRMA app (using the second option).