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

[Bug]: Invalid CSR generated by ESP32s3 #6205

Closed
PaulMartinsen opened this issue Mar 18, 2023 · 31 comments · Fixed by #6624
Closed

[Bug]: Invalid CSR generated by ESP32s3 #6205

PaulMartinsen opened this issue Mar 18, 2023 · 31 comments · Fixed by #6624
Assignees
Labels

Comments

@PaulMartinsen
Copy link

PaulMartinsen commented Mar 18, 2023

Contact Details

No response

Version

Jim/ED25519_SHA2_fix

Description

Library creates an invalid certificate signing request on the ESP32s3 when hardware acceleration is enabled. A valid signing request is generated with hardware acceleration disabled, or the code running on a Windows target. Previously I've seen the same problem on the ESP32 and even with hardware acceleration disabled. Have tested on a recent master branch and also ED25519_SHA2_fix. I used @gojimmypi 's branch as he has a lot of fixes for the Espressif implementation of the library.

Output from the signing command when supplied with a good CSR (ESP32s3 without h/w acceleration):

$ openssl.ex
[user_settings.zip](https://github.com/wolfSSL/wolfssl/files/11007185/user_settings.zip)
e x509 -req -in "Input\BLS-001 - ESP32s3 - no HW acceleration - new key - signing request.csr" -days 10   -CA "CertificateAuthority-4096-PublicKey.crt" -CAkey "CertificateAuthority-4096-PrivateKey.pem" -passin pass:secret -CAcreateserial   -extfile "CertificateExtensions.txt" -extensions LumosDevice   -out Output\BLS-001-Device-Certificate.crt -text
Certificate request self-signature ok
subject=C = NZ, L = Hamilton, O = "Blue Leaf Software, Ltd", CN = Lumos, serialNumber = BLS-001

Output from the signing command when supplied with a bad CSR (ESP32s3 with h/w acceleration):

$ openssl.exe x509 -req -in "Input\BLS-001 - ESP32s3 - HW acceleration - New private key - Jim ED25519_SHA2_fix -signing request.csr" -days 10   -CA "CertificateAuthority-4096-PublicKey.crt" -CAkey "CertificateAuthority-4096-PrivateKey.pem" -passin pass:secret -CAcreateserial   -extfile "CertificateExtensions.txt" -extensions LumosDevice   -out Output\BLS-001-Device-Certificate.crt -text
Certificate request self-signature did not match the contents
545B0000:error:0200008A:rsa routines:RSA_padding_check_PKCS1_type_1:invalid padding:crypto\rsa\rsa_pk1.c:75:
545B0000:error:02000072:rsa routines:rsa_ossl_public_decrypt:padding check failed:crypto\rsa\rsa_ossl.c:599:
545B0000:error:1C880004:Provider routines:rsa_verify:RSA lib:providers\implementations\signature\rsa_sig.c:774:
545B0000:error:06880006:asn1 encoding routines:ASN1_item_verify_ctx:EVP lib:crypto\asn1\a_verify.c:217:

If I take the private key that's generated on the device I can create a CSR as below (in Windows), but that produces the same error message as above when I try to sign the CSR as above suggesting the problem might be with the private key.
openssl req -new -key "Input\BLS-001 - ESP32s3 - HW acceleration - New private key - Jim ED25519_SHA2_fix.pem" -nodes -out Output\BLS-001-req.csr

Code used to generate a private key and signing request on the device:

bool CCredentialStore::GenerateCertificate(const char* pchSerial, const char* pAddress)
{
  // Generate a new private device key. Using a RSA key because the ESP32 has hardware
  // acceleration for that. 
  // https://www.wolfssl.com/documentation/manuals/wolfssl/chapter07.html#rsa-key-generation
  // https://www.wolfssl.com/documentation/manuals/wolfssl/group__Random.html#function-wc_initrng
  // https://www.wolfssl.com/documentation/manuals/wolfssl/group__RSA.html#function-wc_initrsakey
  // https://www.wolfssl.com/documentation/manuals/wolfssl/group__RSA.html#function-wc_makersakey
  
  // todo: check if this is using the Espressif RNG. If it is, make sure the radio
  // is on. The radio is used as an entropy source to make sure that true random
  // numbers are generated. If it isn't using the Espressif RNG, maybe it should 
  // be since that should generate True Random Numbers. 
  WC_RNG RandomNumberGenerator; 
  int nResult = wc_InitRng(&RandomNumberGenerator);
  if (0 != nResult)
  {
    ReportError("InitRng", nResult);
    return false;
  }

  unique_ptr<RsaKey> pNewKey(new RsaKey());
  nResult = wc_InitRsaKey(pNewKey.get(), nullptr);
  if (0 != nResult)
  {
    ReportError("InitRsaKey", nResult);
    return false;
  }

  nResult = wc_MakeRsaKey(pNewKey.get(), PrivateKeySize, 65537, &RandomNumberGenerator);
  if (0 != nResult)
  {
    ReportError("MakeRsaKey", nResult);
    return false;
  }
  
  const size_t szPrivateKeyBuffer = 2048;
  auto puPrivateKeyDer = make_unique<uint8_t[]>(szPrivateKeyBuffer);
  nResult = wc_RsaKeyToDer(pNewKey.get(), puPrivateKeyDer.get(), szPrivateKeyBuffer);
  if (nResult > 0)
  {
    WriteCertificate(puPrivateKeyDer.get(), nResult, PRIVATEKEY_TYPE);
  }
  else
  {
    cerr << "Error: wc_RsaKeyToDer\n";
  }

  // Generate a certificate signing request. 
  // https://www.wolfssl.com/documentation/manuals/wolfssl/chapter07.html#certificate-signing-request-csr-generation
  // https://www.wolfssl.com/documentation/manuals/wolfssl/asn__public_8h.html#function-wc_makecertreq
  // https://www.wolfssl.com/documentation/manuals/wolfssl/asn__public_8h.html#function-wc_signcert
  unique_ptr<Cert> pRequest(new Cert());
  wc_InitCert(pRequest.get());
  pRequest->daysValid = 30;

#if IS_WINDOWS
  strncpy_s(pRequest->subject.country, "NZ", sizeof(pRequest->subject.country));
  strncpy_s(pRequest->subject.org, "Blue Leaf Software, Ltd", sizeof(pRequest->subject.org));
  strncpy_s(pRequest->subject.locality, "Hamilton", sizeof(pRequest->subject.locality));
  strncpy_s(pRequest->subject.commonName, "Lumos", sizeof(pRequest->subject.commonName));
#else
  strncpy(pRequest->subject.country, "NZ", sizeof(pRequest->subject.country));
  strncpy(pRequest->subject.org, "Blue Leaf Software, Ltd", sizeof(pRequest->subject.org));
  strncpy(pRequest->subject.locality, "Hamilton", sizeof(pRequest->subject.locality));
  strncpy(pRequest->subject.commonName, "Lumos", sizeof(pRequest->subject.commonName));
#endif
  CFixedStringPrint SerialDev(pRequest->subject.serialDev, sizeof(pRequest->subject.serialDev));
  SerialDev << pchSerial;

  const size_t szCertificateRequestBuffer = 4096;
  auto puCertificateRequest = make_unique<uint8_t[]>(szCertificateRequestBuffer);
  int nRequestSize = wc_MakeCertReq(pRequest.get(), puCertificateRequest.get(), szCertificateRequestBuffer, pNewKey.get(), nullptr);
  if (nRequestSize <= 0)
  {
    ReportError("MakeCertReq", nRequestSize);
    return false;
  }

  cout << "SignCert\n";
  pRequest->sigType = CTC_SHA256wRSA;
  int nCertificateSize = wc_SignCert(pRequest->bodySz, pRequest->sigType, puCertificateRequest.get(), szCertificateRequestBuffer, pNewKey.get(), nullptr, &RandomNumberGenerator);
  if (nCertificateSize <= 0)
  {
    ReportError("SignCert", nCertificateSize);
    return false;
  }

  WriteCertificate(puCertificateRequest.get(), nCertificateSize, CERTREQ_TYPE);
  
  cout << "\ndone.\n";

  return true;
}

void CCredentialStore::WriteCertificate(const uint8_t *pDERCertificate, size_t szCertificate, int nType)
{
  const size_t szTextRequestBuffer = 4096;
  auto puTextRequest = make_unique<uint8_t[]>(szTextRequestBuffer);
  memset(puTextRequest.get(), 0, szTextRequestBuffer);
  int nPemSize = wc_DerToPem(pDERCertificate, szCertificate, puTextRequest.get(), szTextRequestBuffer, nType);
  if (nPemSize <= 0 || nPemSize == szTextRequestBuffer)
  {
    ReportError("DerToPem", nPemSize);
    return;
  }

  // make sure null terminated. 
  puTextRequest.get()[nPemSize - 1] = 0;

  cout << reinterpret_cast<char*>(puTextRequest.get()) << endl << endl;
}

Also attached, WolfSSL user_settings.zip files, Keys and certificate signing requests.zip

Reproduction steps

No response

Relevant log output

No response

@dgarske
Copy link
Contributor

dgarske commented Mar 18, 2023

@anhu will you review this or assign to another engineer? Jim is traveling next week.

@anhu
Copy link
Member

anhu commented Mar 20, 2023

Hi @PaulMartinsen ,

Thank you very much for all the details. I have read your report very carefully and reproduced what you have seen on windows on my linux machine. Her are my steps (for the benefit of future readers):

#create root cert
$ openssl req -x509 -newkey rsa:2048 -keyout key.pem -out self_signed_req.pem -nodes
<enter dummy data>
# try to create the cert
$ openssl x509 -req -in BLS-001\ -\ ESP32s3\ -\ HW\ acceleration\ -\ New\ private\ key\ -\ Jim\ ED25519_SHA2_fix\ -signing\ request.csr -CA self_signed_req.pem  -CAkey key.pem -CAcreateserial -out certificate.pem -text 

Interestingly, when I use openssl to generate a CSR with the same private key, the same thing happens:

openssl req -new -key "BLS-001 - ESP32s3 - HW acceleration - New private key - Jim ED25519_SHA2_fix.pem" -nodes -out req.csr
<enter dummy data>
openssl x509 -req -in req.csr -CA self_signed_req.pem  -CAkey key.pem -CAcreateserial -out certificate.pem -text

This would make me agree with you that there is a problem with the private key. However, I find it strange that the problem would manifest itself in the usage of the public key that is based on the private key and not usage of the private key itself.

I will first do my best to better understand what kind of problem OpenSSL is encountering and report my findings here. However, please note that I do not have the ESP hardware. As such, once I report my findings, I will need to assign to another developer who has the relevant hardware.

Warm regards, Anthony

@anhu
Copy link
Member

anhu commented Mar 20, 2023

It would appear that OpenSSL is expecting this format of padding:

    /*
     * The format is
     * 00 || 01 || PS || 00 || D
     * PS - padding string, at least 8 bytes of FF
     * D  - data.
     */

I think this is called PKCS1 padding. Instead of getting the initial 0, it is finding a value of decimal 135. So, it would appear the padding is being done incorrectly.

I will need to dig into this a big deeper. @PaulMartinsen , I'm unfamiliar with the ESP hardware acceleration code and hardware. I will need to lookup whether it is the code or the hardware that is responsible for the padding. Please stay tuned while I do the research. If you can provide insights, I'd really appreciate it.

Warm regards, Anthony

@PaulMartinsen
Copy link
Author

Hi @anhu , thanks very much for your analysis. A great idea to use the private key to sign a CSR locally.

My understanding is the ESP hardware accelerates low level operations (e.g., Z = X * Y (mod M) ; Z = X^Y mod M), SHA and AES operations. I don't know enough about how the private key's are generated to know where the padding is being done though. I do have TLSv1.3 connections working on the ESP32s3 using the hardware acceleration and @gojimmypi has fixed some issues in SHA (that's the branch I mentioned above).

Is there a function in the library to load a PEM private key? I could upload a private key onto the device then do the CSR on that to confirm the private key is the issue if that were useful. I couldn't find a library function for that though. Another possibility would be to use the Wolf library to make a CSR using the bad private key. Would we expect the problem to be detected by wc_MakeCertReq or is that not possible?

Have a great day
Paul.

@anhu
Copy link
Member

anhu commented Mar 20, 2023

Hi @PaulMartinsen ,
The API is wc_RsaPrivateKeyDecode() . It takes as input a buffer containing a DER encoded key. If your key is PEM encoded, you will also need wc_KeyPemToDer(). There are examples of how to use these here: https://github.com/wolfSSL/wolfssl-examples/tree/master/certgen

@PaulMartinsen
Copy link
Author

Thanks @anhu .

I modified the code sample from above to load the private key I'd previously made with OpenSSL and loaded onto the ESP32s3 instead of calling wc_MakeRsaKey, copied below. Then the rest of the code created the certificate signing request, with the following output:

$ openssl.exe x509 -req -in "Input\Sign existing certificate - ESP32s3.csr" -days 10   -CA "CertificateAuthority-4096-PublicKey.crt" -CAkey "CertificateAuthority-4096-PrivateKey.pem" -passin pass:secret -CAcreateserial   -extfile "CertificateExtensions.txt" -extensions LumosDevice   -out Output\test-Device-Certificate.crt -text
Certificate request self-signature ok
subject=C = NZ, L = Hamilton, O = "Blue Leaf Software, Ltd", CN = Lumos, favouriteDrink = BLS-001

So that doesn't have the self-signature miss-match I see when using wc_MakeRsaKey to create a new private key, which again suggests the issue lies with the private key itself, somehow. One weird behaviour is that favouriteDrink instead of serialNumber appears in the subject distinguished name. Though the value (BLS-001) is what I loaded for subject.serialDev. That could just be a distraction though.

#if 1
  ifstream file("/content/Prvt+PubKy.crt");
  file.seekg(0, ios::beg);
  ostringstream Buffer;
  Buffer << file.rdbuf();
  string strContent = Buffer.str();
  
  unsigned char abyBuffer[1250];
  int nWritten = wc_KeyPemToDer((const unsigned char*)(strContent.c_str()), strContent.length(), abyBuffer, sizeof(abyBuffer), nullptr);
  if (nWritten < 0)
  {
    ReportError("wc_KeyPemToDer", nWritten);
    return false; 
  }
  
  word32 nStartOfKey = 0; 
  nResult = wc_RsaPrivateKeyDecode(abyBuffer, &nStartOfKey, pNewKey.get(), nWritten);
  if (0 != nResult)
  {
    ReportError("wc_RsaPrivateKeyDecode", nResult);
    return false;
  }
#else
  nResult = wc_MakeRsaKey(pNewKey.get(), PrivateKeySize, 65537, &RandomNumberGenerator);
  if (0 != nResult)
  {
    ReportError("MakeRsaKey", nResult);
    return false;
  }
#endif

@anhu
Copy link
Member

anhu commented Mar 21, 2023

Hi @PaulMartinsen ,

Wow. Thank you so much for your prompt reaction time and hard work!! It seems quite clear to me now that when the private key is generated, it has the public components in it. Then, when the CSR is generated, the public components are just being copied into the CSR. Once the public key is then actually used to verify the CSR, the padding problem is discovered.

As you said in your original post, this only happens when ESP32s3 acceleration is enabled. Please forgive my ignorance, but can you please let me know which flag/macro turns on ESP32s3 acceleration?

Warm regards, Anthony

@anhu
Copy link
Member

anhu commented Mar 21, 2023

@PaulMartinsen ,

Is this an urgent issue for you? Is temporarily disabling hardware acceleration a viable temporary solution?

Warm regards, Anthony

@anhu
Copy link
Member

anhu commented Mar 21, 2023

For the benefit of future readers:

I have looked in the code where CONFIG_IDF_TARGET_ESP32S3 is defined and as it relates to RSA, it seems to affect fairly low level mathematical operations. As such, it would seem to not relate to padding. As I have no hardware, I can't think of any further path I can take. I will confer with my colleagues.

Warm regards, Anthony

@anhu
Copy link
Member

anhu commented Mar 21, 2023

Hi @PaulMartinsen ,

The next sensible step would be to turn off hardware acceleration and see the differences in the csr that are generated. Can you try that? I do not have the correct hardware so I can't try that.

Note that @gojimmypi will be helping here to setup an environment to reproduce what you are seeing. However, due to other activities he will have limited time to work on this. I'm assigning this to him, but will continue to monitor and help out on this issue.

@anhu
Copy link
Member

anhu commented Mar 21, 2023

Oh, the unaccelerated version is in the zip file. Let me look into that.

@PaulMartinsen
Copy link
Author

Hi @anhu

This isn't a blocking problem for me at the moment as I can load externally generated keys onto the device. But I'm keenly interestedin, and happy to help with, its resolution :).

I add the following pre-processor symbols to user_settings.h to turn off hardware acceleration:

#if 1
// Undocumented. Disable hardware acceleration of cryptography functions including RSA
#define NO_ESP32WROOM32_CRYPT

// Undocumented. Disable hardware acceleration of RSA primitives?
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI 

// Undocumented. Disable hardware acceleration of SHA (e.g., SHA-256, SHA-512)
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH

// Undocumented. Disable hardware acceleration of AES
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_AES

#endif

BLS-001 - ESP32s3 - no HW acceleration - new key - signing request.csr in Keys.and.certificates.zip was generated with hardware acceleration off.

BTW, I just tried openssl rsa -in "BLS-001 - ESP32s3 - HW acceleration - New private key - Jim ED25519_SHA2_fix.pem" -text -pubout to see if any errors but it printed private key and public key things without reporting any errors.

I'm not quite sure what you mean by comparing the CSR with and without hardware acceleration. I'm generating a separate private key each time. Since you saw the same problem with OpenSSL, I'm not sure if reusing the hardware acceleration generated private key in a non-hardware accelerated CSR will tell us anything new? Did I missunderstand?

Thanks for your help with this. Much appreciated.

BTW, I'm in New Zealand. We are about 12 hours behind GMT, so sometimes there might be delays in my responses :)

@anhu
Copy link
Member

anhu commented Mar 21, 2023

I'm not so worried about the delays in responses, but thanks for letting me know. I just like to keep my notes here in case I need to move onto something else and another developer needs some context.

@anhu
Copy link
Member

anhu commented Mar 21, 2023

Hi @PaulMartinsen ,

I'm not sure if reusing the hardware acceleration generated private key in a non-hardware accelerated CSR will tell us anything new?

So I wanted to confirm that the format of the padding was conforming to this:

    /*
     * The format is
     * 00 || 01 || PS || 00 || D
     * PS - padding string, at least 8 bytes of FF
     * D  - data.
     */

And it is!! I saw a 0, about 200 FF bytes, and a 0 and then some other bytes after that.

I need to talk to my colleagues. Please stay tuned.

Warm regards, Anthony
...and indeed in openssl's `RSA_padding_check_PKCS1_type_1

@anhu
Copy link
Member

anhu commented Mar 21, 2023

Sorry, I've made a rookie mistake. I thought this was a problem with padding, but actually this indicates that there are problems somewhere else in the cryptographic procedure. Basically, the verify operation is failing. To get a better idea of the root cause we would need to override the RNG with a fake RNG that gives deterministic data (ie a buffer of incremented byte values) and see what the difference is between hardware acceleration and no hardware acceleration.

Warm regards, Anthony

@PaulMartinsen
Copy link
Author

Heh. @anhu , i don't think that's a mistake; could be anything! To me, the real mystery is why the bad private key doesn't raise an error earlier.

I'm using the hardware random number generator. With the following user_settings.h and implementation:

// Use the ESP32 RNG directly
// Bypass P-RNG and use ESP32 HW RNG directly 
#define WC_NO_HASHDRBG

// Define custom random number generator
// https://www.wolfssl.com/documentation/manuals/wolfssl/chapter02.html#custom_rand_generate_block
#ifdef __cplusplus
extern "C" int wolf_rng_gen(unsigned char* output, unsigned int sz);
#else
extern int wolf_rng_gen(unsigned char* output, unsigned int sz);
#endif
#define CUSTOM_RAND_GENERATE_BLOCK  wolf_rng_gen
// To use Espressif random number generator in place of the Wolf one
// See: https://www.wolfssl.com/documentation/manuals/wolfssl/chapter02.html#custom_rand_generate_block
// pOutput: buffer where random numbers are written
// szOutput: size of output (number of random bytes to write)
// returns: 
//   0 => success
//   BAD_FUNC_ARG => pOutput is null
//   RNG_FAILURE_E => random number generator isn't ready. 
int wolf_rng_gen(unsigned char* pOutput, unsigned int szOutput)
{
  if (NULL == pOutput)
    return BAD_FUNC_ARG;

  if (szOutput == 0)
    return 0;

  // Note, this is only a source of true random numbers when an
  // entropy source is available (e.g., WiFi radio). Since we should
  // only be handling certificates using the radio, that should be
  // okay. However, the radio might be turned on when generating 
  // a device key. Make sure it is. 
  esp_fill_random(pOutput, szOutput);
  
  return 0; 
}

I removed the user_settings.h to go back to whatever the standard random number generator is. But that didn't make any difference (I still get the self-signature miss-match).

I'm not sure what you had in mind by a deterministic data, but I put the custom settings back and modified the random number generator as shown below. I tried to create a new private key and CSR, but after more than 5 minutes it wasn't done (normally takes about 30 seconds). To me it looked like the code was trying to find a prime number so maybe my crude sequential RNG isn't up to the job?

unsigned char NextRandomValue = 1; 

void PrintRandomValue()
{
  printf("Next random value = %d\n", NextRandomValue);
}

// To use Espressif random number generator in place of the Wolf one
// See: https://www.wolfssl.com/documentation/manuals/wolfssl/chapter02.html#custom_rand_generate_block
// pOutput: buffer where random numbers are written
// szOutput: size of output (number of random bytes to write)
// returns: 
//   0 => success
//   BAD_FUNC_ARG => pOutput is null
//   RNG_FAILURE_E => random number generator isn't ready. 
int wolf_rng_gen(unsigned char* pOutput, unsigned int szOutput)
{
  if (NULL == pOutput)
    return BAD_FUNC_ARG;

  if (szOutput == 0)
    return 0;

  // Note, this is only a source of true random numbers when an
  // entropy source is available (e.g., WiFi radio). Since we should
  // only be handling certificates using the radio, that should be
  // okay. However, the radio might be turned on when generating 
  // a device key. Make sure it is. 
  //esp_fill_random(pOutput, szOutput);
  
  while (szOutput--)
  {
    *pOutput++ = NextRandomValue++;
  }
  
  return 0; 
}

@PaulMartinsen
Copy link
Author

FWIW, I generated a 512 bit key and CSR on the ESP32s3 (previous keys have been 2048 bits). But I still see the signature miss-match error.

@gojimmypi
Copy link
Contributor

For those following along, I looked at the docs and the csr example and created a VisualGDB example & README for a sample ESP32 app that creates a RSA Key cert:

    RsaKey genKey;
    RNG    rng;

    wc_InitRng(&rng);
    wc_InitRsaKey(&genKey, 0);

    ret = wc_MakeRsaKey(&genKey, 1024, 65537, &rng);
    if (ret != 0) {
        ESP_LOGI(TAG, "wc_MakeRsaKey error %d", ret);
    }

    byte der[4096];
    int  derSz = wc_RsaKeyToDer(&genKey, der, sizeof(der));
    if (derSz < 0) {
        ESP_LOGI(TAG, "wc_RsaKeyToDer error %d", ret);
    }

    byte pem[4096];
    int  pemSz = wc_DerToPem(der, derSz, pem, sizeof(pem),
                      PRIVATEKEY_TYPE);
    if (pemSz < 0) {
        ESP_LOGI(TAG, "wc_DerToPem error %d", ret);
        /* pemSz contains error */
        ;
    }

    ESP_LOGI(TAG, "%s", pem);

I manually copied the console output and saved to a file, for example ./input/gojimmypi_private_rsa_generated_key_hw.txt for example:

-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQDh1MnHdI/gGE7qG+PXGZ2U8pkW27T0QXEtBupI7//b+sOl2Q0B
bh/IUqI61Ys5MiDi9jr01r0nxOiMqnAfzx+DhEpu8PFfbElokg3nVLXcuKLBQRdz
kcyl/DbSeatvl0behZY6fMmUqpklWqZ74Zr2gQbiFJIyIu9QDLcdKPY1gQIDAQAB
AoGATdTjki13JLncAM2J8elvKRWPc5RXQlOHqQgYGPk2Sl+brH4pAFQu+gCYzwQo
DpdbRD2uxhF4cctohop7SEs/RZOveoDJyzK7+vIsHkvtSK2idQcQtnzyINDPM61s
8yaniDVJjySek19bBk3bMsOAyidmcdMAdYuwY7NzE5LtgAECQQD4iLEo6rxguW4P
OKXotWGk8Hfq/XL0nwgzGOku2NEHCtqLo5ZEYa6Yq52y/492OCppHhbqxSMSXK5x
Ee29uesBAkEA6J2BKhKVQdDnL1itwzuUmhfJLWpmMVcIh4y6qypiC2pZrKckIiBh
rcyY6BGL24pbS7XG5Ytzo/uf5fy3ktjKgQJBANvhK0N08XZtd0a2KmpIVnvzV1Sq
KJLtQ/bY+iw9HCkr3BAgiWg0N6DQbDOZz9WaeUMIoOPRx2aOYrKcVwY8vwECQD8X
EQRHPAI41asqwx4zl69fiinuNL9nGqIhEjQAb80m0CGax5sV60hHOKWgtK0bzn0v
VT3G39t3ELxBGc0TTYECQH4tQToR7UMrI1YVjE0mDDBiZOSLL4keOKfZ85JPqnF0
Q/zCMEmj8dls1tagi2a8ALuwvUPByVCALWMZ3JUYdck=
-----END RSA PRIVATE KEY-----

Adjusting the default user_settings.h both enabling and disabling this section for either no hardware or use hardware acceleration:

#define NO_ESP32WROOM32_CRYPT
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_AES
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI

In each case, with a hardware-generated cert or not, both files could be successfully signed. So it appears this is not a hardware acceleration issue, rather a possible incompatible settings.

Thanks @PaulMartinsen for the tips on using OpenSSL:

Create request:

SRC=gojimmypi_private_rsa_generated_key_hw.txt
CSR_OUT=$SRC-CertificateRequest.csr
DEVICE_ID=4243
openssl req -new -key ./input/$SRC \
   -nodes \
   -out ./output/$CSR_OUT \
   -subj "/C=NZ/ST=Waikato/L=Hamilton/O=Blue Leaf Software Ltd/CN=Lumos-%DeviceId%/serialNumber=$DEVICE_ID" \
   -addext "subjectAltName = URI:dev:ops:59430-Lumos-$DEVICE_ID" \
   -addext "extendedKeyUsage = serverAuth, clientAuth"

Sign the request:

openssl x509 -req -in "./output/$CSR_OUT" -days 10   \
    -CA "private/CertificateAuthority-4096-PublicKey.crt" \
    -CAkey "private/CertificateAuthority-4096-PrivateKey.pem" \
    -passin pass:secret -CAcreateserial  \
    -extfile "private/CertificateExtensions.txt" -extensions LumosDevice  \
     -out ./output/gojimmypi-Device-Certificate_hw.crt -text

Settings

A side note: I'm not sure about the impact of having a relative include path to a separate header file from the wolfSSL user_settings.h. Intuitively I think it would be best to keep all the settings in one user_settings.h file. I suppose this is a trivial pursuit question of the compiler and linker would always work this out for all wolfSSL files referencing the user_settings (itself also from a relative directory in a CMakeLists.txt Espressif component). In theory it should work, but I recommend keeping everything in a single user_serttings.h.

In the coming days I'll wrap up this example and save it over on the wolfssl-examples/ESP32. I'll also add some code to make a cert request from the ESP32.

I'll take a closer look at the original settings to see if there was some sort of conflict that could be detected and flagged with a warning.

@PaulMartinsen
Copy link
Author

PaulMartinsen commented Mar 22, 2023

That's a useful test case @gojimmypi . A couple of suggestions:

  • It could be simplified by dropping the -extfile and -extensions arguments ( I'm adding custom policies to the certificates when signing the CSR, which I don't believe is common, has been very complicated and not relevant here).
  • Please use the number 32473, the registered private enterprise number for documentation/examples, instead of 59430 in the subjAltName content.

It looks like key size plays a role in this problem. Consider two processes:

  1. Generate a private key on ESP32s3 → create CSR with OpenSSL → sign CSR with OpenSSL
  2. Generate a private key on ESP32s3 → create CSR on ESP32s3 → sign CSR with OpenSSL

OpenSSL here refers to OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022) running on Windows. The result for each process using typical key sizes is summarized below. The keys posted in the original issue were all 2048-bit. "ok" means I don't get any errors from OpenSSL when the CSR is signed. I haven't tested the signed certificates back on the ESP32s3 yet.

