Skip to content

Commit 985a5c4

Browse files
timleggetimlegge
committed
Add support for use_pkcs1_pss_padding
This change effectively reverses the fatal failure if use_pkcs1_padding is called and sets the padding to default to RSA_PKCS1_PSS_PADDING which is the recommended replacement for RSA_PKCS1_PADDING This will allow modules to continue to call use_pkcs1_padding but effectively replace it with the recommended alternative This may avoid changes to other modules but might not be what the user wants It should alos be noted that RSA_PKCS1_PSS_PADDING (and use_pkcs1_pss_padding should only be used for signing/verification operations
1 parent dbf24f4 commit 985a5c4

3 files changed

Lines changed: 52 additions & 23 deletions

File tree

‎RSA.pm‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,9 @@ Encrypting user data directly with RSA is insecure.
245245
246246
PKCS #1 v1.5 padding has been disabled as it is nearly impossible to use this
247247
padding method in a secure manner. It is known to be vulnerable to timing
248-
based side channel attacks. use_pkcs1_padding() results in a fatal error.
248+
based side channel attacks.
249+
250+
use_pkcs1_padding() now sets the padding method to use_pkcs1_pss_padding.
249251
250252
L<Marvin Attack|https://github.com/tomato42/marvin-toolkit/blob/master/README.md>
251253
@@ -256,6 +258,13 @@ an empty encoding parameter. This mode of padding is recommended for
256258
all new applications. It is the default mode used by
257259
C<Crypt::OpenSSL::RSA>.
258260
261+
=item use_pkcs1_pss_padding
262+
263+
Use C<RSA-PSS> padding as defined in PKCS#1 v2.1. In general, RSA-PSS
264+
should be used as a replacement for RSA-PKCS#1 v1.5. The module specifies
265+
the message digest being requested and the appropriate mgf1 setting and
266+
salt length for the digest.
267+
259268
=item use_sslv23_padding
260269
261270
Use C<PKCS #1 v1.5> padding with an SSL-specific modification that

‎RSA.xs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -927,14 +927,20 @@ void
927927
use_pkcs1_padding(p_rsa)
928928
rsaData* p_rsa;
929929
CODE:
930-
croak("PKCS#1 1.5 is disabled as it is known to be vulnerable to marvin attacks.");
930+
p_rsa->padding = RSA_PKCS1_PSS_PADDING;
931931

932932
void
933933
use_pkcs1_oaep_padding(p_rsa)
934934
rsaData* p_rsa;
935935
CODE:
936936
p_rsa->padding = RSA_PKCS1_OAEP_PADDING;
937937

938+
void
939+
use_pkcs1_pss_padding(p_rsa)
940+
rsaData* p_rsa;
941+
CODE:
942+
p_rsa->padding = RSA_PKCS1_PSS_PADDING;
943+
938944
#if OPENSSL_VERSION_NUMBER < 0x30000000L
939945

940946
void
@@ -977,7 +983,10 @@ sign(p_rsa, text_SV)
977983

978984
int md_status;
979985
CHECK_OPEN_SSL((md_status = EVP_PKEY_CTX_set_signature_md(ctx, md)) > 0);
980-
986+
if (p_rsa->padding == RSA_PKCS1_PSS_PADDING) {
987+
CHECK_OPEN_SSL((md_status = EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md)) > 0);
988+
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, RSA_PSS_SALTLEN_DIGEST) > 0);
989+
}
981990
CHECK_OPEN_SSL(EVP_PKEY_sign(ctx, NULL, &signature_length, digest, get_digest_length(p_rsa->hashMode)) == 1);
982991

983992
//signature = OPENSSL_malloc(signature_length);
@@ -1034,6 +1043,10 @@ PPCODE:
10341043

10351044
int md_status;
10361045
CHECK_OPEN_SSL((md_status = EVP_PKEY_CTX_set_signature_md(ctx, md)) > 0);
1046+
if (p_rsa->padding == RSA_PKCS1_PSS_PADDING) {
1047+
CHECK_OPEN_SSL((md_status = EVP_PKEY_CTX_set_rsa_mgf1_md(ctx, md)) > 0);
1048+
CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, RSA_PSS_SALTLEN_DIGEST) > 0);
1049+
}
10371050

10381051
switch (EVP_PKEY_verify(ctx, sig, sig_length, digest, get_digest_length(p_rsa->hashMode)))
10391052
#else

‎t/rsa.t‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use Crypt::OpenSSL::RSA;
66
use Crypt::OpenSSL::Guess qw(openssl_version);
77

