Mailing List Archive

[PATCH v2 1/2] fips: Use ELF header to find hmac file offset
* src/fips.c [ENABLE_HMAC_BINARY_CHECK] (hmac256_check): Use ELF headers
to locate the file offset for the HMAC in addition to information from
the loader

--

The previous method of locating the offset of the .rodata1 section in
the ELF file on disk used information obtained from the loader. This
computed the address of the value in memory at runtime, but the offset
in the file can be different. Specifically, the old code computed
a value relative to ElfW(Phdr).p_vaddr, but the offset in the file is
relative to ElfW(Phdr).p_offset. These values can differ, so the
computed address at runtime must be translated into a file offset
relative to p_offset.

This is largely cosmetic, since the text section that should contain the
HMAC usually has both p_vaddr and p_offset set to 0.

Signed-off-by: Clemens Lang <cllang@redhat.com>
---
README | 3 ++-
src/fips.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/README b/README
index 3b465c1b..4d7697dd 100644
--- a/README
+++ b/README
@@ -157,7 +157,8 @@
--enable-hmac-binary-check
Include support to check the binary at runtime
against a HMAC checksum. This works only in FIPS
- mode and on systems providing the dladdr function.
+ mode on systems providing the dladdr function and using
+ the ELF binary format.

--with-fips-module-version=version
Specify a string used as a module version for FIPS
diff --git a/src/fips.c b/src/fips.c
index 391b94f1..c40274d9 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -25,6 +25,8 @@
#include <string.h>
#ifdef ENABLE_HMAC_BINARY_CHECK
# include <dlfcn.h>
+# include <elf.h>
+# include <limits.h>
# include <link.h>
#endif
#ifdef HAVE_SYSLOG
@@ -594,6 +596,57 @@ run_random_selftests (void)
static const unsigned char __attribute__ ((section (".rodata1")))
hmac_for_the_implementation[HMAC_LEN];