Key size [bits] Process 1 Process 2
512 ok signature mismatch
1024 ok ok
2048 signature mismatch signature mismatch

Unfortunately, relative paths are a necessary side effect of multiple targets, build systems and finite resources for me. But I agree, simpler is better!

@PaulMartinsen
Copy link
Author

Follow up observation for Process 2 with 1024 bit keys. OpenSSL reports the subject in the signed CSR as:
subject=C = NZ, L = Hamilton, O = "Blue Leaf Software, Ltd", CN = Lumos, favouriteDrink = BLS-001.
That last field should be serialNumber = BLS-001. Set on the ESP32s3 using:
strncpy(pRequest->subject.serialDev, "BLS-001", sizeof(pRequest->subject.serialDev));.

I noticed ASN_FAVOURITE_DRINK and ASN_SERIAL_NUMBER have the same value in asn.h (0x05), though I'm not sure if that's a related or different problem.

@gojimmypi
Copy link
Contributor

gojimmypi commented Mar 23, 2023

@PaulMartinsen I've been able to duplicate your issue on the ESP32-S3, even with fresh code and what I believe to be a good user_settings/h file.

Shown below is a table of RSA key bit length sizes, along with whether the keys were generated with HW or SW. This latest version of code is using wolfSSL to create and sign the generated CSR.

