Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

possible issue in mbedtls ECDH #8128

Closed
1 task done
anishsheikh opened this issue Apr 26, 2023 · 13 comments
Closed
1 task done

possible issue in mbedtls ECDH #8128

anishsheikh opened this issue Apr 26, 2023 · 13 comments
Labels
Status: Community help needed Issue need help from any member from the Community. Type: Question Only question

Comments

@anishsheikh
Copy link

Board

ESP32-C3-DevKitM-1

Device Description

official Espressif Board

Hardware Configuration

Nothing Connected , just bare devboard

Version

v2.0.8

IDE Name

Arduino IDE

Operating System

Windows 11 22h2

Flash frequency

40

PSRAM enabled

no

Upload speed

115200

Description

I first wrote mbedtls code in C and tested with corresponding dart code which reproduces the same shared secret.
In Arduino IDE the main compilation happens on provided gcc compiler so , the code actually works just fine with mbedtls_mpi_write_binary(),
mbedtls_mpi_read_binary(),
It just doesn't produce the intended shared secret .
in RHEL 8 , gcc 8.5.0, i used mbedtls_mpi_write_binary_le(),
mbedtls_mpi_read_binary_le(),
which produced actual intended shared key in both Dart and C program, However in specific case of arduino ide , if we use the

mbedtls_mpi_write_binary_le(),
mbedtls_mpi_read_binary_le(),

both calculating functions produce error which are
mbedtls_ecdh_calc_secret() and mbedtls_ecdh_compute_shared(),

Sketch

