Skip to content

Commit b24e1c0

Browse files
committed
Make padding selection clearer and update documentation
Also add t/padding.t that attempts to test the paddingi combinations more thoroughly or possible more clearly?
1 parent bb6cf94 commit b24e1c0

3 files changed

Lines changed: 205 additions & 6 deletions

File tree

RSA.pm

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ Crypt::OpenSSL::RSA - RSA encoding and decoding, using the openSSL libraries
8585
Version 0.35 makes the use of PKCS#1 v1.5 padding a fatal error. It is
8686
very difficult to implement PKCS#1 v1.5 padding securely. If you are still
8787
using RSA in in general, you should be looking at alternative encryption
88-
algorithms.
88+
algorithms. Version 0.36 implements RSA-PSS padding (PKCS#1 v2.1) and makes
89+
setting an invalid padding a fatal error. Note, PKCS1_OAEP can only be used
90+
for encryption and PKCS1_PSS can only be used for signing.
8991
9092
=head1 DESCRIPTION
9193
@@ -112,6 +114,10 @@ C<-----BEGIN...-----> and C<-----END...-----> lines.
112114
The padding is set to PKCS1_OAEP, but can be changed with the
113115
C<use_xxx_padding> methods.
114116
117+
Note, PKCS1_OAEP can only be used for encryption. You must specifically
118+
call use_pkcs1_pss_padding (or use_pkcs1_pss_padding) prior to signing
119+
operations.
120+
115121
=item new_private_key
116122
117123
Create a new C<Crypt::OpenSSL::RSA> object by loading a private key in
@@ -235,6 +241,22 @@ Sign a string using the secret (portion of the) key.
235241
236242
Check the signature on a text.
237243
244+
=back
245+
246+
=head1 Padding Methods
247+
248+
Versions prior to 0.35 allowed using pkcs1 padding for both encryption
249+
and signature operations but has been disabled for security reasons.
250+
251+
While B<use_no_padding> can be used for encryption or signature operations
252+
B<use_pkcs1_pss_padding> is used for signature operations and
253+
B<use_pkcs1_oaep_padding> is used for encryption operations.
254+
255+
Version 0.38 sets the appropriate padding for each operation unless
256+
B<use_no_padding> is called before either operation.
257+
258+
=over
259+
238260
=item use_no_padding
239261
240262
Use raw RSA encryption. This mode should only be used to implement
@@ -254,7 +276,7 @@ L<Marvin Attack|https://github.com/tomato42/marvin-toolkit/blob/master/README.md
254276
Use C<EME-OAEP> padding as defined in PKCS #1 v2.0 with SHA-1, MGF1 and
255277
an empty encoding parameter. This mode of padding is recommended for
256278
all new applications. It is the default mode used by
257-
C<Crypt::OpenSSL::RSA>.
279+
C<Crypt::OpenSSL::RSA> but is only valid for encryption/decryption.
258280
259281
=item use_pkcs1_pss_padding
260282
@@ -273,6 +295,12 @@ denotes that the server is SSL3 capable.
273295
274296
Not available since OpenSSL 3.
275297
298+
=back
299+
300+
=head1 Hash/Digest Methods
301+
302+
=over
303+
276304
=item use_md5_hash
277305
278306
Use the RFC 1321 MD5 hashing algorithm by Ron Rivest when signing and

RSA.xs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,11 @@ SV* rsa_crypt(rsaData* p_rsa, SV* p_from,
339339
CHECK_OPEN_SSL(ctx);
340340

341341
CHECK_OPEN_SSL(init_crypt(ctx) == 1);
342-
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding) > 0);
342+
int crypt_pad = p_rsa->padding;
343+
if (p_rsa->padding != RSA_NO_PADDING) {
344+
crypt_pad = RSA_PKCS1_OAEP_PADDING;
345+
}
346+
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, crypt_pad) > 0);
343347
CHECK_OPEN_SSL(p_crypt(ctx, NULL, &to_length, from, from_length) == 1);
344348
CHECK_OPEN_SSL(p_crypt(ctx, to, &to_length, from, from_length) == 1);
345349

@@ -980,7 +984,11 @@ sign(p_rsa, text_SV)
980984
CHECK_OPEN_SSL(ctx);
981985
CHECK_OPEN_SSL(EVP_PKEY_sign_init(ctx));
982986
/* FIXME: Issue setting padding in some cases */
983-
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding) > 0);
987+
int sign_pad = p_rsa->padding;
988+
if (p_rsa->padding != RSA_NO_PADDING) {
989+
sign_pad = RSA_PKCS1_PSS_PADDING;
990+
}
991+
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, sign_pad) > 0);
984992