The links in this table are to cert_peek output files in my ./output directory.

Key size [bits] SW HW HW with patch ( see below)
512 ok signature mismatch ok w/ TFM patch
1024 ok ok ok w/ TFM patch
2048 ok signature mismatch ok w/ TFM patch

The cert_peek.sh script looks like this:

#!/bin/bash
if [ "$1" == "HW" ] || [ "$1" == "SW" ]; then
    echo HW/SW: $1
else
    echo "Not a valid HW/SW value: $1"
    exit 1
fi

if [ "$2" == "512" ] || [ "$2" == "1024" ] || [ "$2" == "2048" ]; then
    echo RSA size: $2
else
    echo "No a valid RSA size 512/1024/2048 value: $2"
    exit 1
fi


echo ""
echo "Files:"
echo ""

ls ./output/gojimmypi-ESP32-S3_"$1"_"$2"* -al
echo ""
openssl rsa -in "./output/gojimmypi-ESP32-S3_"$1"_"$2"_private_key.txt" -noout -text

echo ""
echo "Request:"
echo ""

openssl req -in "./output/gojimmypi-ESP32-S3_"$1"_"$2"_request.crt" -noout -text

echo ""
echo "CA sign:"
echo ""

openssl x509 -req -in "./output/gojimmypi-ESP32-S3_"$1"_"$2"_request.crt" \
        -days 10                                                          \
        -CA "private/CertificateAuthority-4096-PublicKey.crt"             \
        -CAkey "private/CertificateAuthority-4096-PrivateKey.pem"         \
        -passin pass:secret -CAcreateserial                               \
        -extfile "private/CertificateExtensions.txt"                      \
        -extensions LumosDevice                                           \
        -out ./output/gojimmypi-Device-Certificate_"$1"_"$2".crt -text

