Remove erroneous handling of /EFF for stream decryption

I thought /EFF was supposed to be used as a default for decrypting embedded file streams, but actually it's supposed to be advice to a conforming writer about handling new ones. This makes sense since the findAttachmentStreams code, which is not actually needed, was never right.

Remove erroneous handling of /EFF for stream decryption
I thought /EFF was supposed to be used as a default for decrypting embedded file streams, but actually it's supposed to be advice to a conforming writer about handling new ones. This makes sense since the findAttachmentStreams code, which is not actually needed, was never right.
Jay Berkenbilt
1 parent ac2b3b96
Showing 12 changed files with 64 additions and 85 deletions
TODO
include/qpdf/QPDF.hh
libqpdf/QPDF.cc
libqpdf/QPDF_encryption.cc
qpdf/qpdf.testcov
qpdf/qtest/qpdf.test
qpdf/qtest/qpdf/indirect-r-arg.out
qpdf/qtest/qpdf/nontrivial-crypt-filter-decrypted.pdf
qpdf/qtest/qpdf/nontrivial-crypt-filter.out
qpdf/qtest/qpdf/nontrivial-crypt-filter.pdf
qpdf/qtest/qpdf/obj0-check.out
qpdf/qtest/qpdf/test73.out
@@ -425,25 +425,13 @@ I find it useful to make reference to them in this list.
    http://multivalent.sourceforge.net/Tools/pdf/Extract.html
    http://multivalent.sourceforge.net/Tools/pdf/Embed.html
- * The description of Crypt filters is unclear with respect to how to
-   use them to override /StmF for specific streams.  I'm not sure
-   whether qpdf will do the right thing for any specific individual
-   streams that might have crypt filters, but I believe it does based
-   on my testing of a limited subset.  The specification seems to imply
-   that only embedded file streams and metadata streams can have crypt
-   filters, and there are already special cases in the code to handle
-   those.  Most likely, it won't be a problem, but someday someone may
-   find a file that qpdf doesn't work on because of crypt filters.
-   There is an example in the spec of using a crypt filter on a
-   metadata stream.
-
-   For now, we notice /Crypt filters and decode parameters consistent
-   with the example in the PDF specification, and the right thing
-   happens for metadata filters that happen to be uncompressed or
-   otherwise compressed in a way we can filter.  This should handle
-   all normal cases, but it's more or less just a guess since I don't
-   have any test files that actually use stream-specific crypt filters
-   in them.
+ * Qpdf does not honor /EFF when adding new file attachments. When it
+   encrypts, it never generates streams with explicit crypt filters.
+   Prior to 10.2, there was an incorrect attempt to treat /EFF as a
+   default value for decrypting file attachment streams, but it is not
+   supposed to mean that. Instead, it is intended for comforming
+   writers to obey this when adding new attachments. Qpdf is not a
+   conforming writer in that respect.
  * The second xref stream for linearized files has to be padded only
    because we need file_size as computed in pass 1 to be accurate.  If
@@ -814,7 +814,6 @@ class QPDF
             int foreign_generation,
             qpdf_offset_t offset,
             size_t length,
-            bool is_attachment_stream,
             QPDFObjectHandle local_dict);
       private:
@@ -824,7 +823,6 @@ class QPDF
         int foreign_generation;
         qpdf_offset_t offset;
         size_t length;
-        bool is_attachment_stream;
         QPDFObjectHandle local_dict;
     };
@@ -919,7 +917,6 @@ class QPDF
 	int& act_objid, int& act_generation);
     PointerHolder<QPDFObject> resolve(int objid, int generation);
     void resolveObjectsInStream(int obj_stream_number);
-    void findAttachmentStreams();
     void stopOnError(std::string const& message);
     // Calls finish() on the pipeline when done but does not delete it
@@ -938,7 +935,6 @@ class QPDF
                                int objid, int generation,
                                qpdf_offset_t offset, size_t length,
                                QPDFObjectHandle dict,
-                               bool is_attachment_stream,
                                Pipeline* pipeline,
                                bool suppress_warnings,
                                bool will_retry);
@@ -1001,7 +997,7 @@ class QPDF
         PointerHolder<InputSource> file,
         QPDF& qpdf_for_warning, Pipeline*& pipeline,
         int objid, int generation,
-	QPDFObjectHandle& stream_dict, bool is_attachment_stream,
+	QPDFObjectHandle& stream_dict,
 	std::vector<PointerHolder<Pipeline> >& heap);
     // Methods to support object copying
@@ -1422,7 +1418,6 @@ class QPDF
         PointerHolder<QPDFObjectHandle::StreamDataProvider> copied_streams;
         // copied_stream_data_provider is owned by copied_streams
         CopiedStreamDataProvider* copied_stream_data_provider;
-        std::set<QPDFObjGen> attachment_streams;
         bool reconstructed_xref;
         bool fixed_dangling_refs;
         bool immediate_copy_from;
@@ -18,7 +18,6 @@
 #include <qpdf/FileInputSource.hh>
 #include <qpdf/BufferInputSource.hh>
 #include <qpdf/OffsetInputSource.hh>