*/Arduino Side Code/*

#include <mbedtls/ecdh.h>
#include <mbedtls/entropy.h>
#include <mbedtls/ctr_drbg.h>
#include "mbedtls/error.h"


  unsigned char my_pubkey[32] = { 0 };
  unsigned char my_privkey[32] = { 0 };
  unsigned char server_pubkey[32] = {0};
  unsigned char server_privkey[32] = {0};
  unsigned char shared_secret[32] = { 0 };
  unsigned char shared_secret2[32] = { 0 };

void printhex(unsigned char* array, int size) {
  for (int i = 0; i < size ; i++) {
    char str[3];
    sprintf(str, "%02x", (int)array[i]);
    Serial.print(str);
  }
  Serial.println("\n");
}
void swapEndianArray(unsigned char* ptrIn[], unsigned char * ptrOut[], int size) {
    int i;
    for (i = 0; i < size; i++) {
        ptrOut[i] = ptrIn[size - i - 1];
    }
    return;
}
void genkeyx25519(unsigned char* pubkey, unsigned char* privkey) {
    unsigned char my_pubkey[32] = {0};
    unsigned char my_privkey[32] = {0};
    mbedtls_ecdh_context ctx;
    mbedtls_entropy_context entropy;
    mbedtls_ctr_drbg_context ctr_drbg;

    // generate the keys and save to buffer
    mbedtls_ctr_drbg_init(&ctr_drbg);
    mbedtls_entropy_init(&entropy);

    mbedtls_ecdh_init(&ctx);
    mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, 0,  0);
        
    mbedtls_ecp_group_load(&ctx.grp, MBEDTLS_ECP_DP_CURVE25519);        
    mbedtls_ecdh_gen_public(&ctx.grp, &ctx.d, &ctx.Q, mbedtls_ctr_drbg_random, &ctr_drbg);

    mbedtls_mpi_write_binary_le(&ctx.Q.X, my_pubkey, sizeof(my_pubkey));
    mbedtls_mpi_write_binary_le(&ctx.d, my_privkey, sizeof(my_privkey));
    memcpy(pubkey, my_pubkey, sizeof(my_pubkey));
    memcpy(privkey, my_privkey, sizeof(my_privkey));
    mbedtls_ecdh_free(&ctx);
    mbedtls_ctr_drbg_free(&ctr_drbg);
    mbedtls_entropy_free(&entropy);
    
}
void calcsecretx25519(unsigned char * privkey, unsigned char * serverpubkey, unsigned char* sharedsecret) {
    unsigned char my_privkey[32] = {0};
    unsigned char server_pubkey[32] = {0};
    unsigned char shared_secret[32] = {0};
    unsigned char buffer[32] = {0};
    char error_buf[100];

    memcpy(my_privkey, privkey, sizeof(privkey));
    memcpy(server_pubkey, serverpubkey, sizeof(serverpubkey));
    
    int ret = 1;
    mbedtls_ecdh_context ctx;
    mbedtls_entropy_context entropy;
    mbedtls_ctr_drbg_context ctr_drbg;

    // generate the keys and save to buffer
    mbedtls_ctr_drbg_init(&ctr_drbg);
    mbedtls_entropy_init(&entropy);

    mbedtls_ecdh_init(&ctx);
    mbedtls_ctr_drbg_seed(
        &ctr_drbg,
        mbedtls_entropy_func,
        &entropy,
        0,
        0
    );

    ret = mbedtls_ecp_group_load(&ctx.grp, MBEDTLS_ECP_DP_CURVE25519);
      
    if (ret != 0) {
      printf("error in group load");
      ret = 0 ;
    }    
    
    // read my private key
    ret = mbedtls_mpi_read_binary_le(&ctx.d, my_privkey, sizeof(my_privkey));
    if (ret != 0) {
      printf("error in reading privkey");
      ret = 0 ;
    } 

    ret = mbedtls_mpi_lset(&ctx.Qp.Z, 1);
    if (ret != 0) {
      printf("error in mpi lset for secret");
      ret = 0 ;
    } 
    
    // read server key
    ret = mbedtls_mpi_read_binary_le(&ctx.Qp.X, server_pubkey, sizeof(server_pubkey));
    if (ret != 0) {
      printf("error in reading pubkey\n");
      ret = 0 ;
    } 

     //generate shared secret
      size_t olen;
      ret = mbedtls_ecdh_calc_secret(&ctx, &olen, shared_secret, 32,mbedtls_ctr_drbg_random, &ctr_drbg);
      if (ret != 0) {
      printf("error in calculating secret\n");
      mbedtls_strerror(ret, error_buf, sizeof(error_buf));
      printf("Error message for error code %d: %s\n", ret, error_buf);

      ret = 0 ;
    }  
    mbedtls_mpi_write_binary_le(&ctx.z, buffer, sizeof(buffer));
    printf("Secret buffer :");
    for (size_t i = 0; i < sizeof(buffer); i++)
        printf("%02X", buffer[i]);
    printf("\n");

    printf("Secret :");
    for (size_t i = 0; i < sizeof(shared_secret); i++)
        printf("%02X", shared_secret[i]);
    printf("\n");


    memcpy(sharedsecret, shared_secret, sizeof(shared_secret));
    mbedtls_ecdh_free(&ctx);
    mbedtls_ctr_drbg_free(&ctr_drbg);
    mbedtls_entropy_free(&entropy);

   
}




void setup() {
  Serial.begin(115200);
  // put your setup code here, to run once:
  genkeyx25519(my_pubkey, my_privkey);
  printf("PUBKEY: ");
  for (size_t i = 0; i < sizeof(my_pubkey); i++)
        printf("%02x", my_pubkey[i]);
    printf("\n");
  printf("Privkey: ");
  for (size_t i = 0; i < sizeof(my_privkey); i++)
        printf("%02x", my_privkey[i]);
    printf("\n");  
   genkeyx25519(server_pubkey, server_privkey);
  printf("Server PUBKEY: ");
  for (size_t i = 0; i < sizeof(server_pubkey); i++)
        printf("%02x", server_pubkey[i]);
    printf("\n");
  printf("Server Privkey: ");
  for (size_t i = 0; i < sizeof(server_privkey); i++)
        printf("%02x", server_privkey[i]);
    printf("\n");   
  calcsecretx25519(my_privkey, server_pubkey, shared_secret);
   printf("Secret :");
    for (size_t i = 0; i < sizeof(shared_secret); i++)
        printf("%02X", shared_secret[i]);
    printf("\n");
  calcsecretx25519(server_privkey, my_pubkey, shared_secret2);
   printf("Server Secret :");
    for (size_t i = 0; i < sizeof(shared_secret2); i++)
        printf("%02X", shared_secret2[i]);
    printf("\n");  


}

void loop() {
  // put your main code here, to run repeatedly:

}
*/ END/*




*/DART CODE/*
import 'package:cryptography/cryptography.dart';
import 'package:cryptography/dart.dart';
import 'dart:convert';
import 'dart:math';
import 'dart:io';
import 'dart:typed_data';
import "package:hex/hex.dart";

Future<void> main() async {
  // Generate a key pair for Alice
  final algorithm = X25519();
  final aliceKeyPair = await algorithm.newKeyPair();

  // Generate a key pair for Bob.
  //
  // In a real application, we will receive or know Bob's public key
  // somehow.
  print("Write the HEX PUBKEY :");
  final hexPublicKey = stdin.readLineSync()!;
  print("HEX PUBKEY : $hexPublicKey");
  final publicKeyBytes = HEX.decode(hexPublicKey);
  final bobKeyPair = await algorithm.newKeyPair();
  final bobPublicKey = await bobKeyPair.extractPublicKey();
  final publicKey = SimplePublicKey(publicKeyBytes, type: KeyPairType.x25519);
  final hexPubKey = await aliceKeyPair.extractPublicKey();
  final publickeybytes = hexPubKey.bytes;
  final pubkey = uint8ListToHex(publickeybytes);
  print("PUBLIC KEY: $pubkey");

  // We can now calculate a shared secret.
  final sharedSecret = await algorithm.sharedSecretKey(
    keyPair: aliceKeyPair,
    remotePublicKey: publicKey,
  );
  final sharedSecretBytes = await sharedSecret.extractBytes();
  final hex = uint8ListToHex(sharedSecretBytes);
  print("Shared Secret HEX : $hex");
  print('Shared secret: $sharedSecretBytes');
}

String uint8ListToHex(List<int> uint8List) {
  String hexString = '';

  for (int byte in uint8List) {
    // Convert the byte value to a hexadecimal string with 2 characters
    String hexByte = byte.toRadixString(16).padLeft(2, '0');
    hexString += hexByte;
  }
  return hexString;
}

*/Dart Code/*

Debug Message

ESP-ROM:esp32c3-api1-20210207
Build:Feb  7 2021
rst:0x1 (POWERON),boot:0xc (SPI_FAST_FLASH_BOOT)
SPIWP:0xee
mode:DIO, clock div:1
load:0x3fcd5810,len:0x438
load:0x403cc710,len:0x91c
load:0x403ce710,len:0x25b0
entry 0x403cc710
PUBKEY: d1cf1e2c0eaea894cd556df1a20ad3bbbb95818b57bce5a90f3786abbc67246c
Privkey: 70625c0deabc8b43fe8cb15e7fd7b7178a28e3e6ceb76f7a5719f32f48bd2944
Server PUBKEY: 7983b2d2c9201f1ac0cfb328d093b25017c706865f0738945ba6c8857eb6893c
Server Privkey: d03a05f065300175edda18d31cebcbe96ee8fcf53bb084284af9f41ff8536364
error in calculating secret
Error message for error code -19584: ECP - Invalid private or public key
Secret buffer :0000000000000000000000000000000000000000000000000000000000000000
Secret :0000000000000000000000000000000000000000000000000000000000000000
Secret :0000000000000000000000000000000000000000000000000000000000000000
error in calculating secret
Error message for error code -19584: ECP - Invalid private or public key
Secret buffer :0000000000000000000000000000000000000000000000000000000000000000
Secret :0000000000000000000000000000000000000000000000000000000000000000
Server Secret :0000000000000000000000000000000000000000000000000000000000000000

Other Steps to Reproduce

it works just fine in x86_64 so i guess it is not a mbedtls specific problem, most probably the problem corresponds to compiler specific endianness issue or linking problems so i opened an issue here, any specific help would be great, the code maybe messy but logic is understandable. can you please help me out?

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@anishsheikh anishsheikh added the Status: Awaiting triage Issue is waiting for triage label Apr 26, 2023
@mrengineer7777 mrengineer7777 added Type: Question Only question Status: Community help needed Issue need help from any member from the Community. and removed Status: Awaiting triage Issue is waiting for triage labels Apr 26, 2023
@anishsheikh
Copy link
Author

anishsheikh commented Apr 27, 2023

camphoto_1254324197
camphoto_1483920592
IMG_4337

as you can see a bit factored code is working both on dart on aarch64 , c on x86_64 and vice versa , so the problem is esp related.

I don't have access to full riscv or xtensta hardware, i will try to do it on arm m series.

@mrengineer7777
Copy link
Collaborator

Does your test code work properly under earlier versions of arduino-esp32?

@anishsheikh
Copy link
Author

Yeah, it did . Can you see any irregularities in the code btw ?
I will try to fix it today.

@mrengineer7777
Copy link
Collaborator

@anishsheikh I haven't used MBED so I can't comment on your code. I can think of two reasons the behavior might have changed between versions:

  1. Change in random Adds pseudo random numbers generation #7848
  2. The MBED version bundled in IDF might have changed.

It would be useful to know the arduino-esp32 versions your test code works on vs breaks.

@anishsheikh
Copy link
Author

anishsheikh commented Apr 27, 2023

I got it working, somehow it messes up using different contexts irregularly, the mpi bignum functions don't actually read the key properly,
this seems not to affect other architectures, if we use same context things work just fine, but that is not how its supposed to be,
can you consult the compiler related problems, this in not a fix, its just a trick to make it work, an appropriate fix will be on compiler, and how it handles the mpi functions. the issue can be closed here but this is not really a proper fix.

@mrengineer7777
Copy link
Collaborator

@anishsheikh That sounds important. Unfortunately I'm not following you. Can you spell it out for us? What lines of code need to change?

@anishsheikh
Copy link
Author

anishsheikh commented Apr 27, 2023

Can you try to reproduce the problem?

You have to just add some method to give the esp32 the generated public key and esp32 have to convert the hexadecimal into a byte array then input the byte array into calcsecret function , I lack time a bit right now to check what can be wrong with the compiler . I would certainly look into it later

@lbernstone
Copy link
Contributor

This really has nothing to do with arduino. I would recommend you post the issue on esp-idf. They will have much deeper knowledge of the workings of the mbedtls hardware.

@anishsheikh
Copy link
Author

anishsheikh commented Apr 28, 2023

well i used arduino ide, so I posted here. anyway i will try to post there. I did mention compiler which comes with something called GCC(GNU C COMPILER) which is built for specific architecture, which is not "ARDUINO" Specific.

@PavloMcr
Copy link

PavloMcr commented Aug 1, 2023

@anishsheikh, could you please share your fix of the issue, it would be much appreciated.

@PavloMcr
Copy link

PavloMcr commented Aug 1, 2023

@anishsheikh, I found the issue in your code, when you call sizeof on a pointer, it doesn't give you the size of the data the pointer points to, but rather the size of the pointer itself. This would be 4 bytes for a 32-bit system or 8 bytes for a 64-bit system. Thus, in memcpy you're only porting the first 4 bytes instead of 32. Replacing sizeof with 32 fixes the issue for me.

@anishsheikh
Copy link
Author

I’ve forgot to notify , i fixed the issue long ago.
giving the size of the array is a fix . But isn’t the compiler supposed to manage that .

@anishsheikh
Copy link
Author

In x86_64 and aarch64 the compiler itself managed it. For code portability it should be manually managed .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Community help needed Issue need help from any member from the Community. Type: Question Only question
Projects
None yet
Development

No branches or pull requests

4 participants