Note the naming convention is [username]-[Espressif chipset]_[HW or SW]_[RSA bit length]_[file content] for files that cert_peek.sh will process.

The code has a #define KEY_SIZE for values of 512, 1024 or 2048. HW/SW is still defined in the user_settings.h by leaving or commenting out the crypto defines:

/* Default is HW enabled unless turned off.
** Uncomment these lines for SW: */
/*
#define NO_ESP32WROOM32_CRYPT
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_AES
#define NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI
*/

Now to figure out why only HW 512 and 2048 bit lengths have OpenSSL complain about signature mismatches, with otherwise no errors or indications of any sort of problems.

gojimmypi added a commit to gojimmypi/wolfssl that referenced this issue Mar 24, 2023
@gojimmypi
Copy link
Contributor

I have an interim solution in place for this on my ED25519_SHA2_fix branch. The current wolfSSL master branch will need to completely turn off one of the hardware encryption features: NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI if this problem is encountered.

I've confirmed this is problematic on Xtensa architectures: ESP32 and ESP32-S3 with wolfSSL hardware encryption turned on..

As it turns out, the cause of the certificate signature failure is in the wolfcrypt/tfc.c for certain RSA bit lengths when Espressif Hardware Acceleration is turned on. There are 2 different problems.

The first is when doing RSA 512 bit: the fp_exptmod( ) returns an incorrect result when calling the HW esp_mp_exptmod().

