Is zeroing of data->internal without reset of data->initialized safe?

[solardiz]

This issue is a code review in progress. I see no suitable issue label for that, so am leaving it with none.

While reviewing #201, I looked into other places where we may zeroize the data struct, and found this:

libxcrypt/lib/crypt.c

Line 178 in 88ecfa5

struct crypt_internal *cint = get_internal (data);

  struct crypt_internal *cint = get_internal (data);
  h->crypt (phrase, phr_size, setting, set_size,
            (unsigned char *)data->output, sizeof data->output,
            cint->alg_specific, sizeof cint->alg_specific);

  explicit_bzero (data->internal, sizeof data->internal);

This zeroizes internal, but leaves the initialized field as-is. For hash implementations that compute and cache constants in internal, like I think descrypt does, doesn't this result in incorrect computation on the next call?

How does this even pass tests? Maybe I'm misinformed and our descrypt doesn't cache anything anymore?

Let's at least figure this out.

[besser82]

AFAIK, the only purpose of the initialized field in struct crypt_data is for backwards compatibility with glibc. I cannot find any code, that actually checks whether that variable has a particular value assigned.

Anyways, I agree upon explicitly setting this to 0 after zeroizing internal, in order to comply with the documentation.

[solardiz]

AFAIK, the only purpose of the initialized field in struct crypt_data is for backwards compatibility with glibc. I cannot find any code, that actually checks whether that variable has a particular value assigned.

Oh, right. I also don't see any uses of it in libxcrypt, not even in much older versions where the struct actually matched glibc's. And now that the struct doesn't even match glibc's, the only compatibility that comes from having this field is that the same source code can be built against either library - but the resulting binaries wouldn't be compatible (portable between libxcrypt and glibc libcrypt) because the struct layout is different.