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

show signatures graphically irrespective of their cryptographic validation status #13058

Closed
ralfhauser opened this issue Mar 7, 2021 · 7 comments

Comments

@ralfhauser
Copy link

@luk-lo 's recommendation #10507 (comment) (also in brokermint#1) worked nicely for example in version pdfjs-2_4_456-es5-dist

Unfortunately, in pdfjs-2.6.347-es5-dist and pdfjs-2.7.570-es5-dist , the data.fieldValue remains empty
(in the attached example
minimum-sig7.pdf
the fieldValue of obj with id "28R" (signature annotation) is "27R" with 2.4.456, in the newer code this remains null ;(

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Mar 7, 2021

Signatures are currently not supported, since they cannot be verified, which means that any "hack" to attempt to display them are also (obviously) unsupported; hence this is a duplicate of issue #1076.


the data.fieldValue remains empty

It needs to be empty, given that signatures are currently not parsed, as explained by this comment:

pdf.js/src/core/annotation.js

Lines 1051 to 1055 in 709c57a

// Hide signatures because we cannot validate them, and unset the fieldValue
// since it's (most likely) a `Dict` which is non-serializable and will thus
// cause errors when sending annotations to the main-thread (issue 10347).
if (data.fieldType === "Sig") {
data.fieldValue = null;

@ralfhauser
Copy link
Author

ralfhauser commented Mar 7, 2021

Signatures are currently not supported, since they cannot be verified, which means that any "hack" to attempt to display them are also (obviously) unsupported; hence this is a duplicate of issue #1076.

the data.fieldValue remains empty

It needs to be empty, given that signatures are currently not parsed, ...

Just strange that the signatures were correctly parsed in v2.4.456-es5-dist and now they are no longer , which is really sad.

as explained by this comment:
...

As the title of my issue suggests, I have seen this comment before.

... "hack" to attempt to display ...

Looking at the number issues raised around this, it seems to be need for many

and in our empirical experience, the "attempt to display" is successful almost all the times - we never got any complaints.

But we got immediately lots of complaints once we upgraded beyond v2.4.456-es5-dist

@Snuffleupagus
Copy link
Collaborator

Just strange that the signatures were correctly parsed in v2.4.456-es5-dist and now they are no longer , which is really sad.

If it "worked" previously, that was a bug rather a feature given that we've never actually supported signatures. Hence this should never have worked in the first place, and consequently it's not really a regression.

Looking at the number issues raised around this, it seems to be need for many

Yes, but validating signatures is no simple task, which is why it's not yet implemented.

the "attempt to display" is successful almost all the times

There's a very real risk in displaying unverified signatures, since users could then be fooled by invalid ones, which is why just displaying them is not a good idea and why we explicitly hide signatures (for now).

@ralfhauser
Copy link
Author

There's a very real risk in displaying unverified signatures, since users could then be fooled by invalid ones, which is why just displaying them is not a good idea and why we explicitly hide signatures (for now).

There are some users out there who _do_know_what_they_do and still think that the graphical display of unverified signature is a worthwhile action, therefore I suggest:

a) not to display them by default
b) but have a switch where it can be configured to be displayed
c) add some disclaimers (e.g. mouse-over ?) that the display doesn't imply it being valid

@timvandermeij
Copy link
Contributor

Closing since we're not going to do this until the signatures can be verified, which is tracked in #1076 but is non-trivial. If this "worked" before, it's indeed a bug and there is simply no guarantee that it will remain to work since it's clearly unsupported. If you want to display them anyway, that's entirely your responsibility and up to you to build. We're not going to provide a means to display signatures that might very well be invalid since that defeats the whole purpose of digital signatures.

@ralfhauser
Copy link
Author

ralfhauser commented Mar 7, 2021

display signatures that might very well be invalid since that defeats the whole purpose of digital signatures.

"Always being valid" is not_at_all the purpose of digital signatures.

The first purpose of digital signatures is to give a mean to

  • find out whether the content is still the same as it was at the origin. And therefore, it is very OK, to also show a signature that proves that this is not the case and that the content has been altered (some do it with a red X vs. green check).
  • furthermore, many jurisdictions have come up with multiple levels of assurance / "expression of will" (qualified signatures, etc.) as an additional dimension on top of the pure integrity ==> therefore signature validity often is not a binary or boolean topic, but way more involved.

Thus, as for suggestion c) you should refer the less validity-sure end-users to third-party services about signature validity. If you try to do all of it inside pdf.js for all jurisdictions, you will probably never get there.

For example there is https://validator.ch (https://www.e-service.admin.ch/validator/upload/all/en) with ELEVEN flavours of signature validity - and this for just one of the smallest countries of the planet.

Hence, please reconsider re-opening this issue and removing the "non-regression" of data.fieldValue w.r.t. v 2.4.456 !

P.S.: I am also fine, if you by default put a big red X over each signature display to be on the safe side...

@ralfhauser
Copy link
Author

#13214 seems to be positive news

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

No branches or pull requests

3 participants