Require --allow-insecure to create certain encrypted files (fixes #501)

For now, --allow-insecure allows creation of files with the owner
passwords empty or matching the user password.
This commit is contained in:
Jay Berkenbilt 2021-02-04 15:55:41 -05:00
parent faa2e3ddfd
commit 21b0f4acfc
5 changed files with 137 additions and 11 deletions

View File

@ -1,3 +1,12 @@
2021-02-04 Jay Berkenbilt <ejb@ql.org>
* 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 <ejb@ql.org>
* Bug fix: if a form XObject lacks a resources dictionary,

View File

@ -1214,7 +1214,11 @@ make
</para>
<para>
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
<option>--allow-insecure</option>, as described below.
</para>
<para>
The value for
@ -1223,6 +1227,25 @@ make
When no additional restrictions are given, the default is to be
fully permissive.
</para>
<para>
For all key lengths, the following options are available:
<variablelist>
<varlistentry>
<term><option>--allow-insecure</option></term>
<listitem>
<para>
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.
</para>
</listitem>
</varlistentry>
</variablelist>
</para>
<para>
If <option><replaceable>key-length</replaceable></option> is 40,
the following restriction options are available:
@ -4824,7 +4847,28 @@ print "\n";
<itemizedlist>
<listitem>
<para>
Behavior Changes
CLI Behavior Changes
</para>
<itemizedlist>
<listitem>
<para>
By default, <command>qpdf</command> no longer allows
creation of encrypted PDF files whose owner password is
empty or matches the user password. The
<option>--allow-insecure</option>, specified inside the
<option>--encrypt</option> 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.
</para>
</listitem>
</itemizedlist>
</listitem>
<listitem>
<para>
Library Behavior Changes
</para>
<itemizedlist>
<listitem>

View File

@ -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))
{

View File

@ -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"},

View File

@ -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