The other one is for RSA 1024 bit: the mp_mulmod( ) returns a incorrect result. Note this one is related to the #define ESP_RSA_MULM_BITS 2000 setting in the default Espressif user_settings.h file. The HW esp_mp_mulmod() is only called when fp_count_bits(a) and fp_count_bits (b) are both greater than ESP_RSA_MULM_BITS (2000), otherwise it falls back to software calculation. I'll need to so some more homework on this one.

Curiously, the wolfssl_test passes all of the tests. Clearly there's a missing test that I'll need to add. As this is not a SHA test, I've not yet decided if I create a new issue, or just rename #5989.

For now, I've added compile-time macro gating that detects and falls back to software calculations for both of the problematic Espressif HW calcs. This requires that the RSA bit length be defined in the user_settings.h:

#define WOLFSSL_RSA_KEY_SIZE 1024

Currently if this is not defined, and the TFM USE_FAST_MATH is enabled, there will be a compile-time warning.

As a side note: For experimenting with various settings that get assigned with ./configure, see #5876. I've added this tool to my WIP branch. It helped me discover that the commandline options for certs turn on these:

#undef  WOLFSSL_KEY_GEN
#define WOLFSSL_KEY_GEN

#undef  WOLFSSL_CERT_REQ
#define WOLFSSL_CERT_REQ