985993
EVP_MD* md = get_md_bynid(p_rsa->hashMode);
986994
CHECK_OPEN_SSL(md != NULL);
@@ -1040,8 +1048,11 @@ PPCODE:
10401048
CHECK_OPEN_SSL(ctx);
10411049
CHECK_OPEN_SSL(EVP_PKEY_verify_init(ctx) == 1);
10421050
/* FIXME: Issue setting padding in some cases */
1043-
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding) > 0);
1044-
1051+
int verify_pad = p_rsa->padding;
1052+
if (p_rsa->padding != RSA_NO_PADDING) {
1053+
verify_pad = RSA_PKCS1_PSS_PADDING;
1054+
}
1055+
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, verify_pad) > 0);
10451056
EVP_MD* md = get_md_bynid(p_rsa->hashMode);
10461057
CHECK_OPEN_SSL(md != NULL);
10471058

t/padding.t

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
use strict;
2+
use Test::More;
3+
4+
use Crypt::OpenSSL::Random;
5+
use Crypt::OpenSSL::RSA;
6+
use Crypt::OpenSSL::Guess qw(openssl_version);
7+
8+
my ($major, $minor, $patch) = openssl_version;
9+
10+
BEGIN {
11+
plan tests => 87 + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ? 4 * 5 : 0 );
12+
}
13+
14+
sub _Test_Encrypt_And_Decrypt {
15+
my ( $p_plaintext_length, $p_rsa, $p_check_private_encrypt, $padding, $hash ) = @_;
16+
17+
my ( $ciphertext, $decoded_text );
18+
my $plaintext = pack(
19+
"C${p_plaintext_length}",
20+
(
21+
1, 255, 0, 128, 4, # Make sure these characters work
22+
map { int( rand 256 ) } ( 1 .. $p_plaintext_length - 5 )
23+
)
24+
);
25+
ok( $ciphertext = $p_rsa->encrypt($plaintext), "Padding method $padding is valid for encrypting with $hash" );
26+
ok( $decoded_text = $p_rsa->decrypt($ciphertext), "Padding method $padding is valid for decrypting with $hash" );
27+
ok( $decoded_text eq $plaintext );
28+
29+
if ($p_check_private_encrypt) {
30+
ok( $ciphertext = $p_rsa->private_encrypt($plaintext), "Padding method $padding is valid for private_encrypt with $hash" );
31+
ok( $decoded_text = $p_rsa->public_decrypt($ciphertext), "Padding method $padding is valid for private_decrypt with $hash" );
32+
ok( $decoded_text eq $plaintext );
33+
}
34+
}
35+
36+
sub _Test_Sign_And_Verify {
37+
my ( $p_plaintext_length, $rsa, $rsa_pub, $padding, $hash ) = @_;
38+
39+
my $plaintext = pack(
40+
"C${p_plaintext_length}",
41+
(
42+
1, 255, 0, 128, 4, # Make sure these characters work
43+
map { int( rand 256 ) } ( 1 .. $p_plaintext_length - 5 )
44+
)
45+
);
46+
47+
my $sig = eval { $rsa->sign($plaintext) };
48+
49+
SKIP: {
50+
skip "OpenSSL error: illegal or unsupported padding mode - $hash", 6 if $@ =~ /illegal or unsupported padding mode/i;
51+
skip "OpenSSL error: invalid digest - $hash", 6 if $@ =~ /invalid digest/i;
52+
ok(!$@, "Padding method $padding is valid for signing with $hash");
53+
ok( $rsa_pub->verify( $plaintext, $sig ), "Padding method $padding is valid for verifying with $hash");
54+
55+
my $false_sig = unpack "H*", $sig;
56+
$false_sig =~ tr/[a-f]/[0a-d]/;
57+
ok( !$rsa_pub->verify( $plaintext, pack( "H*", $false_sig )), "rsa_pub: False signature does not verify");
58+
ok( !$rsa->verify( $plaintext, pack( "H*", $false_sig )), "rsa: False signature does not verify");
59+
60+
my $sig_of_other = $rsa->sign("different");
61+
ok( !$rsa_pub->verify( $plaintext, $sig_of_other ), "rsa_pub: plaintext does not match signature" );
62+
ok( !$rsa->verify( $plaintext, $sig_of_other ), "rsa: plaintext does not match signature");
63+
}
64+
}
65+
66+
Crypt::OpenSSL::Random::random_seed("OpenSSL needs at least 32 bytes.");
67+
Crypt::OpenSSL::RSA->import_random_seed();
68+
69+
my $rsa = Crypt::OpenSSL::RSA->generate_key(2048);
70+
ok( $rsa->size() * 8 == 2048);
71+
ok( $rsa->check_key() );
72+
73+
my $private_key_string = $rsa->get_private_key_string();
74+
my $public_key_string = $rsa->get_public_key_string();
75+
76+
ok( $private_key_string and $public_key_string );
77+
78+
my $plaintext = "The quick brown fox jumped over the lazy dog";
79+
my $rsa_priv = Crypt::OpenSSL::RSA->new_private_key($private_key_string);
80+
ok( $plaintext eq $rsa_priv->decrypt( $rsa_priv->encrypt($plaintext) ) );
81+
82+
my $rsa_pub = Crypt::OpenSSL::RSA->new_public_key($public_key_string);
83+
84+
my @unsupported_paddings = qw/pkcs1 sslv23/;
85+
86+
$plaintext .= $plaintext x 5;
87+
# pkcs1 sslv23 are unsupported methods
88+
foreach my $pad (@unsupported_paddings) {
89+
my $method = "use_${pad}_padding";
90+
SKIP: {
91+
skip "OpenSSL version less than 3.0 supports sslv23", 1
92+
if $major lt '3.0' && $pad eq 'sslv23';
93+
eval {
94+
$rsa->$method;
95+
};
96+
ok($@, "Padding method $pad unsupported");
97+
}
98+
}
99+
100+
my @supported_paddings = qw/no pkcs1_pss pkcs1_oaep/;
101+
# no pkcs1_pss pkcs1_oaep are supported methods
102+
foreach my $pad (@supported_paddings) {
103+
my $method = "use_${pad}_padding";
104+
eval {
105+
$rsa->$method;
106+
};
107+
ok(!$@, "Padding method $pad supported");
108+
}
109+
110+
my @hashes = qw/md5 sha1 sha224 sha256 sha384 sha512 ripemd160/; # whirlpool/;
111+
112+
my %padding_methods = (
113+
'no' => {'sign' => 1, 'encrypt' => 1, 'pad' => 0},
114+
'pkcs1_pss' => {'sign' => 1, 'encrypt' => 0, 'pad' => 1},
115+
'pkcs1_oaep' => {'sign' => 0, 'encrypt' => 1, 'pad' => 42},
116+
'pkcs1' => {'sign' => 0, 'encrypt' => 0, 'pad' => 11},
117+
#'sslv23' => {'sign' => 0, 'encrypt' => 0, 'pad' => 11},
118+
);
119+
120+
121+
foreach my $padding (keys %padding_methods) {
122+
diag $padding;
123+
foreach my $hash (@hashes) {
124+
next if $hash ne 'sha256' && $padding eq 'x931';
125+
my $props = $padding_methods{$padding};
126+
my $sign = $props->{sign};
127+
my $encrypt = $props->{encrypt};
128+
my $pad = $props->{pad};
129+
130+
my $hash_mth = "use_${hash}_hash";
131+
$rsa->$hash_mth;
132+
$rsa_pub->$hash_mth;
133+
my $method = "use_${padding}_padding";
134+
if ($sign || $encrypt ) {
135+
$rsa->$method;
136+
$rsa_pub->$method;
137+
}
138+
# Valid signing methods
139+
if ($sign && $pad) {
140+
_Test_Sign_And_Verify( $rsa->size() - $pad, $rsa, $rsa_pub, $padding, $hash );
141+
}
142+
143+
# Invalid signing methods
144+
if ((!$sign) && $pad) {
145+
eval {
146+
$rsa->$method;
147+
$rsa->sign($plaintext);
148+
};
149+
ok(defined $@, "Padding $padding is invalid for signing");
150+
}
151+
152+
# Valid encryption methods with padding
153+
if ($encrypt) {
154+
_Test_Encrypt_And_Decrypt( $rsa->size() - $pad, $rsa, 0, $padding, $hash );
155+
}
156+
157+
}
158+
}
159+
160+
# Try

0 commit comments

Comments
 (0)