Re: An encryption patch to ensure there are no zero's in data CRC

[Jose E. Marchesi]

> Hi,
>
> This patch removes any zero bytes from the CRC that is calculated for
> a data checksum.
>
> I remove them when the CRC is calculated for encryption, and then
> remove them too from the calculated CRC for decryption. I don't know
> whether it's impossible for a CRC to contain a zero, but since we're
> using strlen on the encrypted data, if there were a zero in the CRC,
> the decryption would fail.

Woulnd't it be better to not use strlen on the encrypted data?  We
ultimately print out an ASCII encoded version of that data, and strlen
would make sense there, but definitely not in the raw bytes.

> Craig
>
> --- ../recutils/src/rec-crypt.c       2023-09-03 17:56:51.475134762 +0200
> +++ src/rec-crypt.c   2023-09-03 18:36:37.110005393 +0200
> @@ -41,6 +41,14 @@
>                         strlen (REC_ENCRYPTED_PREFIX)) == 0));
>  }
>  
> +void remove_zero_bytes_from_uint32(uint32_t *up);
> +void remove_zero_bytes_from_uint32(uint32_t *up) {
> +  char *p = (char*)up;
> +  for (int i=0; i<sizeof(uint32_t); i++) {
> +    if (p[i]==0) p[i]=1;
> +  }
> +}
> +
>  bool
>  rec_encrypt (char   *in,
>               size_t  in_size,
> @@ -69,6 +77,10 @@
>  #if defined WORDS_BIGENDIAN
>    crc = rec_endian_swap (crc);
>  #endif
> +  /* We append the CRC to the string, but use strlen in
> +     decryption, so we ensure there aren't any 0's in 
> +     the crc */
> +  remove_zero_bytes_from_uint32(&crc);
>  
>    real_in_size = in_size + 4;
>    real_in = malloc (real_in_size + 4);
> @@ -231,7 +243,11 @@
>  #if defined WORDS_BIGENDIAN
>        crc = rec_endian_swap (crc);
>  #endif
> -      if (crc32 (*out, outlen - 4) != crc)
> +      // We don't have to remove any zero's from the retrieved CRC value,
> +      // since it was located by strnlen, so by definition it won't contain 0
> +      uint32_t calculatedCrc = crc32 (*out, outlen - 4);
> +      remove_zero_bytes_from_uint32(&calculatedCrc);
> +      if (calculatedCrc != crc)
>          {
>            gcry_cipher_close (handler);
>            return false;