Re: explicit_bzero and -std=c99

[Simon Josefsson]

Paul Eggert <eggert@cs.ucla.edu> writes:

>> 2) It seems explicit_bzero.c in gnulib fall backs to using 'asm' for
>> GCC, which isn't working in non-GNU modes of gcc.  Further wondering:
>
> I hope I fixed this particular problem by installing the attached.

Thank you!

> Perhaps Gnulib's other uses of asm should also be changed?

Yes I think we should '__asm__' instead of 'asm' for the reason
explained by the gcc manual that Bruno linked to.

>> 3) Is the idiom of using separate functions bzero() vs explicit_bzero()
>>     to avoid security-problematic compiler optimization still a good one?
>
> Yes

If so, I would prefer a read_sensitive_file() API instead of read_file()
with a flag to enable the security-sensitive functionality.  I'll leave
it for the future, as this the immediate problem is resolved.

Bruno Haible <bruno@clisp.org> writes:

>> 1) Does gnulib support building with gcc -std=c99?  I think we should,
>> but it could have documented missing functionality or breakage.
>
> No, Gnulib does not support this. We freely use GCC extensions,
> such as ({...}) or asm, usually conditionalized with
>   defined __GNUC__ || defined __clang__
> Only in math.in.h and xalloc-oversized.h we also test __STRICT_ANSI__.
>
> We could test __STRICT_ANSI__ also in more places, but what really is the
> point? So that people then complain "the asyncsafe-spin and simple-atomic
> tests fail for me"?
>
> The point of '-std=c99' is to verify that the source code is pure ISO C
> without any extensions. Gnulib is not in that category.

Your answer is a bit different from Paul's, and both seems like
reasonable approaches to me.  This may be a situation where sometimes we
make a small effort of being compatible with -std=c99 and sometimes
decide against it.  I think what could help is a bit more documentation
about this problem.  Building gnulib with -std=c99 and fixing some of
the minor issues will likely help future compatibility of code, so I
think we should make small efforts to comply.  I agree that there is
likely some parts of gnulib that simply don't work in C99-mode --
documenting what they are would be useful.

In libtasn1, we want to support C89 environments since it is such a
low-level and bootstrap-relevant library.  At least for the library, the
command-line tool doesn't have to be C89-compatible IMHO.

>>    1) The reason for having explicit_bzero is read_file, which needs it
>>    for reading sensitive files, a feature we don't use.  Uncoupling this
>>    unnecessary dependency would have been nice.
>
> No, we have explicit_bzero because it's a glibc function that we think
> should be available to programs on all OSes.
> <https://www.gnu.org/software/gnulib/manual/html_node/explicit_005fbzero.html>

Sorry I was unclear: the reason for LIBTASN1 to have explicit_bzero is
read_file.  But libtasn1 never uses the sensitive flag, and thus never
really excercise the explicit_bzero code path.

>>    3) Is there a way to detect if the compiler supports 'asm'?  The
>>    current test 'defined __GNUC__ && !defined __clang__' is what is
>>    really failing here.
>
> Probably something like
>   (defined __GNUC__ || defined __clang__) && !defined __STRICT_ANSI__

Using __asm__ instead seems more elegant, and even aligned with gcc
manual.

>> 3) Is the idiom of using separate functions bzero() vs explicit_bzero()
>>    to avoid security-problematic compiler optimization still a good one?
>> 
>>    1) If yes, I think we should have read_sensitive_file() rather than
>>    extending read_file() with a flag for this purpose.
>> 
>>    2) If no, what is the better idiom to use here instead of
>>    explicit_bzero?
>
> When the code for average contexts and the code for secure contexts differ
> only by a few lines of code, we would like to avoid code duplication. As
> code duplication means twice the maintenance effort in the future.

Sure -- although it would be possible to implement the essence of
read_file in a way where support for the sensitive flag is a
compile-time option.

/Simon

signature.asc
Description: PGP signature