On 17/06/2019 19:29, Ard Biesheuvel wrote: > On Mon, 17 Jun 2019 at 19:05, Milan Broz <gmazyland@xxxxxxxxx> wrote: >> >> On 17/06/2019 16:39, Ard Biesheuvel wrote: >>>> >>>> In other words, if you add some additional limit, we are breaking backward compatibility. >>>> (Despite the configuration is "wrong" from the security point of view.) >>>> >>> >>> Yes, but breaking backward compatibility only happens if you break >>> something that is actually being *used*. So sure, >>> xts(aes)-essiv:sha256 makes no sense but people use it anyway. But is >>> that also true for, say, gcm(aes)-essiv:sha256 ? >> >> These should not be used. The only way when ESSIV can combine with AEAD mode >> is when you combine length-preserving mode with additional integrity tag, for example >> >> # cryptsetup luksFormat -c aes-cbc-essiv:sha256 --integrity hmac-sha256 /dev/sdb >> >> it will produce this dm-crypt cipher spec: >> capi:authenc(hmac(sha256),cbc(aes))-essiv:sha256 >> >> the authenc(hmac(sha256),cbc(aes)) is direct crypto API cipher composition, the essiv:sha256 >> IV is processed inside dm-crypt as IV. >> >> So if authenc() composition is problem, then yes, I am afraid these can be used in reality. >> >> But for things like gcm(aes)-essiv:sha256 (IOW real AEAD mode with ESSIV) - these are >> not supported by cryptsetup (we support only random IV in this case), so these should >> not be used anywhere. >> > > OK, understood. Unfortunately, that means that the essiv template > should be dynamically instantiated as either a aead or a skcipher > depending on the context, but perhaps this is not a big deal in > reality, I will check. > > One final question before I can proceed with my v2: in > crypt_ctr_blkdev_cipher(), do you think we could change the code to > look at the cipher string rather than the name of the actual cipher? > In practice, I don't think they can be different, but in order to be > able to instantiate > 'essiv(authenc(hmac(sha256),cbc(aes)),sha256,aes)', the cipher part > needs to be parsed before the TFM(s) are instantiated. You mean to replace crypto_tfm_alg_name() with the cipher string from the device-mapper table constructor? I hope I am not missing anything, but it should be ok. It just could fail later (in tfm init). The constructor is de-facto atomic step for device-mapper, I think it does not matter when it fails, the effect of failure is the same for userspace. Milan
