mbedtls: add GCM support by AdrianMoranMontes · Pull Request #1764 · libssh2/libssh2

AdrianMoranMontes · 2025-12-22T09:12:51Z

Add AES-GCM support for mbedTLS backend

Summary

This PR implements AES-128-GCM and AES-256-GCM cipher support for the mbedTLS backend, enabling connections to SSH servers that require GCM encryption algorithms.

Problem

The mbedTLS backend had GCM support disabled (LIBSSH2_AES_GCM = 0) even though mbedTLS provides full GCM implementation through its mbedtls_gcm_* API.

Changes

mbedtls.h

Enabled GCM support: #define LIBSSH2_AES_GCM 1
Added #include <mbedtls/gcm.h>
Defined algorithm mappings for _libssh2_cipher_aes128gcm and _libssh2_cipher_aes256gcm

mbedtls.c

Created unified cipher context structure _libssh2_mbedtls_cipher_ctx with union to handle both regular ciphers and GCM
Modified _libssh2_mbedtls_cipher_init() to initialize GCM contexts with proper key setup
Implemented GCM operations in _libssh2_mbedtls_cipher_crypt():
- AAD (Additional Authenticated Data) processing for SSH packet length
- Encryption/decryption with proper IV management
- Authentication tag generation and verification (16 bytes)
- IV increment after each packet (last 8 bytes of 12-byte IV)
Updated _libssh2_mbedtls_cipher_dtor() to properly free GCM contexts

Technical Details

GCM uses mbedTLS native API (mbedtls_gcm_starts/update_ad/update/finish) rather than generic cipher API
IV structure: 4 bytes fixed + 8 bytes invocation counter (incremented per packet)
SSH GCM packet format: 4-byte AAD + encrypted payload + 16-byte auth tag

Testing

Tested successfully with SFTP servers requiring GCM-only encryption.

Pull request overview

This PR aims to add AES-128-GCM and AES-256-GCM cipher support to the mbedTLS crypto backend so libssh2 can negotiate and use aes{128,256}-gcm@openssh.com with servers requiring GCM.

Changes:

Refactors the mbedTLS cipher handling to use a unified cipher context structure intended to support both classic ciphers and AES-GCM.
Adds an AES-GCM encrypt/decrypt path using mbedtls_gcm_* APIs (currently guarded by #if LIBSSH2_AES_GCM).
Updates the mbedTLS cipher init/crypt/dtor plumbing to use the new unified context.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
`src/mbedtls.h`	Adjusts the cipher-init macro signature (but does not currently enable/define the advertised GCM support).
`src/mbedtls.c`	Introduces a unified cipher context and implements a GCM code path alongside existing cipher handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-05-01T07:18:14Z

+    _libssh2_mbedtls_cipher_ctx *cctx = *(_libssh2_mbedtls_cipher_ctx **)ctx;
+
+    if(!cctx)
+        return;
+


_libssh2_mbedtls_cipher_dtor() also assumes _libssh2_cipher_ctx is a pointer-like container (*(_libssh2_mbedtls_cipher_ctx **)ctx). This has the same undefined-behavior/ABI problem as init/crypt. Once _libssh2_cipher_ctx is corrected for mbedTLS, make sure the dtor reads the context in a type-correct way and clears the stored pointer (e.g., set it to NULL) to avoid accidental double-free on repeated teardown.

Copilot · 2026-05-01T07:18:14Z

+#define _libssh2_cipher_init(h, type, iv, secret, encrypt) \
+    _libssh2_mbedtls_cipher_init(h, type, iv, secret, encrypt)


The PR description says AES-GCM is enabled for the mbedTLS backend, but in the current codebase LIBSSH2_AES_GCM is still defined as 0 in src/mbedtls.h and there are no _libssh2_cipher_aes{128,256}gcm mappings for mbedTLS. As a result, all the new GCM code in mbedtls.c is compiled out and GCM won’t actually be negotiable/usable. Please flip LIBSSH2_AES_GCM to 1, add the required mbedtls/gcm.h include, and add the cipher type mappings (consistent with how openssl.h exposes _libssh2_cipher_aes128gcm/_libssh2_cipher_aes256gcm).

Copilot · 2026-05-01T07:18:15Z

+
+            /* Store the context pointer */
+        *(void **)h = cctx;
+        return 0;
    }


_libssh2_mbedtls_cipher_init() stores cctx via *(void **)h = cctx;, but in the mbedTLS backend _libssh2_cipher_ctx is defined as mbedtls_cipher_context_t (a struct), not a pointer type. This is undefined behavior (strict aliasing/alignment) and makes the _libssh2_cipher_ctx ABI inconsistent. Fix by redefining _libssh2_cipher_ctx for mbedTLS to be a pointer type (and updating init/crypt/dtor accordingly) or by embedding the unified context directly in _libssh2_cipher_ctx.

Copilot · 2026-05-01T07:18:15Z

+    cctx = *(_libssh2_mbedtls_cipher_ctx **)ctx;
+    if(!cctx)
+        return -1;


