[Bug] apps: -CApath does not fail for non-directories (on Linux)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Bug] apps: -CApath does not fail for non-directories (on Linux)

Steffen Nurpmeso-2
Hello.

I am sorry, but i have no github account.  Maybe it is possible to
have some @bug address which creates issues automatically?
I see this on

  ? openssl version
  OpenSSL 1.0.2k  26 Jan 2017
  ? /home/steffen/usr/opt/.ssl-1.1.0/bin/openssl version                                                                      
  OpenSSL 1.1.0  25 Aug 2016
  ? openssl verify -verbose -CAfile SOME-FILE  SOME-CERT
  SOME-FILE: OK
  ? openssl verify -verbose -CApath SOME-FILE  SOME-CERT
  error 20 at 0 depth lookup:unable to get local issuer certificate

I look into apps/apps.c, but i cannot tell why this does not bail
out when the directory is found to be a file.
Ciao,

--steffen
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Bug] apps: -CApath does not fail for non-directories (on Linux)

Salz, Rich
> I am sorry, but i have no github account.  Maybe it is possible to have some
> @bug address which creates issues automatically?

Nah, too much spam will happen :(  Posting to openssl-dev is fine.

>   ? openssl version
>   OpenSSL 1.0.2k  26 Jan 2017
>   ? /home/steffen/usr/opt/.ssl-1.1.0/bin/openssl version
>   OpenSSL 1.1.0  25 Aug 2016

So this happens on both 1.0.2 and 1.1.0?

Does the following patch fix it for 1.1.0?

; git diff
diff --git a/apps/opt.c b/apps/opt.c
index 499a0b5..4547da1 100644
--- a/apps/opt.c
+++ b/apps/opt.c
@@ -661,7 +661,8 @@ int opt_next(void)
             return -1;
         case '<':
             /* Input file. */
-            if (strcmp(arg, "-") == 0 || app_access(arg, R_OK) >= 0)
+            if (strcmp(arg, "-") == 0
+                    || (!app_isdir(arg) && app_access(arg, R_OK) >= 0))
                 break;
             BIO_printf(bio_err,
                        "%s: Cannot open input file %s, %s\n",



--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Bug] apps: -CApath does not fail for non-directories (on Linux)

Steffen Nurpmeso-2
Sorry for the late reply, this really is a slow machine (and
i cleanup again completely anything once it is installed, _and_
the tests compile a long time even if not run)..

"Salz, Rich" <[hidden email]> wrote:
 |> I am sorry, but i have no github account.  Maybe it is possible to \
 |> have some
 |> @bug address which creates issues automatically?
 |
 |Nah, too much spam will happen :(  Posting to openssl-dev is fine.

Ok, good.

 |>   ? openssl version
 |>   OpenSSL 1.0.2k  26 Jan 2017
 |>   ? /home/steffen/usr/opt/.ssl-1.1.0/bin/openssl version
 |>   OpenSSL 1.1.0  25 Aug 2016
 |
 |So this happens on both 1.0.2 and 1.1.0?
 |
 |Does the following patch fix it for 1.1.0?

No, not that i know.  But this -- thanks -- lead me to the
following, which is the KISS that you want?
Ciao!

diff --git a/apps/apps.c b/apps/apps.c
index 216bc797d..3afbbaef2 100644
--- a/apps/apps.c
+++ b/apps/apps.c
@@ -1221,7 +1221,8 @@ X509_STORE *setup_verify(const char *CAfile, const char *CApath, int noCAfile, i
         if (lookup == NULL)
             goto end;
         if (CApath) {
-            if (!X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_PEM)) {
+            if (!app_isdir(CApath) ||
+                    !X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_PEM)) {
                 BIO_printf(bio_err, "Error loading directory %s\n", CApath);
                 goto end;
             }

--steffen
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Bug] apps: -CApath does not fail for non-directories (on Linux)

Viktor Dukhovni

> On Mar 1, 2017, at 11:46 AM, Steffen Nurpmeso <[hidden email]> wrote:
>
> No, not that i know.  But this -- thanks -- lead me to the
> following, which is the KISS that you want?
> Ciao!
>
> diff --git a/apps/apps.c b/apps/apps.c
> index 216bc797d..3afbbaef2 100644
> --- a/apps/apps.c
> +++ b/apps/apps.c
> @@ -1221,7 +1221,8 @@ X509_STORE *setup_verify(const char *CAfile, const char *CApath, int noCAfile, i
>         if (lookup == NULL)
>             goto end;
>         if (CApath) {
> -            if (!X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_PEM)) {
> +            if (!app_isdir(CApath) ||
> +                    !X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_PEM)) {
>                 BIO_printf(bio_err, "Error loading directory %s\n", CApath);
>                 goto end;
>             }

We may need to be careful.  With OpenSSL <= 1.0.2, one way to suppress the
built-in default CApath was to set "-CApath" to a non-existent directory.
Users may have scripts relying on this behaviour.  Now with 1.1.0 on some
platforms OpenSSL already rejects non-existent directories, and we also
provide a "-no-CAfile" option, but this change will extend the change to
what is likely our most popular platform.

So it will at least deserve a comment in the "NEWS"/"CHANGES" files.  

--
        Viktor.

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Bug] apps: -CApath does not fail for non-directories (on Linux)

Steffen Nurpmeso-2
Hello again,

Viktor Dukhovni <[hidden email]> wrote:
 |> On Mar 1, 2017, at 11:46 AM, Steffen Nurpmeso <[hidden email]> wrote:
 |> No, not that i know.  But this -- thanks -- lead me to the
 |> following, which is the KISS that you want?
 ...
 |> diff --git a/apps/apps.c b/apps/apps.c
 |> index 216bc797d..3afbbaef2 100644
 |> --- a/apps/apps.c
 |> +++ b/apps/apps.c
 |> @@ -1221,7 +1221,8 @@ X509_STORE *setup_verify(const char *CAfile, \
 |> const char *CApath, int noCAfile, i
 |>         if (lookup == NULL)
 |>             goto end;
 |>         if (CApath) {
 |> -            if (!X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_PEM))\
 |>  {
 |> +            if (!app_isdir(CApath) ||
 |> +                    !X509_LOOKUP_add_dir(lookup, CApath, X509_FILETYPE_P\
 |> EM)) {
 |>                 BIO_printf(bio_err, "Error loading directory %s\n", \
 |>                 CApath);
 |>                 goto end;
 |>}
 |
 |We may need to be careful.  With OpenSSL <= 1.0.2, one way to suppress the
 |built-in default CApath was to set "-CApath" to a non-existent directory.
 |Users may have scripts relying on this behaviour.  Now with 1.1.0 on some
 |platforms OpenSSL already rejects non-existent directories, and we also
 |provide a "-no-CAfile" option, but this change will extend the change to
 |what is likely our most popular platform.
 |
 |So it will at least deserve a comment in the "NEWS"/"CHANGES" files.  

I understand that this is a "lingering" configuration, which is
inspected on the fly as a last (or second last, if there is
SSL_CTX_set_default_verify_paths(3).  In fact i don't know, the
documentation is horrific!  E.g., just today i have implemented
some *{smime,ssl}-ca-flags*, mostly to be able to provide
X509_V_FLAG_PARTIAL_CHAIN, but you know you can't find just any
X509_STORE_set_flags(_not_ 3) at all, it is, again, _horrific_.
I for one, as someone unrelated, now that you have so many
sponsors, i really wonder why you don't spend a little bit of
money to some crypto nerd student(s) who really need(s) it in
order to improve it.  Really, and honestly.) resort.

But for an application that does a one-shot check i would at least
expect a message that leads into the right direction, at least
with -verbose.  I for one _of course_ looked for
X509_LOOKUP_add_dir() first, but i gave up after the second
indirection.  If you would ask me, i surely would add
a X509_FILETYPE_CHECK_OR_ERROR or similar flag to get the
synchronous checks.  But who am i.

--steffen
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Loading...