[PATCH 1/2] fips: Use ELF header to find .rodata1 section

[Dmitry Belyavskiy]

Dear Florian,

On Fri, Feb 11, 2022 at 5:09 PM Florian Weimer <fweimer at redhat.com> wrote:

> * Clemens Lang via Gcrypt-devel:
>
> > diff --git a/src/fips.c b/src/fips.c
> > index 193af36b..fabc5158 100644
> > --- a/src/fips.c
> > +++ b/src/fips.c
> > @@ -592,7 +592,10 @@ run_random_selftests (void)
> >  # endif
> >  #define HMAC_LEN 32
> >
> > -static const unsigned char __attribute__ ((section (".rodata1")))
> > +/* Compilers can and will constant-propagate this as 0 when reading if
> it is
> > + * not declared volatile. Since this value will be changed using
> objcopy(1)
> > + * after compilation, this can cause the HMAC verification to fail. */
> > +static const volatile unsigned char __attribute__ ((section
> (".rodata1")))
> >  hmac_for_the_implementation[HMAC_LEN];
>
> volatile causes GCC to emit a writable section, and the link editor will
> make .rodata1 (and typically .text) writable as a result.  This is a
> fairly significant loss of security hardening.
>
> This bug is relevant here:
>
>   various services trigger { execmem } denials in FIPS mode
>   <https://bugzilla.redhat.com/show_bug.cgi?id=2034320>
>
> > +/**
> > + * Obtain the ElfN_Shdr.sh_offset value for the section with the given
> name in
> > + * the ELF file opened as fp and return it in offset. Rewinds fp to the
> > + * beginning on success.
> > + */
> >  static gpg_error_t
> > -hmac256_check (const char *filename, const char *key, struct link_map
> *lm)
> > +get_section_offset (FILE *fp, const char *section, unsigned long
> *offset)
> > +{
> > +  unsigned char e_ident[EI_NIDENT];
> > +#if __WORDSIZE == 64
> > +  Elf64_Ehdr ehdr;
> > +  Elf64_Shdr shdr;
> > +#define ELFCLASS_NATIVE ELFCLASS64
>
> __WORDSIZE is an internal glibc macro, not to be used outside of glibc.
> glibc's <link.h> defines ElfW as an official macro, and you could use
> ElfW(Ehdr) and ElfW(Shdr) here.
>
> The code looks at section headers.  These can be stripped.  Furthermore,
> the .rodata1 section is not really reserved for application use.
>
> I haven't reviewed Dmitry's OpenSSL changes (which I probably should
> do), but I'd suggest to use the same approach. 8-)
>

Yes, I used the same approach. But the situation is a bit more strange.

I had to add a `volatile` modifier to the HMAC variable because the
.section attribute was ignored otherwise.
After the issue you refer to was raised, I removed this modifier - and the
section was preserved.


-- 
Dmitry Belyavskiy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gcrypt-devel/attachments/20220211/5cea8360/attachment.html>