#undef  WOLFSSL_CERT_GEN
#define WOLFSSL_CERT_GEN

#undef  WOLFSSL_CERT_EXT
#define WOLFSSL_CERT_EXT

but these are apparently enabled by default:

#undef  WOLFSSL_SYS_CA_CERTS
#define WOLFSSL_SYS_CA_CERTS

#undef  WOLFSSL_ASN_TEMPLATE
#define WOLFSSL_ASN_TEMPLATE

Note that I've also added #define WOLFSSL_SYS_CA_CERTS and #define WOLFSSL_ASN_TEMPLATE to the user_settings.h. See the user_settings.h file used during testing.

As noted above, this is tested with the the cert_peek.sh script that has since been updated to also accept a 3rd parameter for target chip set. (ESP32 or ESP32-C3 or ESP32-S3)

To test with existing files, there are 6 new files with a _fixed suffix in my ./output directory.

I'll need more time to dig into esp_mp_exptmod() and determine the actual root cause.

@anhu
Copy link
Member

anhu commented Mar 31, 2023

Great work @gojimmypi !!

@gojimmypi
Copy link
Contributor

Hi @PaulMartinsen a brief update: I'm still working on a better solution than just disabling the known problematic key lengths as noted in #6286.

I believe I have found the root cause. As the problem is not reproducible, even on a fixed binary when running the same code after fresh reboot, more testing is in order before I can confirm.

