Support for AES GCM mode - #6389

On the subject of tests, I wrote some, but currently the tests are run on the Unix build and the Unix build doesn't pass the standard tests when built with mbedTLS, and this only runs with mbedTLS. The axTLS project seems to be dead, so we should probably switch the Unix build over to mbedTLS. At some point when I have time I will fix the standard tests to work with mbedTLS and at that time I'll expand that test coverage to cover this too, but at the moment any tests that I write for this won't be able to run under the existing test build setup.

Is there any consensus on merging this? The Travis failure is a Zephyr docker image issue, which is unrelated to the PR, and the 0.004% reduction in coverage seems to be a sampling error since the changes are to do with things that are untouched by this PR.

@nickovs Any updates on this?

@AmirHmZz This probably now needs a substantial rework, given all the changes to Micropython over the last five years. If I get the chance I will look into rebasing it against the current mainline branch, but I have a lot of other things going on right now so I'm not sure when I can find the time.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (b14d129) to head (fa7fe36).

@@           Coverage Diff           @@
##           master    #6389   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22298    22363   +65     
=======================================
+ Hits        21937    22002   +65     
  Misses        361      361           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code size report:

Reference:  tests/float/complex1.py: Fix CPython 3.14 deprecation. [b14d129]
Comparison: extmod/modcryptolib: Support for AES GCM mode. [merge of fa7fe36]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64: +1528 +0.178% standard[incl +224(data)]
      stm32:    +0 +0.000% PYBV10
      esp32:  +656 +0.038% ESP32_GENERIC[incl +160(data)]
     mimxrt:    +0 +0.000% TEENSY40
        rp2:  +616 +0.067% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

OK, I have rebased this to the latest master branch and I've also updated the tests to hopefully have full coverage of the new code. References to ucryptolib have all been updated to cryptolib in the modern naming style.

I have also changed the default value for MICROPY_PY_CRYPTOLIB_GCM to be the value of MICROPY_SSL_MBEDTLS, so if the later is enabled for a port and that port doesn't set a specific value then GCM mode will be enabled. Given the small incremental size and the fact that Galois Counter Mode is generally preferred to the other modes offered by this module this seemed like the right option.

@projectgus Thanks for picking this up.

Aside from being the generally "recommended mode" for AES these days, AES-GCM shows up in a lot of modern protocols. Probably the one most likely to get used on its own in an embedded situation is JWE (JSON Web Encryption) but it also gets used in various datagram protocols (my need was for PSSST, but it's also in DTLS), it's used in a bunch of low level 802.11 WiFi security protocols, and of course it shows up in TLS and QUIC.

Rolling our own in micropython-lib would be a bad idea at a lot of levels, since while it could sit on top of the existing raw AES code there is a lot of complex, security-sensitive code to deal with the Galois Counter Mode part. The C code to do this efficiently is already in any Micropython that uses mbedTLS, and this code is just a wrapper around that. The inclusion of that wrapper is already conditional on MICROPY_PY_CRYPTOLIB_GCM. At the moment I have this set to default to on for builds that include mbedTLS but if there are builds that are tight on space we could switch this off.

Regarding the submodules versioning, I find Git submodule handling mysterious and any update here was not intentional. If you have the ability to push an update that would be appreciated. I suspect that doing so will trigger a new CI event and hopefully that should un-block things.

Thanks @nickovs for the quick reply!

Probably the one most likely to get used on its own in an embedded situation is JWE (JSON Web Encryption) but it also gets used in various datagram protocols (my need was for PSSST, but it's also in DTLS), it's used in a bunch of low level 802.11 WiFi security protocols, and of course it shows up in TLS and QUIC.

Thanks for explaining. JWE and PSSST are good use cases, thanks! I'm aware it's also part of many common protocols like DTLS and TLS, etc. but we don't need a Python API to support these use cases.

Rolling our own in micropython-lib would be a bad idea at a lot of levels,

I agree!

The C code to do this efficiently is already in any Micropython that uses mbedTLS, and this code is just a wrapper around that

Yes. If the code size impact was a bit smaller then exposing the Python wrapper would be a really easy thing to approve. 500 bytes is not huge but it's not tiny either. I had a look at the code earlier to see if anything could be combined or refactored inside modcryptolib.c to reduce code size, but I didn't see anything really obvious.

If you have the ability to push an update that would be appreciated.

Will do, one sec! This should also give us the latest code size numbers.

Overall this looks like a good addition to me, especially since it's just wrapping existing mbedTLS code that's already in the firmware. And it's an extension of the existing ECB/CBC/CTR modes, implementing GCM.

Three comments:

1. Code size could probably be reduced by combining with the existing aes class as just a 4th mode. And then extending the existing encrypt/decrypt methods to take additional arguments for GCM. But, I think probably you're right here in implementing it as a new class because the semantics are different. I guess one could argue either way.
2. I think the new class should be called aes_gcm because that's easier on the eye to read.
3. What happens if the user has the data to encrypt/decrypt in separate pieces, eg it's very large and can't all fit in memory at once? Does this new class support passing in smaller chunks at a time, or do you need to encrypt/decrypt the entire thing at once? (Since you need a nonce passed in to encrypt()/decrypt(), the answer to this question is not clear to me. Maybe worth adding some tests and/or docs for this chunking case.)