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

Digital signature #243

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

pamapa
Copy link
Contributor

@pamapa pamapa commented Jun 2, 2022

part of #237

I am not so sure if the taken approach is the right way to:

  • PdfItem.WriteObject vs PdfItem.Write seems error prone
  • is the position tracker approach really needed, can't that be done more elegant?

We current do not need to sign PDFs for our project, therefore i am not investing time for this feature.
In order somebody else wants to work on this, i am pushing my changes on a draft branch. This branch is not intended to be merged like it is now...

@pamapa pamapa marked this pull request as draft June 2, 2022 08:01
@pamapa pamapa force-pushed the digital-signature branch from 7154ce6 to ed8d9fb Compare June 2, 2022 08:23
@pamapa pamapa mentioned this pull request Jun 2, 2022
@pamapa pamapa force-pushed the digital-signature branch from ed8d9fb to b3e1154 Compare June 4, 2022 06:25
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 5.98802% with 314 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@4c3bcbb). Learn more about missing BASE report.

Files with missing lines Patch % Lines
PdfSharpCore/Pdf.Signatures/RangedStream.cs 0.00% 83 Missing ⚠️
PdfSharpCore/Pdf.Signatures/PdfSignatureHandler.cs 0.00% 77 Missing ⚠️
PdfSharpCore/Pdf.AcroForms/PdfSignatureField.cs 0.00% 68 Missing ⚠️
PdfSharpCore/Pdf.Signatures/DefaultSigner.cs 0.00% 20 Missing ⚠️
PdfSharpCore/Pdf/PdfArray.cs 11.11% 15 Missing and 1 partial ⚠️
...arpCore/Pdf.Signatures/DefaultAppearanceHandler.cs 0.00% 15 Missing ⚠️
PdfSharpCore/Pdf.Signatures/PositionTracker.cs 0.00% 11 Missing ⚠️
PdfSharpCore/Pdf.Internal/PdfEncoders.cs 0.00% 5 Missing and 2 partials ⚠️
PdfSharpCore/Pdf.Signatures/PdfSignatureOptions.cs 0.00% 5 Missing ⚠️
PdfSharpCore/Pdf.Advanced/PdfCatalog.cs 0.00% 4 Missing ⚠️
... and 4 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #243   +/-   ##
=========================================
  Coverage          ?   26.07%           
=========================================
  Files             ?      244           
  Lines             ?    23645           
  Branches          ?     2833           
=========================================
  Hits              ?     6165           
  Misses            ?    16984           
  Partials          ?      496           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mmmilan
Copy link

mmmilan commented Oct 4, 2022

Are you sure this works? For me, Adobe Reader validates the signature OK, but in the place of the visible signature I got an empty rectangle.

@pamapa
Copy link
Contributor Author

pamapa commented Oct 4, 2022

I'm not 100% sure, its some time ago i did this. At the end we did not need that feature so i stopped and archived this MR as a starting point for somebody else. In the issue i have referenced the places where i did take the code parts from. In general i am not 100% sure if this approach is the right way to do it anyway, as it is very invasive (see description above). So feel free to hack on this and improve it :-)

Have you seen SignatureTestConsole, this might be a good starting point.

@mmmilan
Copy link

mmmilan commented Oct 4, 2022

Thanks for your reply! Yes, I'm fiddling with SignatureTestConsole, I've just added my own certificate. If this had worked for you, I must have messed up something...

@ascott18
Copy link
Contributor

ascott18 commented Nov 30, 2022

@mmmilan @pamapa Its not visible because XForm.Finish() is not implemented in PdfSharpCore.

internal virtual void Finish()
versus https://github.com/empira/PDFsharp/blob/3205bd933b464d150c0f42e8bcdff3314b6c6164/src/PdfSharp/Drawing/XForm.cs#L231

It also doesn't work in SignatureTestConsole by default, because that project's first target framework is netcoreapp3.1, and in the version of PdfSharpCore that this PR is based on, the particular XGraphics ctor that this work requires just silently doesn't create the renderer unless the TFM is net5.0 or higher: https://github.com/pamapa/PdfSharpCore/blob/b3e115417785be2f84150b14b7210854b0767997/PdfSharpCore/Drawing/XGraphics.cs#L211-L217 (this was fixed recently upstream in #295)

If you change SignatureTestConsole to target .NET 6 specifically, and also go add the following implementation of XForm.Finish(), then it works:

        internal virtual void Finish()
        {
            if (_formState == FormState.NotATemplate || _formState == FormState.Finished)
                return;

            Debug.Assert(_formState == FormState.Created || _formState == FormState.UnderConstruction);
            _formState = FormState.Finished;
            Gfx.Dispose();
            Gfx = null;

            if (PdfRenderer != null)
            {
                //pdfForm.CreateStream(PdfEncoders.RawEncoding.GetBytes(PdfRenderer.GetContent()));
                PdfRenderer.Close();
                Debug.Assert(PdfRenderer == null);

                if (_document.Options.CompressContentStreams)
                {
                    _pdfForm.Stream.Value = Filtering.FlateDecode.Encode(_pdfForm.Stream.Value, _document.Options.FlateEncodeMode);
                    _pdfForm.Elements["/Filter"] = new PdfName("/FlateDecode");
                }
                int length = _pdfForm.Stream.Length;
                _pdfForm.Elements.SetInteger("/Length", length);
            }
        }

@mmmilan
Copy link

mmmilan commented Nov 30, 2022

It works! Thank you!

@pamapa
Copy link
Contributor Author

pamapa commented Nov 30, 2022

@ascott18 nice work

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.

4 participants