@PaulMartinsen
Copy link
Author

Thanks for persisting with this @gojimmypi . I've been using 1024-bit keys for now, which are okay for testing, but this is going to be another important bug-fix for the library! Appreciate your hard work on this.

@gojimmypi
Copy link
Contributor

Hi @PaulMartinsen - thank you! Indeed I've learned a lot during this exercise! At this point, I believe I have found the root cause of this issue. I have an initial fix with on my ED25519_SHA2_fix branch.

The problem seems to be a simple sign issue as noted in #6359, specifically related to the WOLFSSL_SP_INT_NEGATIVE setting. (used in esp32_mp.c, not in tfm.c). This certainly seems like a reasonable explanation as to why the problem was not reproducible.

I need to clean up my code and perform more testing, but I wanted you to have this information as soon as possible. It's curious that this problem does not manifest itself in other key sizes, such as the 1024-bit keys that you are using. I do need to revisit the operand pointer issue originally noted in #6359 and convince myself there's no problem there.

@gojimmypi
Copy link
Contributor

In addition to the sign issue in esp_mp_mul described above and shown here:

image

It appears there's another problem in esp_mp_exptmod where although the answer seems the same, the Y.used property is different as shown here:

image

When this condition is encountered, the CSR signing process fails.

@gojimmypi
Copy link
Contributor

