Fix for Brasil A1 Certificates and more

[ghost]

My fork is kind of a mess because I didn't intend to merge so I am going to provide more information here. There are 3 main changes:

1) contact, name, localSign and signatureName can be declared on plainAddPlaceholder. I think it's a good feature.

2) I am using this with react-native and "encryptFn" on pdfObject.js breaked the signature (all the fields would be empty). I commented it so if you don't want to merge that I can revert it quickly to merge and undo it for me after that.

3 and most important) Removed authenticatedAttributes. node-signpdf is only sending default values to node-forge and node-forge doesn't really need it. node-forge will fill the authenticatedAttributes by itself if none are provided and when we let node-forge do it the Brasil A1 certificates will be successfully verified (in https://verificador.iti.gov.br/verifier-2.5.1/). Maybe it will fix AdES signature too if the problem is similar.

[coveralls]

Pull Request Test Coverage Report for Build 313

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 275:	0.0%
Covered Lines:	281
Relevant Lines:	281

---

💛 - Coveralls

[vbuch]

Nice. Seems like I would merge this. Thank you. If you could clean it up before I merge, it would be great. I'm still interested, if those are the defaults for authattr, what becomes different in forge. I'll have a look when I have the time. Meanwhile, please make sure it is mergable. I think, since it introduces the feature with data added, I would make it a feature version.

[ghost]

I fixed some spaces and idents if that's what you meant with cleaning it up. I also tried to figure out what was the difference between what we were sending and the defaults used by forge. I didn't went to deep into it but as far as I reached it was sending the right values.

I also added an how to plainAddPlaceholder to the readme since I changed the way it works.

[vbuch]

Wait, wait. Your changes are only in the dist folder. You know that once I run build those changes will be gone, right?

[ghost]

My bad, I changed the source files now. Changing the index.js broke the tests. I'll try to figure out later why and fix it.

[vbuch]

I think I know why but it is a guess. The signature changed as some attributes are missed. The only way to make sure you are not regressing here would be to manually check what the decoded signature contains (again pointing to https://github.com/ninja-labs-tech/verify-pdf where the tooling should be available) and make sure all the needed data is in the signature. Once that is verified, just updating a snapshot should fix the test. But still, the verification needs to be done. It would be perfect if verify-pdf can be employed in automatic testing.

[therpobinski]

It is just what I thought to implement now, in my 'fork' I will, once I have solved the issue with this pull-request, I will add this which seems very important to me! I hope to upload this until tomorrow.

[vbuch]

@therpobinski please do not do the two things in the same PR. They are separate things. If we keep them separated, chances of one going live before the other are good. Otherwise one may remain blocked by failure with the other. Also please see my comment above. A verification is needed and check with the specs that the generated signatures are not regressed.

[therpobinski]

Ok, so I work on this same PR ?? What do you mean by regressed? This is the body that that library returns to me to verify the PDF, it only works for the first signature it finds.

{ verified: false, authenticity: false, integrity: true, expired: false, meta: { certs: [ [Object], [Object], [Object] ], signatureMeta: { reason: '', contactInfo: 'emailfromp1289@gmail.com', location: 'Location from p12' } } }

The certs inside the object is the information of the first signature and the entity that created the signature.

[vbuch]

https://en.m.wikipedia.org/wiki/Software_regression

[ghost]

Thanks to https://github.com/dipedro for changing the snapshot to pass the build. There was no point for me to be doing those last few changes, those were a mess. Next time I submit a PR I'll make sure to have a cleaner environment for testing and pay attention to PR made to my fork.

[ghost]

I've been looking for the differences between the files generated from both versions using some pdf debuggers, text editors and pdf readers for the last few days. There are only 2 differences I could find between the signatures, the M field (date/time obviously would be different) and the contents field (it would be weird it it wasn't different and worked), but as far as I can tell, the signatures are identical, one is just lighter than the other. I couldn't test with ninja-labs verify-pdf as it does not work well with my app, if someone know a way to debug/read the "/Contents" please comment.

[brenooandrade]

I'm waiting for this changes, @Alph4Four. It'll great for me.

[ghost]

@brenooandrade I spent way too much time trying to figure out why the signature size changed without reaching any conclusion. There should not be any negative effects of using the default values. If you want to use my changes this PR is associeated with a fork that you can use or port the changes to a newer version. I also recommend using pdf-lib for making your placeholders since that will allow you to make a lot of cool stuff like adding an image and text to the signature.

[brenooandrade]

@brenooandrade I spent way too much time trying to figure out why the signature size changed without reaching any conclusion. There should not be any negative effects of using the default values. If you want to use my changes this PR is associeated with a fork that you can use or port the changes to a newer version. I also recommend using pdf-lib for making your placeholders since that will allow you to make a lot of cool stuff like adding an image and text to the signature.

Right @Alph4Four I already got your fork and I'm studying how can I use it in Brazilian certificate A1, in this moment I'm trying to solve an error that I'm getting when I try to sign the PDF ("Signature exceeds placeholder length: 5000 > 3224"). But thank you for the tips, surely I'll follow it!

Best regards!

[ghost]

This is not the best place to discuss that issue but your error can be easly fixed. Assuming that you are using something like plainAddPlaceholder(), you have an argument that is signatureLength. Your signatureLength is likely not big enough. Try to make that value yourPdfBuffer.length or 99999 to see if the error goes away, if it does, you can figure something out from there.

[brenooandrade]

This is not the best place to discuss that issue but your error can be easly fixed. Assuming that you are using something like plainAddPlaceholder(), you have an argument that is signatureLength. Your signatureLength is likely not big enough. Try to make that value yourPdfBuffer.length or 99999 to see if the error goes away, if it does, you can figure something out from there.

Thank you so much @Alph4Four it works for me and I signed my PDF file. So now I'll use your fork to improve and study others possibilities to add more information in the placeholder. Thank you for the tips and the support, if I improve something in your code I'll answer here.

Best regards!

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in the past 90 days. It will be closed if no further activity occurs. Thank you for your contributions.

[dipedro]

@brenooandrade I spent way too much time trying to figure out why the signature size changed without reaching any conclusion. There should not be any negative effects of using the default values. If you want to use my changes this PR is associeated with a fork that you can use or port the changes to a newer version. I also recommend using pdf-lib for making your placeholders since that will allow you to make a lot of cool stuff like adding an image and text to the signature.

The size change because you remove the authenticatedAttributes! The funny thing is that in the documentation of ICPBrasil (Certificate Authority from Brazil) say that the parameter authenticatedAttributes is required, but if i remove all works fine in https://verificador.iti.gov.br/verifier-2.6.1/ If i pass authenticatedAttributes with the defaults values i get the error Problems getting the hash in messageDigest algorithm.