Mailing List Archive

Pull Request (patch libgcrypt)
From fd982bd34338d4824bf69c07b6776d75d1d88877 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ant=C3=B4nio=20Martos=20Harres?= <tom.mharres@gmail.com>
Date: Thu, 13 Aug 2020 00:20:47 -0300
Subject: [PATCH] Fix libgcrypt returning errno 2 (file not found)

I was coding with libcurl and decided to debug my code with a
watchpoint on errno, to my unpleasent surprise, I found that libgcrypt
was returning error, despite that I was doing everything okay and
libgcrypt wasn't really having a decent reason to return error.
So I found the reason why, apparently it was trying to open a file
that doesn't exist, now fips_enabled doesn't actually *need* to exist
by design, so libgcrypt should not set errno if it doesn't exist
---
src/fips.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/fips.c b/src/fips.c
index 1ac7f477..9ff5e578 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -137,9 +137,11 @@ _gcry_initialize_fips_mode (int force)
{
static const char procfname[] = "/proc/sys/crypto/fips_enabled";
FILE *fp;
- int saved_errno;
-
+ int saved_errno = errno;
+ /* since procfname may not exist and that's okay, we should ignore
+ any changes that fopen does to errno. */
fp = fopen (procfname, "r");
+ errno = saved_errno;
if (fp)
{
char line[256];
@@ -197,9 +199,11 @@ _gcry_initialize_fips_mode (int force)
}


+ int saved_errno = errno; /* since FIPS_FORCE_FILE may not
exist, we ignore any error set by fopen */
/* If the FIPS force files exists, is readable and has a number
!= 0 on its first line, we enable the enforced fips mode. */
fp = fopen (FIPS_FORCE_FILE, "r");
+ errno = saved_errno;
if (fp)
{
char line[256];
Re: Pull Request (patch libgcrypt) [ In reply to ]
Hi!

> I was coding with libcurl and decided to debug my code with a
> watchpoint on errno, to my unpleasent surprise, I found that libgcrypt
> was returning error, despite that I was doing everything okay and
> libgcrypt wasn't really having a decent reason to return error.

Can you please describe the problem you are trying to address?

May I assume that you are under the impression that Libgcrypt may not
change ERRNO while you call an arbitrary function of it? That is not
the case. Maybe you should take another path to debuggng that
watchpointing ERRNO.


Shalom-Salam,

Werner

--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
Re: Pull Request (patch libgcrypt) [ In reply to ]
Hello, I will be as descriptive as possible about the issue here:
In order to probe if fips_mode is enabled in the operating system,
libgcrypt will try to fopen "/proc/sys/crypto/fips_enabled", now according
to libgcrypt documentation, this file may not exist...
If it doesn't, then libgcrypt fallsback to "/etc/gcrypt/fips_enabled", it
will again try to fopen it.
This procedure is described here:
https://www.gnupg.org/documentation/manuals/gcrypt/Enabling-FIPS-mode.html
The key point here is that the relevant portion of code is using fopen to
probe for the existence of the file, this may return all sorts of errors,
but commonly it's ENOENT. which is then returned into any code that is
initializing libgcrypt. But, I'm getting errno at something that is not an
error, rather, a configuration detail, the fact that the file doesn't exist
just means that libgcrypt should disable fips mode internally.
While describing the problem here, I understood a flaw in my patch, allow
me to send a new patch that will ignore errno only in case it's ENOENT.

Em qua., 19 de ago. de 2020 às 14:29, Werner Koch <wk@gnupg.org> escreveu:

> Hi!
>
> > I was coding with libcurl and decided to debug my code with a
> > watchpoint on errno, to my unpleasent surprise, I found that libgcrypt
> > was returning error, despite that I was doing everything okay and
> > libgcrypt wasn't really having a decent reason to return error.
>
> Can you please describe the problem you are trying to address?
>
> May I assume that you are under the impression that Libgcrypt may not
> change ERRNO while you call an arbitrary function of it? That is not
> the case. Maybe you should take another path to debuggng that
> watchpointing ERRNO.
>
>
> Shalom-Salam,
>
> Werner
>
> --
> Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
>
Re: Pull Request (patch libgcrypt) [ In reply to ]
diff --git a/src/fips.c b/src/fips.c
index 1ac7f477..c28efaef 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -138,8 +138,17 @@ _gcry_initialize_fips_mode (int force)
static const char procfname[] = "/proc/sys/crypto/fips_enabled";
FILE *fp;
int saved_errno;
-
+ saved_errno = errno;
+ /* since procfname may not exist and that's okay, we should ignore
+ if fopen sets errno to ENOENT (no such file) */
fp = fopen (procfname, "r");
+ /* if file doesn't exist, which is a condition described here:
+ https://www.gnupg.org/documentation/manuals/gcrypt/Enabling-FIPS-mode.html
*/
+ if (errno == ENOENT)
+ {
+ /* restore errno's value before fopen call */
+ errno = saved_errno;
+ }
if (fp)
{
char line[256];
@@ -178,6 +187,7 @@ _gcry_initialize_fips_mode (int force)
{
/* Yes, we are in FIPS mode. */
FILE *fp;
+ int saved_errno;

/* Intitialize the lock to protect the FSM. */
err = gpgrt_lock_init (&fsm_lock);
@@ -197,9 +207,16 @@ _gcry_initialize_fips_mode (int force)
}


+ saved_errno = errno;
/* If the FIPS force files exists, is readable and has a number
!= 0 on its first line, we enable the enforced fips mode. */
fp = fopen (FIPS_FORCE_FILE, "r");
+ if (errno == ENOENT)
+ {
+ /* since FIPS_FORCE_FILE may not exist, we ignore if fopen
+ returns ENOENT (file not found) */
+ errno = saved_errno;
+ }
if (fp)
{
char line[256];


Em qui., 20 de ago. de 2020 às 19:57, Antonio Harres <tom.mharres@gmail.com>
escreveu:

> Hello, I will be as descriptive as possible about the issue here:
> In order to probe if fips_mode is enabled in the operating system,
> libgcrypt will try to fopen "/proc/sys/crypto/fips_enabled", now according
> to libgcrypt documentation, this file may not exist...
> If it doesn't, then libgcrypt fallsback to "/etc/gcrypt/fips_enabled", it
> will again try to fopen it.
> This procedure is described here:
> https://www.gnupg.org/documentation/manuals/gcrypt/Enabling-FIPS-mode.html
> The key point here is that the relevant portion of code is using fopen to
> probe for the existence of the file, this may return all sorts of errors,
> but commonly it's ENOENT. which is then returned into any code that is
> initializing libgcrypt. But, I'm getting errno at something that is not an
> error, rather, a configuration detail, the fact that the file doesn't exist
> just means that libgcrypt should disable fips mode internally.
> While describing the problem here, I understood a flaw in my patch, allow
> me to send a new patch that will ignore errno only in case it's ENOENT.
>
> Em qua., 19 de ago. de 2020 às 14:29, Werner Koch <wk@gnupg.org> escreveu:
>
>> Hi!
>>
>> > I was coding with libcurl and decided to debug my code with a
>> > watchpoint on errno, to my unpleasent surprise, I found that libgcrypt
>> > was returning error, despite that I was doing everything okay and
>> > libgcrypt wasn't really having a decent reason to return error.
>>
>> Can you please describe the problem you are trying to address?
>>
>> May I assume that you are under the impression that Libgcrypt may not
>> change ERRNO while you call an arbitrary function of it? That is not
>> the case. Maybe you should take another path to debuggng that
>> watchpointing ERRNO.
>>
>>
>> Shalom-Salam,
>>
>> Werner
>>
>> --
>> Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
>>
>
Re: Pull Request (patch libgcrypt) [ In reply to ]
I will also attach gdb's output, here my program is called tool.c. When it
attempts to initialize libcurl, it changes errno to ENOENT because of
/etc/gcrypt/fips_enabled, this also changes errno attached to libcurl,
which, in turn, makes my program exit with an "FIle/ directory not found
message".
I almost forgot, answering your previous question:
No, I am not under the impression that libgcrypt may not change errno when
I call any arbitrary function, but that this specific circumstance differs
from the documented behaviour by not considering /etc/gcrypt/fips_enabled
*may* not exist.
Also, my setup doesn't have neither files that gcrypt tries to open to
check fips mode, which is the debian default configuration.

Breakpoint 2, main (argc=1, argv=0x7fffffffe128) at tool.c:656
656 if ( curl_global_init(CURL_GLOBAL_ALL) != 0 ) { // init
libcurl
(gdb) watch *errno_p
Hardware watchpoint 3: *errno_p
(gdb) c
Continuing.

Hardware watchpoint 3: *errno_p

Old value = 0
New value = 2
0x00007ffff7e0171c in __access (file=0x7ffff76c335b
"/etc/gcrypt/fips_enabled", type=0) at
../sysdeps/unix/sysv/linux/access.c:27
27 ../sysdeps/unix/sysv/linux/access.c: No such file or directory.
(gdb) bt
#0 0x00007ffff7e0171c in __access (file=0x7ffff76c335b
"/etc/gcrypt/fips_enabled", type=0)
at ../sysdeps/unix/sysv/linux/access.c:27
#1 0x00007ffff760024e in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
#2 0x00007ffff75f88ea in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
#3 0x00007ffff75f9a5f in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
#4 0x00007ffff75f5789 in gcry_control () from
/lib/x86_64-linux-gnu/libgcrypt.so.20
#5 0x00007ffff7c76794 in libssh2_init () from
/lib/x86_64-linux-gnu/libssh2.so.1
#6 0x00007ffff7f428fb in ?? () from
/lib/x86_64-linux-gnu/libcurl-gnutls.so.4
#7 0x00007ffff7f032d7 in ?? () from
/lib/x86_64-linux-gnu/libcurl-gnutls.so.4
#8 0x0000555555556110 in main (argc=1, argv=0x7fffffffe128) at tool.c:656
(gdb)
Re: Pull Request (patch libgcrypt) [ In reply to ]
In this latest patch, I'm also verifying if access returns ENOENT (which
originally was raising errno for me).

diff --git a/src/fips.c b/src/fips.c
index 1ac7f477..43f70c75 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -101,6 +101,7 @@ _gcry_initialize_fips_mode (int force)
{
static int done;
gpg_error_t err;
+ int saved_errno;

/* Make sure we are not accidentally called twice. */
if (done)
@@ -127,8 +128,14 @@ _gcry_initialize_fips_mode (int force)
file. The filename is hardwired so that there won't be any
confusion on whether /etc/gcrypt/ or /usr/local/etc/gcrypt/ is
actually used. The file itself may be empty. */
+ saved_errno = errno;
if ( !access (FIPS_FORCE_FILE, F_OK) )
{
+ /* don't set errno for access if FIPS_FORCE_FILE doesn't exist */
+ if (errno == ENOENT)
+ {
+ errno = saved_errno;
+ }
gcry_assert (!_gcry_no_fips_mode_required);
goto leave;
}
@@ -137,9 +144,17 @@ _gcry_initialize_fips_mode (int force)
{
static const char procfname[] = "/proc/sys/crypto/fips_enabled";
FILE *fp;
- int saved_errno;
-
+ saved_errno = errno;
+ /* since procfname may not exist and that's okay, we should ignore
+ if fopen sets errno to ENOENT (no such file) */
fp = fopen (procfname, "r");
+ /* if file doesn't exist, which is a condition described here:
+ https://www.gnupg.org/documentation/manuals/gcrypt/Enabling-FIPS-mode.html
*/
+ if (errno == ENOENT)
+ {
+ /* restore errno's value before fopen call */
+ errno = saved_errno;
+ }
if (fp)
{
char line[256];
@@ -197,9 +212,16 @@ _gcry_initialize_fips_mode (int force)
}


+ saved_errno = errno;
/* If the FIPS force files exists, is readable and has a number
!= 0 on its first line, we enable the enforced fips mode. */
fp = fopen (FIPS_FORCE_FILE, "r");
+ if (errno == ENOENT)
+ {
+ /* since FIPS_FORCE_FILE may not exist, we ignore if fopen
+ returns ENOENT (file not found) */
+ errno = saved_errno;
+ }
if (fp)
{
char line[256];


Em qui., 20 de ago. de 2020 às 21:05, Antonio Harres <tom.mharres@gmail.com>
escreveu:

> I will also attach gdb's output, here my program is called tool.c. When it
> attempts to initialize libcurl, it changes errno to ENOENT because of
> /etc/gcrypt/fips_enabled, this also changes errno attached to libcurl,
> which, in turn, makes my program exit with an "FIle/ directory not found
> message".
> I almost forgot, answering your previous question:
> No, I am not under the impression that libgcrypt may not change errno when
> I call any arbitrary function, but that this specific circumstance differs
> from the documented behaviour by not considering /etc/gcrypt/fips_enabled
> *may* not exist.
> Also, my setup doesn't have neither files that gcrypt tries to open to
> check fips mode, which is the debian default configuration.
>
> Breakpoint 2, main (argc=1, argv=0x7fffffffe128) at tool.c:656
> 656 if ( curl_global_init(CURL_GLOBAL_ALL) != 0 ) { // init
> libcurl
> (gdb) watch *errno_p
> Hardware watchpoint 3: *errno_p
> (gdb) c
> Continuing.
>
> Hardware watchpoint 3: *errno_p
>
> Old value = 0
> New value = 2
> 0x00007ffff7e0171c in __access (file=0x7ffff76c335b
> "/etc/gcrypt/fips_enabled", type=0) at
> ../sysdeps/unix/sysv/linux/access.c:27
> 27 ../sysdeps/unix/sysv/linux/access.c: No such file or directory.
> (gdb) bt
> #0 0x00007ffff7e0171c in __access (file=0x7ffff76c335b
> "/etc/gcrypt/fips_enabled", type=0)
> at ../sysdeps/unix/sysv/linux/access.c:27
> #1 0x00007ffff760024e in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
> #2 0x00007ffff75f88ea in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
> #3 0x00007ffff75f9a5f in ?? () from /lib/x86_64-linux-gnu/libgcrypt.so.20
> #4 0x00007ffff75f5789 in gcry_control () from
> /lib/x86_64-linux-gnu/libgcrypt.so.20
> #5 0x00007ffff7c76794 in libssh2_init () from
> /lib/x86_64-linux-gnu/libssh2.so.1
> #6 0x00007ffff7f428fb in ?? () from
> /lib/x86_64-linux-gnu/libcurl-gnutls.so.4
> #7 0x00007ffff7f032d7 in ?? () from
> /lib/x86_64-linux-gnu/libcurl-gnutls.so.4
> #8 0x0000555555556110 in main (argc=1, argv=0x7fffffffe128) at tool.c:656
> (gdb)
>
Re: Pull Request (patch libgcrypt) [ In reply to ]
On Thu, 20 Aug 2020 19:57, Antonio Harres said:

> The key point here is that the relevant portion of code is using fopen to
> probe for the existence of the file, this may return all sorts of errors,
> but commonly it's ENOENT. which is then returned into any code that is
> initializing libgcrypt. But, I'm getting errno at something that is not an

Sorry, that is not correct. Here is the function's prototype:

void _gcry_initialize_fips_mode (int force)

as you can see that function _cannot_ return any error code. I already
explained that you can't assume that an arbitrary library like Libgcrypt
makes any guarantee not to change the ERRNO of the thread. In fact
there is no guarantee for that for many libc function's either.


Salam-Shalom,

Werner

--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.