88
BEGIN {
9-
plan tests => 37 + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ? 4 * 5 : 0 ) + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_whirlpool_hash" ) ? 1 * 5 : 0 );
9+
plan tests => 97 + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ? 4 * 5 : 0 ) + ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_whirlpool_hash" ) ? 1 * 5 : 0 );
1010
}
1111

1212
sub _Test_Encrypt_And_Decrypt {
@@ -37,16 +37,16 @@ sub _Test_Sign_And_Verify {
3737
my $sig = eval { $rsa->sign($plaintext) };
3838
SKIP: {
3939
skip "OpenSSL error: illegal or unsupported padding mode - $hash", 5 if $@ =~ /illegal or unsupported padding mode/i;
40-
ok( $rsa_pub->verify( $plaintext, $sig ) );
40+
ok( $rsa_pub->verify( $plaintext, $sig ), "rsa_pub verify $hash");
4141

4242
my $false_sig = unpack "H*", $sig;
4343
$false_sig =~ tr/[a-f]/[0a-d]/;
44-
ok( !$rsa_pub->verify( $plaintext, pack( "H*", $false_sig ) ) );
45-
ok( !$rsa->verify( $plaintext, pack( "H*", $false_sig ) ) );
44+
ok( !$rsa_pub->verify( $plaintext, pack( "H*", $false_sig ) ), "rsa_pub do not verify invalid $hash" );
45+
ok( !$rsa->verify( $plaintext, pack( "H*", $false_sig ) ), "rsa do not verify invalid $hash" );
4646

4747
my $sig_of_other = $rsa->sign("different");
48-
ok( !$rsa_pub->verify( $plaintext, $sig_of_other ) );
49-
ok( !$rsa->verify( $plaintext, $sig_of_other ) );
48+
ok( !$rsa_pub->verify( $plaintext, $sig_of_other ), "rsa_pub do not verify unmatching message" );
49+
ok( !$rsa->verify( $plaintext, $sig_of_other ), "rsa do not verify unmatching message");
5050
}
5151
}
5252

@@ -69,8 +69,8 @@ Crypt::OpenSSL::RSA->import_random_seed();
6969

7070
ok( Crypt::OpenSSL::RSA->generate_key(512)->size() * 8 == 512 );
7171

72-
my $rsa = Crypt::OpenSSL::RSA->generate_key(1024);
73-
ok( $rsa->size() * 8 == 1024 );
72+
my $rsa = Crypt::OpenSSL::RSA->generate_key(2048);
73+
ok( $rsa->size() * 8 == 2048 );
7474
ok( $rsa->check_key() );
7575

7676
$rsa->use_no_padding();
@@ -121,31 +121,38 @@ _check_for_croak(
121121

122122
$plaintext .= $plaintext x 5;
123123

124-
# check signature algorithms
125-
$rsa->use_md5_hash();
126-
$rsa_pub->use_md5_hash();
127-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "md5" );
124+
my @paddings = qw/pkcs1 pkcs1_oaep pkcs1_pss/;
125+
foreach my $padding (@paddings) {
126+
my $p = "use_$padding\_padding";
128127

129-
$rsa->use_sha1_hash();
130-
$rsa_pub->use_sha1_hash();
131-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha1" );
128+
$rsa->$p;
129+
$rsa_pub->$p;
130+
# check signature algorithms
131+
$rsa->use_md5_hash();
132+
$rsa_pub->use_md5_hash();
133+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "md5 with $padding padding" );
132134

133-
if ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ) {
135+
$rsa->use_sha1_hash();
136+
$rsa_pub->use_sha1_hash();
137+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha1 with $padding padding" );
138+
139+
if ( UNIVERSAL::can( "Crypt::OpenSSL::RSA", "use_sha512_hash" ) ) {
134140
$rsa->use_sha224_hash();
135141
$rsa_pub->use_sha224_hash();
136-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha224" );
142+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha224 with $padding padding" );
137143

138144
$rsa->use_sha256_hash();
139145
$rsa_pub->use_sha256_hash();
140-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha256" );
146+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha256 with $padding padding" );
141147

142148
$rsa->use_sha384_hash();
143149
$rsa_pub->use_sha384_hash();
144-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha384" );
150+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha384 with $padding padding" );
145151

146152
$rsa->use_sha512_hash();
147153
$rsa_pub->use_sha512_hash();
148-
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha512" );
154+
_Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub, "sha512 with $padding padding" );
155+
}
149156
}
150157

151158
my ( $major, $minor, $patch ) = openssl_version();

0 commit comments

Comments
 (0)