-#include <qpdf/QPDFNameTreeObjectHelper.hh>
 #include <qpdf/QPDFExc.hh>
 #include <qpdf/QPDF_Null.hh>
@@ -98,7 +97,6 @@ QPDF::ForeignStreamData::ForeignStreamData(
     int foreign_generation,
     qpdf_offset_t offset,
     size_t length,
-    bool is_attachment_stream,
     QPDFObjectHandle local_dict)
     :
     encp(encp),
@@ -107,7 +105,6 @@ QPDF::ForeignStreamData::ForeignStreamData(
     foreign_generation(foreign_generation),
     offset(offset),
     length(length),
-    is_attachment_stream(is_attachment_stream),
     local_dict(local_dict)
 {
 }
@@ -508,7 +505,6 @@ QPDF::parse(char const* password)
     }
     initializeEncryption();
-    findAttachmentStreams();
     this->m->parsed = true;
 }
@@ -2648,8 +2644,6 @@ QPDF::replaceForeignIndirectObjects(
                     foreign.getGeneration(),
                     stream->getOffset(),
                     stream->getLength(),
-                    (foreign_stream_qpdf->m->attachment_streams.count(
-                        foreign.getObjGen()) > 0),
                     dict);
             this->m->copied_stream_data_provider->registerForeignStream(
                 local_og, foreign_stream_data);
@@ -2882,7 +2876,6 @@ QPDF::pipeStreamData(PointerHolder&lt;EncryptionParameters&gt; encp,
                      int objid, int generation,
 		     qpdf_offset_t offset, size_t length,
 		     QPDFObjectHandle stream_dict,
-                     bool is_attachment_stream,
 		     Pipeline* pipeline,
                      bool suppress_warnings,
                      bool will_retry)
@@ -2892,7 +2885,7 @@ QPDF::pipeStreamData(PointerHolder&lt;EncryptionParameters&gt; encp,
     {
 	decryptStream(encp, file, qpdf_for_warning,
                       pipeline, objid, generation,
-                      stream_dict, is_attachment_stream, to_delete);
+                      stream_dict, to_delete);
     }
     bool success = false;
@@ -2968,14 +2961,11 @@ QPDF::pipeStreamData(int objid, int generation,
                      bool suppress_warnings,
                      bool will_retry)
 {
-    bool is_attachment_stream = (
-        this->m->attachment_streams.count(
-            QPDFObjGen(objid, generation)) > 0);
     return pipeStreamData(
         this->m->encp, this->m->file, *this,
         objid, generation, offset, length,
-        stream_dict, is_attachment_stream,
-        pipeline, suppress_warnings, will_retry);
+        stream_dict, pipeline,
+        suppress_warnings, will_retry);
 }
 bool
@@ -2992,36 +2982,8 @@ QPDF::pipeForeignStreamData(
         foreign->encp, foreign->file, *this,
         foreign->foreign_objid, foreign->foreign_generation,
         foreign->offset, foreign->length,
-        foreign->local_dict, foreign->is_attachment_stream,
-        pipeline, suppress_warnings, will_retry);
-}
-
-void
-QPDF::findAttachmentStreams()
-{
-    QPDFObjectHandle root = getRoot();
-    QPDFObjectHandle names = root.getKey("/Names");
-    if (! names.isDictionary())
-    {
-        return;
-    }
-    QPDFObjectHandle embedded_files = names.getKey("/EmbeddedFiles");
-    if (! embedded_files.isDictionary())
-    {
-        return;
-    }
-    QPDFNameTreeObjectHelper ef_tree(embedded_files, *this);
-    for (auto const& i: ef_tree)
-    {
-        QPDFObjectHandle item = i.second;
-        if (item.isDictionary() &&
-            item.getKey("/EF").isDictionary() &&
-            item.getKey("/EF").getKey("/F").isStream())
-        {
-            QPDFObjectHandle stream = item.getKey("/EF").getKey("/F");
-            this->m->attachment_streams.insert(stream.getObjGen());
-        }
-    }
+        foreign->local_dict, pipeline,
+        suppress_warnings, will_retry);
 }
 void
@@ -1022,6 +1022,20 @@ QPDF::initializeEncryption()
 	this->m->encp->cf_string = interpretCF(this->m->encp, StrF);
 	if (EFF.isName())
 	{
+            // qpdf does not use this for anything other than
+            // informational purposes. This is intended to instruct
+            // conforming writers on which crypt filter should be used
+            // when new file attachments are added to a PDF file, but
+            // qpdf never generates encrypted files with non-default
+            // crypt filters. Prior to 10.2, I was under the mistaken
+            // impression that this was supposed to be used for
+            // decrypting attachments, but the code was wrong in a way
+            // that turns out not to have mattered because no writers
+            // were generating files the way I was imagining. Still,
+            // providing this information could be useful when looking
+            // at a file generated by something else, such as Acrobat
+            // when specifying that only attachments should be
+            // encrypted.
 	    this->m->encp->cf_file = interpretCF(this->m->encp, EFF);
 	}
 	else
