APPS/{ecparam,pkeyparam}: fix case where infile and outfile are the same

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25552)

diff --git a/apps/ecparam.c b/apps/ecparam.c
index 71f93c4ca5e142375ad4e65e3148b2c22301e4a2..35899522d29767d0589982bf76e562b57f0bb01a 100644 (file)
--- a/apps/ecparam.c
+++ b/apps/ecparam.c
@@ -194,16 +194,6 @@ int ecparam_main(int argc, char **argv)
 
     private = genkey ? 1 : 0;
 
-    out = bio_open_owner(outfile, outformat, private);
-    if (out == NULL)
-        goto end;
-
-    if (list_curves) {
-        if (list_builtin_curves(out))
-            ret = 0;
-        goto end;
-    }
-
     if (curve_name != NULL) {
         OSSL_PARAM params[4];
         OSSL_PARAM *p = params;
@@ -276,6 +266,16 @@ int ecparam_main(int argc, char **argv)
         goto end;
     }
 
+    out = bio_open_owner(outfile, outformat, private);
+    if (out == NULL)
+        goto end;
+
+    if (list_curves) {
+        if (list_builtin_curves(out))
+            ret = 0;
+        goto end;
+    }
+
     if (text
         && !EVP_PKEY_print_params(out, params_key, 0, NULL)) {
         BIO_printf(bio_err, "unable to print params\n");

diff --git a/apps/pkeyparam.c b/apps/pkeyparam.c
index 4fefe69e6b3cd334f57e19db7baeeaab690620d9..fd8de0e02fe91b28bbcf82ce86af79fbc7aa5725 100644 (file)
--- a/apps/pkeyparam.c
+++ b/apps/pkeyparam.c
@@ -97,9 +97,6 @@ int pkeyparam_main(int argc, char **argv)
     in = bio_open_default(infile, 'r', FORMAT_PEM);
     if (in == NULL)
         goto end;
-    out = bio_open_default(outfile, 'w', FORMAT_PEM);
-    if (out == NULL)
-        goto end;
     pkey = PEM_read_bio_Parameters_ex(in, NULL, app_get0_libctx(),
                                       app_get0_propq());
     if (pkey == NULL) {
@@ -107,6 +104,9 @@ int pkeyparam_main(int argc, char **argv)
         ERR_print_errors(bio_err);
         goto end;
     }
+    out = bio_open_default(outfile, 'w', FORMAT_PEM);
+    if (out == NULL)
+        goto end;
 
     if (check) {
         if (e == NULL)

diff --git a/doc/man1/openssl-ecparam.pod.in b/doc/man1/openssl-ecparam.pod.in
index 458505051493580bf835fa03e0f3a831139c07c1..27ce10490fe25d6e2f8aea1f4e8d1df85b9166ed 100644 (file)
--- a/doc/man1/openssl-ecparam.pod.in
+++ b/doc/man1/openssl-ecparam.pod.in
@@ -55,14 +55,16 @@ Parameters are encoded as B<EcpkParameters> as specified in IETF RFC 3279.
 
 =item B<-in> I<filename>
 
-This specifies the input filename to read parameters from or standard input if
+This specifies the input file to read parameters from or standard input if
 this option is not specified.
 
 =item B<-out> I<filename>
 
 This specifies the output filename parameters to. Standard output is used
-if this option is not present. The output filename should B<not> be the same
-as the input filename.
+if this option is not present.
+The output filename can be the same as the input filename,
+which leads to replacing the file contents.
+Note that file I/O is not atomic. The output file is truncated and then written.
 
 =item B<-noout>
 

diff --git a/doc/man1/openssl-pkeyparam.pod.in b/doc/man1/openssl-pkeyparam.pod.in
index 5053316c6880ae7b50a6c7926549ce1429e85688..b8b1792f28947e932db7be2e7d8ca49c71993745 100644 (file)
--- a/doc/man1/openssl-pkeyparam.pod.in
+++ b/doc/man1/openssl-pkeyparam.pod.in
@@ -35,13 +35,16 @@ Print out a usage message.
 
 =item B<-in> I<filename>
 
-This specifies the input filename to read parameters from or standard input if
+This specifies the input file to read parameters from or standard input if
 this option is not specified.
 
 =item B<-out> I<filename>
 
 This specifies the output filename to write parameters to or standard output if
 this option is not specified.
+The output filename can be the same as the input filename,
+which leads to replacing the file contents.
+Note that file I/O is not atomic. The output file is truncated and then written.
 
 =item B<-text>
 

diff --git a/test/recipes/15-test_ecparam.t b/test/recipes/15-test_ecparam.t
index 5dba866378a6dc076d38112b0f7856d71f55cae6..869ff301ea7a5a8fddc7e565ea0527b6bd365e47 100644 (file)
--- a/test/recipes/15-test_ecparam.t
+++ b/test/recipes/15-test_ecparam.t
@@ -11,7 +11,8 @@ use strict;
 use warnings;
 
 use File::Spec;
-use File::Compare qw/compare_text/;
+use File::Copy;
+use File::Compare qw/compare_text compare/;
 use OpenSSL::Glob;
 use OpenSSL::Test qw/:DEFAULT data_file srctop_file bldtop_dir/;
 use OpenSSL::Test::Utils;
@@ -29,7 +30,7 @@ if (disabled("sm2")) {
     @valid = grep { !/sm2-.*\.pem/} @valid;
 }
 
-plan tests => 12;
+plan tests => 13;
 
 sub checkload {
     my $files = shift; # List of files
@@ -63,6 +64,19 @@ sub checkcompare {
     }
 }
 
+sub check_identical {
+    my $apps = shift; # List of applications
+
+    foreach (@$apps) {
+        my $inout = "$_.tst";
+        my $backup = "backup.tst";
+
+        copy($inout, $backup);
+        ok(run(app(['openssl', $_, '-in', $inout, '-out', $inout])));
+        ok(!compare($inout, $backup), "converted file $inout did not change");
+    }
+}
+
 my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0);
 
 subtest "Check loading valid parameters by ecparam with -check" => sub {
@@ -120,6 +134,12 @@ subtest "Check pkeyparam does not change the parameter file on output" => sub {
     checkcompare(\@valid, "pkeyparam");
 };
 
+my @apps = ("ecparam", "pkeyparam");
+subtest "Check param apps do not garble infile identical to outfile" => sub {
+    plan tests => 2 * scalar(@apps);
+    check_identical(\@apps);
+};
+
 subtest "Check loading of fips and non-fips params" => sub {
     plan skip_all => "FIPS is disabled"
         if $no_fips;