_libssh2_mbedtls_cipher_crypt() assumes ctx points to a pointer (cctx = *(_libssh2_mbedtls_cipher_ctx **)ctx;). This depends on _libssh2_cipher_ctx being pointer-compatible storage; with the current #define _libssh2_cipher_ctx mbedtls_cipher_context_t, this is undefined behavior and can break under optimization. Fix this together with the _libssh2_cipher_ctx type issue in _libssh2_mbedtls_cipher_init().

Copilot · 2026-05-01T07:18:15Z

+                memcpy(block + blocksize - authlen, tag, authlen);
+            }
+            else {
+                if(memcmp(tag, block + blocksize - authlen, authlen) != 0)


In the GCM decrypt path, the authentication tag is compared with memcmp(), which is not timing-safe. This codebase already has _libssh2_timingsafe_bcmp() for MAC/tag verification; use it here as well to avoid timing side-channels on tag checks.

Suggested change

if(memcmp(tag, block + blocksize - authlen, authlen) != 0)

if(_libssh2_timingsafe_bcmp(tag, block + blocksize - authlen,

authlen) != 0)

Copilot · 2026-05-01T07:18:15Z

+        if(cryptlen > 0) {
+            buf = (unsigned char *)mbedtls_calloc(cryptlen, 1);
+            if(!buf)
+                return -1;
+        }


The temporary buf in the GCM path can hold plaintext during decrypt, but it’s freed with mbedtls_free() (and on some error paths too) without being wiped. Use _libssh2_mbedtls_safe_free(buf, cryptlen) (or equivalent explicit zeroization) for all frees of this buffer to avoid leaving plaintext in heap memory.

AdrianMoranMontes · 2026-05-04T10:22:02Z

@vszakats I think all your comments are now fullfiled.

vszakats changed the title ~~Add GCM support on mbedTLS.~~ mbedtls: add GCM support Dec 25, 2025

vszakats reviewed Jan 2, 2026

View reviewed changes

Comment thread src/mbedtls.c Outdated

AdrianMoranMontes force-pushed the fix-mbedtls-gcm branch 2 times, most recently from 9ce7aaa to 3061d4b Compare April 13, 2026 08:42

vszakats requested a review from Copilot May 1, 2026 07:10

Copilot started reviewing on behalf of vszakats May 1, 2026 07:11 View session

vszakats reviewed May 1, 2026

View reviewed changes

Copilot AI reviewed May 1, 2026

View reviewed changes

AdrianMoranMontes and others added 7 commits May 4, 2026 10:32

Add GCM support on mbedTLS.

bfd3604

Fix style.

8b762ac

drop redundant parenthesis

a73236d

Rebase fix-mbedtls-gcm branch with upstream master branch.

5365298

Fix parameters naming.

7ca9ff5

Try to avoid different parameter names

dd764a4

mbedtls: use _libssh2_timingsafe_bcmp for GCM tag verification

14bf697

AdrianMoranMontes force-pushed the fix-mbedtls-gcm branch from cfd1774 to 14bf697 Compare May 4, 2026 08:36

AdrianMoranMontes added 6 commits May 4, 2026 10:44

mbedtls: replace typedef struct with named struct

e79e57f

mbedtls: fix LONGLINE in cipher_dtor

7aeabcb

use safe_free for GCM temporary buffer

f5f5a04

mbedtls: redefine _libssh2_cipher_ctx as pointer, remove UB casts

bdb48a9

mbedtls: enable AES-GCM support

b8aea89

mbedtls: nullify context pointer in cipher_dtor to prevent double-free

99479d2

+                      } gcm;
+              #endif
+                  } ctx;
+              } _libssh2_mbedtls_cipher_ctx;

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mbedtls: add GCM support#1764

mbedtls: add GCM support#1764
AdrianMoranMontes wants to merge 13 commits into
libssh2:masterfrom
AdrianMoranMontes:fix-mbedtls-gcm

AdrianMoranMontes commented Dec 22, 2025

Uh oh!

Uh oh!

willco007 commented Apr 6, 2026

Uh oh!

AdrianMoranMontes commented Apr 13, 2026

Uh oh!

vszakats May 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

AdrianMoranMontes commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		#define _libssh2_cipher_init(h, type, iv, secret, encrypt) \
		_libssh2_mbedtls_cipher_init(h, type, iv, secret, encrypt)

	if(memcmp(tag, block + blocksize - authlen, authlen) != 0)
	if(_libssh2_timingsafe_bcmp(tag, block + blocksize - authlen,
	authlen) != 0)

Conversation

AdrianMoranMontes commented Dec 22, 2025

Add AES-GCM support for mbedTLS backend

Summary

Problem

Changes

mbedtls.h

mbedtls.c

Technical Details

Testing

Related

Uh oh!

Uh oh!

willco007 commented Apr 6, 2026

Uh oh!

AdrianMoranMontes commented Apr 13, 2026

Uh oh!

vszakats May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

AdrianMoranMontes commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants