node-signpdf icon indicating copy to clipboard operation
node-signpdf copied to clipboard

plainAddPlaceholder breaks the PDF file with existing annotations

Open symbianm opened this issue 1 year ago • 4 comments

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: '[email protected]',
    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

symbianm avatar Oct 20 '23 04:10 symbianm

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 avatar Oct 20 '23 07:10 vbuch

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

vbuch avatar Nov 10 '23 10:11 vbuch

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;

symbianm avatar Nov 13 '23 07:11 symbianm

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.

vbuch avatar Nov 14 '23 09:11 vbuch