+/**
+ * Determine the offset of the given virtual address in the ELF file opened as
+ * fp and return it in offset. Rewinds fp to the beginning on success.
+ */
+static gpg_error_t
+get_file_offset (FILE *fp, unsigned long paddr, unsigned long *offset)
+{
+ ElfW (Ehdr) ehdr;
+ ElfW (Phdr) phdr;
+ uint16_t e_phidx;
+
+ // read the ELF header
+ if (0 != fseek (fp, 0, SEEK_SET))
+ return gpg_error_from_syserror ();
+ if (1 != fread (&ehdr, sizeof (ehdr), 1, fp))
+ return gpg_error_from_syserror ();
+
+ // the section header entry size should match the size of the shdr struct
+ if (ehdr.e_phentsize != sizeof (phdr))
+ return gpg_error (GPG_ERR_INV_OBJ);
+ if (ehdr.e_phoff == 0)
+ return gpg_error (GPG_ERR_INV_OBJ);
+
+ // jump to the first program header
+ if (0 != fseek (fp, ehdr.e_phoff, SEEK_SET))
+ return gpg_error_from_syserror ();
+
+ // iterate over the program headers, compare their virtual addresses with the
+ // address we are looking for, and if the program header matches, calculate
+ // the offset of the given paddr in the file using the program header's
+ // p_offset field.
+ for (e_phidx = 0; e_phidx < ehdr.e_phnum; e_phidx++)
+ {
+ if (1 != fread (&phdr, sizeof (phdr), 1, fp))
+ return gpg_error_from_syserror ();
+ if (phdr.p_type == PT_LOAD && phdr.p_vaddr <= paddr
+ && phdr.p_vaddr + phdr.p_memsz > paddr)
+ {
+ // found section, compute the offset of paddr in the file
+ *offset = phdr.p_offset + (paddr - phdr.p_vaddr);
+
+ if (0 != fseek (fp, 0, SEEK_SET))
+ return gpg_error_from_syserror ();
+ return 0;
+ }
+ }
+
+ // section not found in the file
+ return gpg_error (GPG_ERR_INV_OBJ);
+}
+
static gpg_error_t
hmac256_check (const char *filename, const char *key, struct link_map *lm)
{
@@ -603,6 +656,7 @@ hmac256_check (const char *filename, const char *key, struct link_map *lm)
size_t buffer_size, nread;
char *buffer;
unsigned long paddr;
+ unsigned long offset = 0;
unsigned long off = 0;

paddr = (unsigned long)hmac_for_the_implementation - lm->l_addr;
@@ -611,6 +665,13 @@ hmac256_check (const char *filename, const char *key, struct link_map *lm)
if (!fp)
return gpg_error (GPG_ERR_INV_OBJ);

+ err = get_file_offset (fp, paddr, &offset);
+ if (err)
+ {
+ fclose (fp);
+ return err;
+ }
+
err = _gcry_md_open (&hd, GCRY_MD_SHA256, GCRY_MD_FLAG_HMAC);
if (err)
{
@@ -651,14 +712,14 @@ hmac256_check (const char *filename, const char *key, struct link_map *lm)
nread = fread (buffer+HMAC_LEN, 1, buffer_size, fp);
if (nread < buffer_size)
{
- if (off - HMAC_LEN <= paddr && paddr <= off + nread)
- memset (buffer + HMAC_LEN + paddr - off, 0, HMAC_LEN);
+ if (off - HMAC_LEN <= offset && offset <= off + nread)
+ memset (buffer + HMAC_LEN + offset - off, 0, HMAC_LEN);
_gcry_md_write (hd, buffer, nread+HMAC_LEN);
break;
}

- if (off - HMAC_LEN <= paddr && paddr <= off + nread)
- memset (buffer + HMAC_LEN + paddr - off, 0, HMAC_LEN);
+ if (off - HMAC_LEN <= offset && offset <= off + nread)
+ memset (buffer + HMAC_LEN + offset - off, 0, HMAC_LEN);
_gcry_md_write (hd, buffer, nread);
memcpy (buffer, buffer+buffer_size, HMAC_LEN);
off += nread;
@@ -694,8 +755,8 @@ check_binary_integrity (void)
const char *key = KEY_FOR_BINARY_CHECK;
void *extra_info;

- if (!dladdr1 (hmac_for_the_implementation,
- &info, &extra_info, RTLD_DL_LINKMAP))
+ if (!dladdr1 (hmac_for_the_implementation, &info, &extra_info,
+ RTLD_DL_LINKMAP))
err = gpg_error_from_syserror ();
else
err = hmac256_check (info.dli_fname, key, extra_info);
--
2.35.1


_______________________________________________
Gcrypt-devel mailing list
Gcrypt-devel@lists.gnupg.org
https://lists.gnupg.org/mailman/listinfo/gcrypt-devel
Re: [PATCH v2 1/2] fips: Use ELF header to find hmac file offset [ In reply to ]
On Mon, 14 Feb 2022 18:49, Clemens Lang said:

> + // iterate over the program headers, compare their virtual addresses with the
> + // address we are looking for, and if the program header matches, calculate
> + // the offset of the given paddr in the file using the program header's

Please don't use C++ comments - we stick to standard C90 comments.

> + if (1 != fread (&phdr, sizeof (phdr), 1, fp))

That is hard to read. Given the state of current compilers this old
trick is not needed. We are reading code left to righta nd partially
reversing the direction is an uncommon pattern. So please don't do.


> + unsigned long offset = 0;
> unsigned long off = 0;

The names are too similar - better use off2 or tmpoff instead of offset.

Sorry for nitpicky - thanks for your work.


Salam-Shalom,

Werner



--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
Re: [PATCH v2 1/2] fips: Use ELF header to find hmac file offset [ In reply to ]
Hello,

I'm going to apply this change of Clemens' (v2 1/2), with modification
of coding style.

On top of the change, I'll apply change to the hashing content. It is
better to hash from the start to the last loadable segment, so that we
can allow change of the binary by strip.

It is tracked by:
https://dev.gnupg.org/T5835
--

_______________________________________________
Gcrypt-devel mailing list
Gcrypt-devel@lists.gnupg.org
https://lists.gnupg.org/mailman/listinfo/gcrypt-devel