Disallow `--deterministic-id` with encrypted output and improve error handling f…

…or deterministic ID generation (fixes #1235).

Disallow `--deterministic-id` with encrypted output and improve error handling f…
…or deterministic ID generation (fixes #1235).
m-holger
1 parent e68d5392
Showing 7 changed files with 53 additions and 17 deletions
include/qpdf/QPDFWriter.hh
libqpdf/QPDFJob_config.cc
libqpdf/QPDFWriter.cc
manual/release-notes.rst
qpdf/qpdf.testcov
qpdf/qtest/arg-parsing.test
qpdf/qtest/deterministic-id.test
@@ -495,7 +495,7 @@ class QPDFWriter
     void preserveObjectStreams();
     void generateObjectStreams();
     std::string getOriginalID1();
-    void generateID();
+    void generateID(bool encrypted);
     void interpretR3EncryptionParameters(
         bool allow_accessibility,
         bool allow_extract,
@@ -149,6 +149,9 @@ QPDFJob::Config::jpegQuality(std::string const&amp; parameter)
 QPDFJob::Config*
 QPDFJob::Config::copyEncryption(std::string const& parameter)
 {
+    if (o.m->deterministic_id) {
+        usage("the deterministic-id option is incompatible with encrypted output files");
+    }
     o.m->encryption_file = parameter;
     o.m->copy_encryption = true;
     o.m->encrypt = false;
@@ -168,6 +171,9 @@ QPDFJob::Config::decrypt()
 QPDFJob::Config*
 QPDFJob::Config::deterministicId()
 {
+    if (o.m->encrypt || o.m->copy_encryption) {
+        usage("the deterministic-id option is incompatible with encrypted output files");
+    }
     o.m->deterministic_id = true;
     return this;
 }
@@ -1116,6 +1122,9 @@ std::shared_ptr&lt;QPDFJob::EncConfig&gt;
 QPDFJob::Config::encrypt(
     int keylen, std::string const& user_password, std::string const& owner_password)
 {
+    if (o.m->deterministic_id) {
+        usage("the deterministic-id option is incompatible with encrypted output files");
+    }
     o.m->keylen = keylen;
     if (keylen == 256) {
         o.m->use_aes = true;
@@ -859,7 +859,7 @@ QPDFWriter::interpretR3EncryptionParameters(
 void
 QPDFWriter::setEncryptionParameters(char const* user_password, char const* owner_password)
 {
-    generateID();
+    generateID(true);
     m->encryption->setId1(m->id1);
     m->encryption_key = m->encryption->compute_parameters(user_password, owner_password);
     setEncryptionMinimumVersion();
@@ -871,7 +871,7 @@ QPDFWriter::copyEncryptionParameters(QPDF&amp; qpdf)
     m->preserve_encryption = false;
     QPDFObjectHandle trailer = qpdf.getTrailer();
     if (trailer.hasKey("/Encrypt")) {
-        generateID();
+        generateID(true);
         m->id1 = trailer.getKey("/ID").getArrayItem(0).getStringValue();
         QPDFObjectHandle encrypt = trailer.getKey("/Encrypt");
         int V = encrypt.getKey("/V").getIntValueAsInt();
@@ -1301,7 +1301,7 @@ QPDFWriter::writeTrailer(
         if (linearization_pass == 0 && m->deterministic_id) {
             computeDeterministicIDData();
         }
-        generateID();
+        generateID(m->encryption.get());
         write_string(m->id1, true).write_string(m->id2, true);
     }
     write("]");
@@ -1865,7 +1865,7 @@ QPDFWriter::getOriginalID1()
 }
  
 void
-QPDFWriter::generateID()
+QPDFWriter::generateID(bool encrypted)
 {
     // Generate the ID lazily so that we can handle the user's preference to use static or
     // deterministic ID generation.
@@ -1911,12 +1911,14 @@ QPDFWriter::generateID()
  
         std::string seed;
         if (m->deterministic_id) {
-            if (m->deterministic_id_data.empty()) {
-                QTC::TC("qpdf", "QPDFWriter deterministic with no data");
+            if (encrypted) {
                 throw std::runtime_error(
-                    "INTERNAL ERROR: QPDFWriter::generateID has no data for "
-                    "deterministic ID.  This may happen if deterministic ID "
-                    "and file encryption are requested together.");
+                    "QPDFWriter: unable to generated a deterministic ID because the file to be "
+                    "written is encrypted (even though the file may not require a password)");
+            }
+            if (m->deterministic_id_data.empty()) {
+                throw std::logic_error(
+                    "INTERNAL ERROR: QPDFWriter::generateID has no data for deterministic ID");
             }
             seed += m->deterministic_id_data;
         } else {
@@ -23,6 +23,13 @@ more detail.
       not work on some older Linux distributions. If you need support
       for an older distribution, please use version 12.2.0 or below.
  
+  - CLI Enhancements
+
+   - Disallow option :qpdf:ref:`--deterministic-id` to be used together
+     with the incompatible options :qpdf:ref:`--encrypt` or
+     :qpdf:ref:`--copy-encryption`.
+
+
   - Other enhancements
  
     - ``QPDFWriter`` will no longer add filters when writing empty streams.
@@ -254,7 +254,6 @@ QPDFJob npages 0
 QPDF already reserved object 0
 QPDFWriter standard deterministic ID 1
 QPDFWriter linearized deterministic ID 1
-QPDFWriter deterministic with no data 0
 qpdf-c called qpdf_set_deterministic_ID 0
 QPDFParser invalid objgen 0
 QPDF object id 0 0
@@ -15,7 +15,7 @@ cleanup();
  
 my $td = new TestDriver('arg-parsing');
  
-my $n_tests = 26;
+my $n_tests = 30;
  
 $td->runtest("required argument",
              {$td->COMMAND => "qpdf --password minimal.pdf"},
@@ -138,6 +138,26 @@ $td-&gt;runtest(&quot;missing key length&quot;,
              {$td->REGEXP => ".*encryption key length is required",
                   $td->EXIT_STATUS => 2},
              $td->NORMALIZE_NEWLINES);
+$td->runtest("encrypt and deterministic-id 1",
+             {$td->COMMAND => "qpdf --deterministic-id --encrypt --"},
+             {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files",
+                  $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("encrypt and deterministic-id 2",
+             {$td->COMMAND => "qpdf --deterministic-id --encrypt --bits=256 -- --deterministic-id"},
+             {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files",
+                  $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("copy-encryption and deterministic-id 1",
+             {$td->COMMAND => "qpdf --deterministic-id --copy-encryption=missing.pdf"},
+             {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files",
+                  $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("copy-encryption and deterministic-id 2",
+             {$td->COMMAND => "qpdf --copy-encryption=missing.pdf --deterministic-id"},
+             {$td->REGEXP => ".*the deterministic-id option is incompatible with encrypted output files",
+                  $td->EXIT_STATUS => 2},
+             $td->NORMALIZE_NEWLINES);
  
 # Disallow mixing positional and flag-style encryption arguments.
 my @bad_enc = (
@@ -54,11 +54,10 @@ foreach my $d (&#39;nn&#39;, &#39;ny&#39;, &#39;yn&#39;, &#39;yy&#39;)
  
 $td->runtest("deterministic ID with encryption",
              {$td->COMMAND => "qpdf -deterministic-id encrypted-with-images.pdf a.pdf"},
-             {$td->STRING => "qpdf: INTERNAL ERROR: QPDFWriter::generateID" .
-                  " has no data for deterministic ID." .
-                  "  This may happen if deterministic ID and" .
-                  " file encryption are requested together.\n",
-              $td->EXIT_STATUS => 2},
+             {$td->STRING => "qpdf: QPDFWriter: unable to generated a deterministic ID " .
+                    "because the file to be written is encrypted (even though the file " .
+                    "may not require a password)\n",
+             $td->EXIT_STATUS => 2},
              $td->NORMALIZE_NEWLINES);
 $td->runtest("deterministic ID (C API)",
              {$td->COMMAND =>