Hi @PaulMartinsen

I believe I finally have this issue under control with the discovery of the fp_add and fp_cmp issues as noted in #6380.

For reference, it is interesting that OpenSSL fails here during the signing process:

image

I suppose once the underlying math is wrong, all bets are off on everything.

@PaulMartinsen
Copy link
Author

That's very interesting @gojimmypi . Mind boggling that the 1024-bit keys seem to work okay (I've made a few 1024-bit ones since we first found this issue, still with no problems). So I guess wrong math can give the right answer sometimes!

Thanks again for your persistence with this, rather thorny, problem. Much appreciated!

@gojimmypi
Copy link
Contributor

Hi @PaulMartinsen just an update on this topic: I've certainly not forgotten this issue even though there's be no recent update here. In fact, it continues to have my undivided attention. I've found a variety of peripheral issues and improvements that I'm working on. (e.g. see ESP32 Errata)

The end result should be quite nice. I'm also adding an entire testing infrastructure that will greatly ease the development and validation of the new ESP32-C3/C6 RISC-V Architecture, as soon as I can wrap up the ESP32/S3 Xtensa. This new feature has certainly be quite helpful to confirm proper operation using the wolfcrypt tests.

Thanks very much for your patience. Cheers

@PaulMartinsen
Copy link
Author

Fantastic to see this one resolved. Thanks to all and particularly @gojimmypi for all your hard work on it. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants