esp-hal icon indicating copy to clipboard operation
esp-hal copied to clipboard

Support 192 and 256-bit keys for AES

Open playfulFence opened this issue 11 months ago • 3 comments

closes #1234

playfulFence avatar Mar 20 '24 14:03 playfulFence

Not a huge fan of just panicking on invalid key length, as there's no way for an application to recover from that. I guess without const_generic_exprs there's not much else we can do other than make the method fallible (which might be preferable?).

@bjoernQ what do you think?

jessebraham avatar Mar 21 '24 13:03 jessebraham

Not a huge fan of just panicking on invalid key length, as there's no way for an application to recover from that. I guess without const_generic_exprs there's not much else we can do other than make the method fallible (which might be preferable?).

@bjoernQ what do you think?

Not exactly nice, true. Fallible would be better or maybe we define an Enum for the keysizes, like

pub enum Key {
  Key16([u8;16),
  #[cfg(any(feature = "esp32", feature = "esp32s2")]
  Key24([u8;24),
  Key32([u8;32),
}

Also don't we define cfg-symbols for the chip? Like #[cfg(any(esp32,esp32s2)]

bjoernQ avatar Mar 21 '24 14:03 bjoernQ

Thanks for the input @bjoernQ. Not quite sure what is best right now, I think making it fallible or the key size enum are both probably fine solutions. So, no strong opinions now, will sleep on it.

jessebraham avatar Mar 21 '24 22:03 jessebraham

Can we decide on a path forward here?

MabezDev avatar Apr 04 '24 10:04 MabezDev

I've suggested a solution, so has Bjoern. I'm not going to be too picky about what we decide on as long as it doesn't just panic. If nobody else has any input then we should probably just make it fallible and we can revisit it at some other time.

jessebraham avatar Apr 04 '24 13:04 jessebraham

I'm fine with making it fallible for now. I think the Key enum would be a better solution if we don't end up in having a hell lot of key lengths. But for the sake to get this forward - let's make it fallible

bjoernQ avatar Apr 04 '24 13:04 bjoernQ

I think, as Key enum is currently a "nicest" solution, we can proceed with it 🤔

playfulFence avatar Apr 04 '24 13:04 playfulFence

I think, as Key enum is currently a "nicest" solution, we can proceed with it 🤔

Sure, that's fine too. A fallible API is probably simpler, though.

jessebraham avatar Apr 04 '24 13:04 jessebraham

Let's go with the Key enum, I think we can make the API simpler like @jessebraham was suggesting by adding From<[u8; 16]> impls for Key, meaning the API doesn't get much harder to use, users only need to call into(). We could even add the trait bound for Into<Key> if we wanted to go the extra mile.

MabezDev avatar Apr 05 '24 13:04 MabezDev