Implement state eraser for NIST DRBG and other crypto primitives using macro

Question

As a spare-time personal project, I'm implementing a set of symmetric-key cryptographic primitives.

One thing I've left vacant for a long time, is the key/state eraser, which I decide to add right now.

I intend it to be a macro, so that it doesn't require any additional object file; the macro should be statement-like (as per this question), so that its usage is consistent with those from other lines.

Here's my current code:

#define ERASE_STATES(buf, len)                  \
    do {                                        \
        for(uintptr_t i=0; i<len; i++)          \
            ((char *)buf)[i] = 0;               \
    } while(0)

There's a few things I'm not entirely confident about.

1. len is a cardinal and it'd probably be a size_t, but i is an ordinal, should I also use size_t, or change it to some other type for semantic consistency? (I'm using uintptr_t right now.)

2. The signedness of the type char is explicitly undefined according to standard, but could it nontheless be used without sacrificing semantic consistency, as a generic type representing a byte when assigning non-negative values to it?

3. Anything else I've missed?

Update

I'm not using memset because I designed my code to be compilable in free-standing environment.

Answer 1

There is not much code so I won't write much review

The issue that makes me most uneasy is the interface decision, specifically requiring calling code to calculate len in bytes.

If I am in "trying my best mode" as a coder, late at night with a stressful customer deadline hanging over me, it'll be all I can manage to remember to clear memory. I will inevitably try to clear an int array of 8 elements with ERASE_STATE(arr, 8). If I am lucky I will remember to use 32 and add a comment about the 4x8 calculation. I am unlikely to remember to account for the fact that other architectures could have a 16 or 64 bit int.

I would very much prefer a macro that took care of the type sizes for me and left the interface as passing the the natural array size.

Answer 2

Q1

uintptr_t is used for representing pointers numerically, so it's not suitable even if the underlying representation is identical to that of size_t, the same can be said for ptrdiff_t.

On the other hand, it's common knowledge that the set of finite ordinals is isomorphic to the set of finite cardinals, so size_t can be your friend here. What's more, the loop condition may be improved by adding a cast to size_t to len to ensure the loop always ends even when the original type of len has greater width than size_t although one should always ensure len has a sane value. Also, the cast would be necessarily present had you actually implement ERASE_STATES as a function.

So use size_t.

Q2

If you're really concerned that assigning the literal 0 to a char would get you a "negative zero" in one's complement or signed magnitude representation, then use unsigned char (which is just the same intermediate type used when the standard defined memset). Otherwise, the a world proliferous of two's complement signed integers, a plain char offers better clarity when the value is non-negative.

Also, there's guarantee that char[] is gap-free (unlike uint8_t).

Q3

Since it's a macro and not a function, buf and len will be evaluated multiple times during the loop. This limits its usage to lvalues and constant expressions.

Unless you assign arguments to temporary variables.

Improved Code

#define ERASE_STATES(buf, len)          \
    do {                                \
        char *ba = (void *)buf;         \
        size_t l = (size_t)len;         \
        for(size_t i=0; i<l; i++)       \
            ba[i] = 0;                  \
    } while(0)

Answer 3

There's not much to review here, since it's only a single macro, but there's still room for improvement.

Don't use a macro

I intend it to be a macro, so that it doesn't require any additional object file

If that's the motivation for using a macro, it's based on a misconception. You can get that and avoid the problems of macros, by defining a static function instead. Consider this code:

#include <stdint.h>

#define ERASE_STATES(buf, len)                  \
    do {                                        \
        for(uintptr_t i=0; i<len; i++)          \
            ((char *)buf)[i] = 0;               \
    } while(0)

unsigned buffer1[11];
unsigned buffer2[13];

static void memclr(void *buf, unsigned len) {
    for (char *b = buf ;len; --len, ++b) {
        *b = 0;
    }
}

void foo1() {
    ERASE_STATES(buffer1, sizeof(buffer1)*sizeof(*buffer1));
}

void foo2() {
    ERASE_STATES(buffer2, sizeof(buffer2)*sizeof(*buffer2));
}

void bar1() {
    memclr(buffer1, sizeof(buffer1)*sizeof(*buffer1));
}

void bar2() {
    memclr(buffer2, sizeof(buffer2)*sizeof(*buffer2));
}

int main() {
    foo1();
    foo2();
    bar1();
    bar2();
}

Does a modern compiler generate different code for foo1 versus bar1? No. It generates the exact same code without requiring any additional libraries or translation units, and without any runtime overhead for function calls. It also does not sacrifice type safety and is easier to debug and maintain.