@@ -1224,7 +1238,6 @@ QPDF::decryptStream(PointerHolder&lt;EncryptionParameters&gt; encp,
                     QPDF& qpdf_for_warning, Pipeline*& pipeline,
                     int objid, int generation,
 		    QPDFObjectHandle& stream_dict,
-                    bool is_attachment_stream,
 		    std::vector<PointerHolder<Pipeline> >& heap)
 {
     std::string type;
@@ -1296,15 +1309,7 @@ QPDF::decryptStream(PointerHolder&lt;EncryptionParameters&gt; encp,
 	    }
 	    else
 	    {
-                if (is_attachment_stream)
-                {
-                    QTC::TC("qpdf", "QPDF_encryption attachment stream");
-                    method = encp->cf_file;
-                }
-                else
-                {
-                    method = encp->cf_stream;
-                }
+                method = encp->cf_stream;
 	    }
 	}
 	use_aes = false;
@@ -401,7 +401,6 @@ qpdf image optimize no pipeline 0
 qpdf image optimize no shrink 0
 qpdf image optimize too small 0
 QPDFFormFieldObjectHelper WinAnsi 0
-QPDF_encryption attachment stream 0
 QPDF pipe foreign encrypted stream 0
 QPDF copy foreign stream with provider 0
 QPDF copy foreign stream with buffer 0
@@ -1257,7 +1257,7 @@ $n_tests += 3;
 $td->runtest("closed input source",
 	     {$td->COMMAND => "test_driver 73 minimal.pdf"},
 	     {$td->FILE => "test73.out",
-	      $td->EXIT_STATUS => 0},
+	      $td->EXIT_STATUS => 2},
 	     $td->NORMALIZE_NEWLINES);
 $td->runtest("empty object",
@@ -3878,7 +3878,7 @@ $td-&gt;runtest(&quot;check encryption&quot;,
 	     $td->NORMALIZE_NEWLINES);
 # Look at some actual V4 files
-$n_tests += 14;
+$n_tests += 17;
 foreach my $d (['--force-V4', 'V4'],
 	       ['--cleartext-metadata', 'V4-clearmeta'],
 	       ['--use-aes=y', 'V4-aes'],
@@ -3906,6 +3906,19 @@ $td-&gt;runtest(&quot;decrypt with crypt filter&quot;,
 $td->runtest("check output",
 	     {$td->FILE => 'a.pdf'},
 	     {$td->FILE => 'decrypted-crypt-filter.pdf'});
+$td->runtest("nontrivial crypt filter",
+	     {$td->COMMAND => "qpdf --qdf --decrypt --static-id" .
+		  " nontrivial-crypt-filter.pdf --password=asdfqwer a.pdf"},
+	     {$td->STRING => "", $td->EXIT_STATUS => 0});
+$td->runtest("check output",
+	     {$td->FILE => 'a.pdf'},
+	     {$td->FILE => 'nontrivial-crypt-filter-decrypted.pdf'});
+$td->runtest("show nontrivial EFF",
+	     {$td->COMMAND => "qpdf --show-encryption" .
+		  " nontrivial-crypt-filter.pdf --password=asdfqwer"},
+	     {$td->FILE => "nontrivial-crypt-filter.out",
+                  $td->EXIT_STATUS => 0},
+             $td->NORMALIZE_NEWLINES);
 # Copy encryption parameters
 $n_tests += 10;
+checking indirect-r-arg.pdf
 WARNING: indirect-r-arg.pdf (object 1 0, offset 76): unknown token while reading object; treating as string
 WARNING: indirect-r-arg.pdf (object 1 0, offset 62): expected dictionary key but found non-name object; inserting key /QPDFFake1
 WARNING: indirect-r-arg.pdf (object 1 0, offset 62): expected dictionary key but found non-name object; inserting key /QPDFFake2
-checking indirect-r-arg.pdf
 PDF Version: 1.3
 File is not encrypted
 File is not linearized
+R = 6
+P = -1028
+User password = asdfqwer
+Supplied password is owner password
+Supplied password is user password
+extract for accessibility: allowed
+extract for any purpose: allowed
+print low resolution: allowed
+print high resolution: allowed
+modify document assembly: not allowed
+modify forms: allowed
+modify annotations: allowed
+modify other: allowed
+modify anything: not allowed
+stream encryption method: none
+string encryption method: none
+file encryption method: AESv3
+checking obj0.pdf
 WARNING: obj0.pdf: file is damaged
 WARNING: obj0.pdf (object 1 0, offset 77): expected n n obj
 WARNING: obj0.pdf: Attempting to reconstruct cross-reference table
-checking obj0.pdf
 PDF Version: 1.3
 File is not encrypted
 File is not linearized
-WARNING: closed input source: object 2/0: error reading object: QPDF operation attempted after closing input source
-test 73 done
+WARNING: closed input source: object 1/0: error reading object: QPDF operation attempted after closing input source
+closed input source: unable to find /Root dictionary