vbuch / node-signpdf

Simple signing of PDFs in node.
MIT License
711 stars 178 forks source link

plainAddPlaceholder breaks the PDF file with existing annotations #198

Open symbianm opened 11 months ago

symbianm commented 11 months ago

Describe the bug and the expected behaviour When using a file with the existing form annotations and adding a placeholder using plainAddPlaceholder it breaks the PDF file. Acrobat Reader gives an error. MacO's preview misses all text. But Chrome opens it okay. Results are random and it depends on which annotations it has.

Screenshot 2023-10-20 at 5 52 21 Screenshot 2023-10-20 at 5 51 36

Is it a bug in signing or in the helpers? Issue with @signpdf/placeholder-plain package

To Reproduce Using the slightly modified example from here: https://github.com/vbuch/node-signpdf/blob/develop/packages/examples/src/javascript.js

const fs = require('fs');
const signpdf = require('@signpdf/signpdf').default;
const { plainAddPlaceholder } = require('@signpdf/placeholder-plain');
const { P12Signer } = require('@signpdf/signer-p12');

const signPdf = async () => {
  const p12Buffer = fs.readFileSync(`${__dirname}/certificate.p12`);
  const pdfBuffer = fs.readFileSync(`${__dirname}/document2.pdf`);

  // The PDF needs to have a placeholder for a signature to be signed.
  const pdfWithPlaceholder = plainAddPlaceholder({
    pdfBuffer,
    reason: 'Digitally signing PDF by Me.',
    contactInfo: 'test@test.com',
    name: 'Test',
    location: 'Fake Address',
  });

  // pdfWithPlaceholder is now a modified buffer that is ready to be signed.
  const signer = new P12Signer(p12Buffer);
  const signedPDfBuffer = await signpdf.sign(pdfWithPlaceholder, signer);

  fs.writeFileSync(`${__dirname}/signed-document.pdf`, signedPDfBuffer);
};

signPdf();

The certificate.p12 is from your repo https://github.com/vbuch/node-signpdf/blob/develop/resources/certificate.p12 PDF files: document2.pdf document.pdf

vbuch commented 11 months ago

Hi @symbianm , Thank you for the repro. Your document is PDF-1.7 which placeholder-plain notoriusly cannot handle as it includes streams ( see PDF > 1.3 ). There are some workarounds that I can think of:

  1. If you have control over the generation of that PDF, you may generate it with a version <= 1.3 or 1.7 but without streams. This will allow placeholder-plain to work with it.
  2. Try and work with pdf-lib. This example was written with the previous version of @signpdf node-signpdf but things will generally work the same way. Additionally a @signpdf/placeholder-pdf-lib could be a great addition to @signpdf so... PRs are welcome.
vbuch commented 11 months ago

https://github.com/vbuch/node-signpdf/issues/79#issuecomment-1803905603

symbianm commented 10 months ago

Hi @symbianm , Thank you for the repro. Your document is PDF-1.7 which placeholder-plain notoriusly cannot handle as it includes streams ( see PDF > 1.3 ). There are some workarounds that I can think of:

  1. If you have control over the generation of that PDF, you may generate it with a version <= 1.3 or 1.7 but without streams. This will allow placeholder-plain to work with it.
  2. Try and work with pdf-lib. This example was written with the previous version of @signpdf node-signpdf but things will generally work the same way. Additionally a @signpdf/placeholder-pdf-lib could be a great addition to @signpdf so... PRs are welcome.

Hi @vbuch sorry for the delay, and thanks for the details. Unfortunately, I have no control over the PDF generator and can't change the PDF version. And yes, I found that example too, but it required some adjustments because it removes all existing annotations when it adds signature annotation.

Here is my version if someone is looking for a resolution:

const fs = require('fs');
const signpdf = require('@signpdf/signpdf').default;
const utils = require('@signpdf/utils');
const { P12Signer } = require('@signpdf/signer-p12');
const {
  PDFDocument,
  PDFName,
  PDFNumber,
  PDFHexString,
  PDFString,
  PDFArray,
} = require('pdf-lib');
// Custom code to add Byterange to PDF
const PDFArrayCustom = require('./pdfArrayCustom.helper');

const p12Buffer = fs.readFileSync(`${__dirname}/certificate.p12`);

const signPdfService = async (buffer) => {
  const pdfDoc = await PDFDocument.load(buffer);
  const pages = pdfDoc.getPages();

  const ByteRange = PDFArrayCustom.withContext(pdfDoc.context);
  ByteRange.push(PDFNumber.of(0));
  ByteRange.push(PDFName.of(utils.DEFAULT_BYTE_RANGE_PLACEHOLDER));
  ByteRange.push(PDFName.of(utils.DEFAULT_BYTE_RANGE_PLACEHOLDER));
  ByteRange.push(PDFName.of(utils.DEFAULT_BYTE_RANGE_PLACEHOLDER));

  const signatureDict = pdfDoc.context.obj({
    Type: 'Sig',
    Filter: 'Adobe.PPKLite',
    SubFilter: 'adbe.pkcs7.detached',
    ByteRange,
    Contents: PDFHexString.of('A'.repeat(p12Buffer.byteLength * 2)),
    Reason: PDFString.of('Digitally verifiable PDF document signed by On Q.'),
    M: PDFString.fromDate(new Date()),
  });
  const signatureDictRef = pdfDoc.context.register(signatureDict);

  const lastPage = pages[pages.length - 1];
  const widgetDict = pdfDoc.context.obj({
    Type: 'Annot',
    Subtype: 'Widget',
    FT: 'Sig',
    Rect: [0, 0, 0, 0],
    V: signatureDictRef,
    T: PDFString.of('Signature1'),
    F: 4,
    P: lastPage.ref,
    DR: pdfDoc.context.obj({}),
  });
  const widgetDictRef = pdfDoc.context.register(widgetDict);

  // keep older annotations
  try {
    const annots = lastPage.node.lookup(PDFName.of('Annots'), PDFArray);
    annots.push(widgetDictRef);
    // Add our signature widget to the first page
    lastPage.node.set(PDFName.of('Annots'), pdfDoc.context.obj(annots));
  } catch (err) {
    lastPage.node.set(
      PDFName.of('Annots'),
      pdfDoc.context.obj([widgetDictRef]),
    );
  }

  // Create an AcroForm object containing our signature widget
  pdfDoc.catalog.set(
    PDFName.of('AcroForm'),
    pdfDoc.context.obj({
      SigFlags: 3,
      Fields: [widgetDictRef],
    }),
  );

  const modifiedPdfBytes = await pdfDoc.save({ useObjectStreams: false });
  const modifiedPdfBuffer = Buffer.from(modifiedPdfBytes);

  const signer = new P12Signer(p12Buffer);
  return signpdf.sign(modifiedPdfBuffer, signer);
};

module.exports = signPdfService;
vbuch commented 10 months ago

Thank you for the input @symbianm It was just the thing I was about to do on the placeholder-pdf-lib PR. Here is my take on it and here is how I test this

Once I rewrite the history so it makes more sense, I'll be welcoming reviews on this PR.

I think it's important to note that currently pdf-lib does not provide incremental updates so it is very likely that adding a new signature on top of the previous one invalidates the first one.