diff --git a/ChangeLog b/ChangeLog index 2670dd07..32410df9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2021-02-04 Jay Berkenbilt + + * By default, give an error if a user attempts to encrypt a file + with an empty owner password or an owner password that is the same + as the user password. Such files are insecure. Most viewers either + won't open such files or will not enforce security settings. To + allow explicit creation of files like this, pass the new + --allow-insecure option. Fixes #501. + 2021-02-02 Jay Berkenbilt * Bug fix: if a form XObject lacks a resources dictionary, diff --git a/manual/qpdf-manual.xml b/manual/qpdf-manual.xml index fda9c1fd..0d9fd489 100644 --- a/manual/qpdf-manual.xml +++ b/manual/qpdf-manual.xml @@ -1214,7 +1214,11 @@ make Either or both of the user password and the owner password may be - empty strings. + empty strings. Starting in qpdf 10.2, qpdf defaults to not + allowing creation of PDF files with an empty owner password or an + owner password that matches the user password. If you want to + create such files, specify the encryption option + , as described below. The value for @@ -1223,6 +1227,25 @@ make When no additional restrictions are given, the default is to be fully permissive. + + For all key lengths, the following options are available: + + + + + + From qpdf 10.2, qpdf defaults to not allowing creation of PDF + files where the owner password is blank or matches the user + password. Files created in this way are insecure and can't be + opened by some viewers. Users would ordinarily never want to + create such files. If you are using qpdf to intentionally + created strange files for testing (a definite valid use of + qpdf!), this option allows you to create such insecure files. + + + + + If is 40, the following restriction options are available: @@ -4824,7 +4847,28 @@ print "\n"; - Behavior Changes + CLI Behavior Changes + + + + + By default, qpdf no longer allows + creation of encrypted PDF files whose owner password is + empty or matches the user password. The + , specified inside the + options, allows creation of such + files. Behavior changes in the CLI are avoided when + possible, but an exception was made here because this is + security-related. qpdf must always allow creation of weird + files for testing purposes, but it should not default to + letting users unknowingly create insecure files. + + + + + + + Library Behavior Changes diff --git a/qpdf/qpdf.cc b/qpdf/qpdf.cc index 0405a45d..702a6b9e 100644 --- a/qpdf/qpdf.cc +++ b/qpdf/qpdf.cc @@ -113,6 +113,7 @@ struct Options password_is_hex_key(false), suppress_password_recovery(false), password_mode(pm_auto), + allow_insecure(false), keylen(0), r2_print(true), r2_modify(true), @@ -211,6 +212,7 @@ struct Options bool password_is_hex_key; bool suppress_password_recovery; password_mode_e password_mode; + bool allow_insecure; std::string user_password; std::string owner_password; int keylen; @@ -742,6 +744,7 @@ class ArgParser void argEncrypt(); void argDecrypt(); void argPasswordIsHexKey(); + void argAllowInsecure(); void argPasswordMode(char* parameter); void argSuppressPasswordRecovery(); void argCopyEncryption(char* parameter); @@ -1074,13 +1077,17 @@ ArgParser::initOptionTable() t = &this->encrypt40_option_table; (*t)["--"] = oe_bare(&ArgParser::argEndEncrypt); + (*t)["allow-insecure"] = oe_bare(&ArgParser::argAllowInsecure); + // The above 40-bit options are also 128-bit and 256-bit options, + // so copy what we have so far to 128. Then continue separately + // with 128. We later copy 128 to 256. + this->encrypt128_option_table = this->encrypt40_option_table; (*t)["print"] = oe_requiredChoices(&ArgParser::arg40Print, yn); (*t)["modify"] = oe_requiredChoices(&ArgParser::arg40Modify, yn); (*t)["extract"] = oe_requiredChoices(&ArgParser::arg40Extract, yn); (*t)["annotate"] = oe_requiredChoices(&ArgParser::arg40Annotate, yn); t = &this->encrypt128_option_table; - (*t)["--"] = oe_bare(&ArgParser::argEndEncrypt); (*t)["accessibility"] = oe_requiredChoices( &ArgParser::arg128Accessibility, yn); (*t)["extract"] = oe_requiredChoices(&ArgParser::arg128Extract, yn); @@ -1317,6 +1324,10 @@ ArgParser::argHelp() << "\n" << "Additional flags are dependent upon key length.\n" << "\n" + << " For all key lengths:\n" + << " --allow-insecure allow the owner password to be empty or the\n" + << " same as the user password\n" + << "\n" << " If 40:\n" << "\n" << " --print=[yn] allow printing\n" @@ -1849,6 +1860,12 @@ ArgParser::argPasswordMode(char* parameter) } } +void +ArgParser::argAllowInsecure() +{ + o.allow_insecure = true; +} + void ArgParser::argCopyEncryption(char* parameter) { @@ -3337,6 +3354,18 @@ ArgParser::doFinalChecks() " together"); } + if (o.encrypt && (! o.allow_insecure) && + (o.owner_password.empty() || + (o.owner_password == o.user_password))) + { + usage("An encrypted PDF with an empty owner password or an" + " owner password that is the same as a user password" + " is insecure and can't be opened by some viewers. If you" + " really want to do this, you must also give the" + " --allow-insecure option before the -- that follows" + " --encrypt."); + } + if (o.require_outfile && o.outfilename && (strcmp(o.outfilename, "-") == 0)) { diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 0ec1b834..f16210e7 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -3189,19 +3189,19 @@ foreach my $f (qw(compressed-metadata.pdf enc-base.pdf)) check_metadata("a.pdf", 0, 1); $td->runtest("encrypt normally", {$td->COMMAND => - "qpdf --encrypt '' '' 128 -- a.pdf b.pdf"}, + "qpdf --encrypt '' o 128 -- a.pdf b.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); check_metadata("b.pdf", 1, 0); unlink "b.pdf"; $td->runtest("encrypt V4", {$td->COMMAND => - "qpdf --encrypt '' '' 128 --force-V4 -- a.pdf b.pdf"}, + "qpdf --encrypt '' o 128 --force-V4 -- a.pdf b.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); check_metadata("b.pdf", 1, 0); unlink "b.pdf"; $td->runtest("encrypt with cleartext metadata", {$td->COMMAND => - "qpdf --encrypt '' '' 128 --cleartext-metadata --" . + "qpdf --encrypt '' o 128 --cleartext-metadata --" . " a.pdf b.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); check_metadata("b.pdf", 1, 1); @@ -3212,7 +3212,7 @@ foreach my $f (qw(compressed-metadata.pdf enc-base.pdf)) unlink "b.pdf", "c.pdf"; $td->runtest("encrypt with aes and cleartext metadata", {$td->COMMAND => - "qpdf --encrypt '' '' 128" . + "qpdf --encrypt '' o 128" . " --cleartext-metadata --use-aes=y -- a.pdf b.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); check_metadata("b.pdf", 1, 1); @@ -3441,7 +3441,7 @@ my @encrypted_files = 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1], ); -$n_tests += 5 + (2 * (@encrypted_files)) + (7 * (@encrypted_files - 6)) + 9; +$n_tests += 9 + (2 * (@encrypted_files)) + (7 * (@encrypted_files - 6)) + 9; $td->runtest("encrypted file", {$td->COMMAND => "test_driver 2 encrypted-with-images.pdf"}, @@ -3458,6 +3458,26 @@ $td->runtest("recheck encrypted file", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +$td->runtest("empty owner password", + {$td->COMMAND => "qpdf --encrypt '' '' 128 -- minimal.pdf a.pdf"}, + {$td->REGEXP => ".*is insecure.*--allow-insecure.*", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("matching user/owner password", + {$td->COMMAND => "qpdf --encrypt q q 128 -- minimal.pdf a.pdf"}, + {$td->REGEXP => ".*is insecure.*--allow-insecure.*", + $td->EXIT_STATUS => 2}, + $td->NORMALIZE_NEWLINES); +$td->runtest("allow insecure", + {$td->COMMAND => "qpdf --encrypt '' '' 128 --allow-insecure --" . + " minimal.pdf a.pdf"}, + {$td->STRING => "", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); +$td->runtest("check insecure", + {$td->COMMAND => "qpdf --check a.pdf"}, + {$td->FILE => "insecure-passwords.out", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + # Test that long passwords that are one character too short fail. We # test the truncation cases in the loop below by using passwords # longer than the supported length. @@ -3587,6 +3607,10 @@ foreach my $d (@encrypted_files) $enc_json =~ s/---upm---/$upm/; my $eflags = "-encrypt \"$upass\" \"$opass\" $bits $xeflags --"; + if (($opass eq "") || ($opass eq $upass)) + { + $eflags =~ s/--$/--allow-insecure --/; + } if (($pass ne $upass) && ($V >= 5)) { # V >= 5 can no longer recover user password with owner @@ -3758,7 +3782,7 @@ $td->runtest("check linearization", # Test AES encryption in various ways. $n_tests += 18; $td->runtest("encrypt with AES", - {$td->COMMAND => "qpdf --encrypt '' '' 128 --use-aes=y --" . + {$td->COMMAND => "qpdf --encrypt '' o 128 --use-aes=y --" . " enc-base.pdf a.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); $td->runtest("check encryption", @@ -3779,7 +3803,7 @@ $td->runtest("compare files", {$td->FILE => 'a.qdf'}, {$td->FILE => 'b.qdf'}); $td->runtest("linearize with AES and object streams", - {$td->COMMAND => "qpdf --encrypt '' '' 128 --use-aes=y --" . + {$td->COMMAND => "qpdf --encrypt '' o 128 --use-aes=y --" . " --linearize --object-streams=generate enc-base.pdf a.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); $td->runtest("check encryption", @@ -3845,7 +3869,8 @@ foreach my $d (['--force-V4', 'V4'], my ($args, $out) = @$d; $td->runtest("encrypt $args", {$td->COMMAND => "qpdf --static-aes-iv --static-id" . - " --encrypt '' '' 128 $args -- enc-base.pdf a.pdf"}, + " --encrypt '' '' 128 $args --allow-insecure --" . + " enc-base.pdf a.pdf"}, {$td->STRING => "", $td->EXIT_STATUS => 0}); $td->runtest("check output", {$td->FILE => "a.pdf"}, diff --git a/qpdf/qtest/qpdf/insecure-passwords.out b/qpdf/qtest/qpdf/insecure-passwords.out new file mode 100644 index 00000000..f0ba33e8 --- /dev/null +++ b/qpdf/qtest/qpdf/insecure-passwords.out @@ -0,0 +1,19 @@ +checking a.pdf +PDF Version: 1.4 +R = 3 +P = -4 +User password = +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: allowed +modify forms: allowed +modify annotations: allowed +modify other: allowed +modify anything: allowed +File is not linearized +No syntax or stream encoding errors found; the file may still contain +errors that qpdf cannot detect