Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Json submission set option #30

Merged
merged 5 commits into from
Jul 30, 2024
Merged

Conversation

ritikarawlani
Copy link
Contributor

📑 Description

Chapter 37

  • Updated table 37.2.1
  • Added JSON SubmissionSet Signature Option

Chapter 5.10

  • Added 5.10.3.5 JSON SubmissionSet Signature Option
    • removed specifics of Hash value 0 but didn't add anything more specific about hashing, expecting JAdES guidance to be followed
  • Edited 5.10.3 to ObjectIyURIHash

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed
  • I have selected a committee co-chair to review the PR

ℹ Additional Information

@litlfred
Copy link

I think we should provide more detail here:
https://github.com/IHE/ITI.DSGj/pull/30/files#diff-fd3dcdaef6b0234698d224ee3132e04de8987a04f5dfeb52198271064136b491R198
on the data schema encoding uniqueID, particularly as we should require base64 encoding it (who knows that is in the uniqueID).

Probably need to add to the list of normative standards:
  https://datatracker.ietf.org/doc/html/rfc2397

Here:
  https://github.com/IHE/ITI.DSGj/pull/30/files#diff-fd3dcdaef6b0234698d224ee3132e04de8987a04f5dfeb52198271064136b491R212
do we want to say that the he SubmissionSet.uniqueId is included in the manifest via the data URI

@JohnMoehrke
Copy link
Contributor

We know what is in the document uniqueId... it is highly profiled by IHE
https://profiles.ihe.net/ITI/TF/Volume3/ch-4.2.html#4.2.3.2.26

I would rather use an encoding of that that we already use, rather than create a new one with base64 encoding of it.

The SubmissionSet uniqueId is even more tightly defined
https://profiles.ihe.net/ITI/TF/Volume3/ch-4.2.html#4.2.3.3.12

if we are saying data URL, why are we saying that? We have plenty of current explanation on how to take an OID and make it a URI. We should not be sending our readers off to other documents for no good reason.
https://profiles.ihe.net/ITI/TF/Volume3/ch-4.2.html#4.2.3.1.7

That said, I would not be against being more clear. Having examples is a great way to be clear. Having base64 encoding in your examples is a really bad way to be more clear.

@litlfred
Copy link

Thanks @JohnMoehrke  for the references.   My comments were in regards to the discussion we had at FTF on having algorithmic and standard way to replace the '0' hash value associated to a unqiueID.  

In the case of DSGj, @ritikarawlani  mentioned that according to the ETSI spec, the hashV SHALL be the hased value that is referenced by the corresponding URI in the pars array.  

In the case of the XML signature, the ds:reference element is supposed to be a URI.

From your reference:

     Document creators should use one of the following formats for the uniqueId:

          * OID in dot notation (see OID in Table 4.2.3.1.7-2).

          * UUID URN-encoded (see UUID in Table 4.2.3.1.7-2).

          * URI (see URI in Table 4.2.3.1.7-2).

           * For documents using HL7v3 Instance Identifiers (e.g., CDAs) with an extension attribute, the uniqueId should be a serialization of the root and extension attributes in the form root^extension. The HL7v3 Instance Identifier URN encoding (using the namespace urn:hl7ii) should not be used.

For the second and third bullets, as urn:uuid and urn:oid: are URIs and the character set is constrained we are fine and agree we don't need that data schema URI.

For the first bullet, I supposed we could provide guidance that if given an OID as UniqueID then you should represent it is an urn:oid.

For the fourth bullet, I don't know what we would want to do there to make it a valid URI.  The ^ is not a valid character in URI.   In this case, one option could be the data URI schema and then we need to encode it as base64 or another is to do it as a percent encoding.

@JohnMoehrke
Copy link
Contributor

So, they all have URI encodings. Even the v3 with an extension just understood that the ^ must be url escaped. Note that this ^ problem has not been pointed out before now, so that should likely go in more generally as a comment. That said, it is very unusual now days, so likely a non issue. You should report it.

@JohnMoehrke
Copy link
Contributor

doesn't this though just address how the id values are represented?

This is not associated with the zero hash problem... Why are we trying to solve this zero hash problem? zero is a number. and there is no solution to getting a proper hash. It seems your approach of indicating that the hash is not calculated on the content of the SubmissionSet, but rather across the uniqueId of the submissionset seems more complex than just saying that it is understood that the hash will fail to validate and that the application layer understands that this will happen. Which is what XML-Signature effectively does, although it does not explain it that bluntly.

Copy link
Contributor

@JohnMoehrke JohnMoehrke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems okay

@litlfred
Copy link

doesn't this though just address how the id values are represented?

This is not associated with the zero hash problem... Why are we trying to solve this zero hash problem? zero is a number. and there is no solution to getting a proper hash. It seems your approach of indicating that the hash is not calculated on the content of the SubmissionSet, but rather across the uniqueId of the submissionset seems more complex than just saying that it is understood that the hash will fail to validate and that the application layer understands that this will happen. Which is what XML-Signature effectively does, although it does not explain it that bluntly.

well the issue is not just that the zero seems funny, but more that the application layer would need to special case this that seems off. specifically in the ETSI spec they say:
 

The hashV member shall be a non-empty array of strings. Each element of the array shall contain:

  1. The base64url-encoded digest value of the data object referenced by the parameter value that is present in the same position of the pars array if the b64 header parameter is present and set to "false". 2) The base64url-encoded digest value of the base64url-encoded data object referenced by the parameter value that is present in the same position of the pars array if the b64 header parameter is absent or it is present and set to "true".
     

so if we are using the zero, we are breaking the ETSI spec.

Actually, because the ETSI spec says we need to dereference the URI (and presumably something similar on the XML side), I don't know that the fact that the uniqueID is already an URI (in most cases) really matters - what does it mean exactly to "dereference" a urn:uuid or a urn:oid (unless we say that is a tautology).    Dereferencing a data URI makes sense to me in that you are resolving that URI to get the underlying payload.

@JohnMoehrke
Copy link
Contributor

so in the case of SubmissionSet signature, which is a specific option... it is very clear that there is no intention for the signature to be fully valid. The option is defined as not having a valid signature for the SubmissionSet object itself.

Might we think a bit different, can we add an extension to the signed header that just carries this URI? This would have been the way I would have defined it originally, as it is clearly not been signed, but is important metadata for the use-case.

@litlfred
Copy link

@JohnMoehrke  I checked the (normative reference for DSG XML singatures) XML signature spec , and section 2.1.1 says:
 

   [s05-08] This identification, along with the transforms, is a description provided by the signer on how they obtained the signed data object in the form it was digested (i.e. the digested content). The verifier may obtain the digested content in another method so long as the digest verifies**.** In particular, the verifier may obtain the content from a different location such as a local store than that specified in the URI.

I would interpret that as meaning the zero hash in invalid and so the current IHE DSG XML is not compliant with xmldsig-core

@JohnMoehrke
Copy link
Contributor

@JohnMoehrke  I checked the (normative reference for DSG XML singatures) XML signature spec , and section 2.1.1 says:  

[s05-08] This identification, along with the transforms, is a description provided by the signer on how they obtained the signed data object in the form it was digested (i.e. the digested content). The verifier may obtain the digested content in another method so long as the digest verifies**.** In particular, the verifier may obtain the content from a different location such as a local store than that specified in the URI.

I would interpret that as meaning the zero hash in invalid and so the current IHE DSG XML is not compliant with xmldsig-core

You might be right. But that is not what we are working on. Those that want this option are not complaining.

Can we make this URI a part of the metadata of the json signature?

@litlfred
Copy link

litlfred commented Jul 26, 2024 via email

<li>The signature document would be added to the SubmissionSet according to <a href="5.10.6">Section 5.10.6</a>. The SubmissionSet may, but is not required, include all the “SIGNS” association defined in <a href="5.10.6.4">Section 5.10.6.4</a> with associations to all the other documents in the SubmissionSet. The “SIGNS” association is redundant in this case as the SubmissionSet already groups these documents.</li>
<li>The SubmissionSet with the (n) documents and the Digital Signature document is submitted using the Provide and Register Document Set-b [ITI-41] transaction, or equivalent from the other Document Sharing infrastructures.</li>
</ol>
<h5 id="5.10.3.5.1">5.10.3.5.1 "SSId" (SubmissionSet uniqueId) Header Parameter</h4>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new sub section added @JohnMoehrke @litlfred

@ritikarawlani
Copy link
Contributor Author

ritikarawlani commented Jul 29, 2024

@litlfred @JohnMoehrke Made updates. I have proposed a new extension header parameter "SSId" that can contain the SubmissionSet Unique ID in the oid format when SubmissionSet option is used. Since OIDs don't have the ^ , I didn't propose any additional encoding. This will be in the protected header so will follow JWS specification for encodings.

We will also have to write to IANA JOSE Registry group to register the new parameter and they will review it for conflicts, appropriateness etc. They have a TAT of 21 days for the request. Below is the request I formatted:

Header Parameter Name: SSId
Header Parameter Description: The SSId header parameter's value shall specify the SubmissionSet.uniqueId as per the https://profiles.ihe.net/ITI/TF/Volume3/ch-4.2.html#4.2.3.3.12.
Header Parameter Usage Location: JWS
Change Controller: IHE ITI
Specification document: https://profiles.ihe.net/ITI/DSGj/Volume3/ch-5.10.html#5.10 

Before sending the request forward to IANA, it would be nice to get confirmation from you both regarding the proposed name. Also we would need the final URL for the specification document to be sent to IANA.

The one consideration I think for the name is whether we would like to add any IHE ITI prefixes to the name. For example, ETSI has added "ETSIU" as a parameter which stands for ETSI Unprotected header.

We will also need to create new JSON Schema file which includes this new parameter specification. I have added the mark tag for this relevant text in the html

@JohnMoehrke
Copy link
Contributor

This formality of registering the extension seems very unusual. Especially for json, where schema is such a joke. But, we should follow our peer standards rules.

I would prefix it with IHE. I don't think it is important to include the ITI.

I like this solution much better.

@litlfred
Copy link

litlfred commented Jul 29, 2024 via email

@ritikarawlani
Copy link
Contributor Author

Made a final commit -

  1. Added open issue to address the need for a JSON schema file
  2. added prefix of ihe, so iheSSId is the new param name
  3. Typo fixes

Request sent to IANA to register the parameter

I think we're ready to merge. @JohnMoehrke @litlfred

@JohnMoehrke
Copy link
Contributor

I have approved, so you are clear to merge.

@ritikarawlani ritikarawlani merged commit 9e05ccb into main Jul 30, 2024
@JohnMoehrke JohnMoehrke deleted the json-submission-set-option branch October 25, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants