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

srp errors #6870

Closed
AchimPieters opened this issue Oct 16, 2023 · 29 comments
Closed

srp errors #6870

AchimPieters opened this issue Oct 16, 2023 · 29 comments
Assignees

Comments

@AchimPieters
Copy link

Version

5.6

Description

On my project : https://github.com/AchimPieters/esp32-homekit-demo and in this case the submodule: https://github.com/AchimPieters/esp32-homekit

reproductions steps can be found here: https://github.com/AchimPieters/esp32-homekit-demo

I tried to look into the encodign errors like:

 4122 |             Base64_Encode_NoNl((const unsigned char *)shaHash, 4, encodedHash, &len);
      |             ^~~~~~~~~~~~~~~~~~
      |             base64_encode 

here: https://www.wolfssl.com/documentation/manuals/wolfssl/coding_8h.html#function-base64_encode_nonl

and

project/components/homekit/src/crypto.c:59:23: error: unknown type name 'Srp'
   59 | int wc_SrpSetUsername(Srp * srp, byte * secret, word32 size) {
      |                       ^~~
/project/components/homekit/src/crypto.c:80:1: error: unknown type name 'Srp'
   80 | Srp *crypto_srp_new() {
      | ^~~
/project/components/homekit/src/crypto.c: In function 'crypto_srp_new':
/project/components/homekit/src/crypto.c:81:5: error: unknown type name 'Srp'
   81 |     Srp *srp = malloc(sizeof(Srp));
      |     ^~~
/project/components/homekit/src/crypto.c:81:30: error: 'Srp' undeclared (first use in this function); did you mean 'srp'?
   81 |     Srp *srp = malloc(sizeof(Srp));
      |                              ^~~
      |                              srp
/project/components/homekit/src/crypto.c:81:30: note: each undeclared identifier is reported only once for each function it appears in
/project/components/homekit/src/crypto.c:84:13: error: implicit declaration of function 'wc_SrpInit' [-Werror=implicit-function-declaration]
   84 |     int r = wc_SrpInit(srp, SRP_TYPE_SHA512, SRP_CLIENT_SIDE);
      |             ^~~~~~~~~~
/project/components/homekit/src/crypto.c:84:29: error: 'SRP_TYPE_SHA512' undeclared (first use in this function); did you mean 'WC_HASH_TYPE_SHA512'?
   84 |     int r = wc_SrpInit(srp, SRP_TYPE_SHA512, SRP_CLIENT_SIDE);
      |                             ^~~~~~~~~~~~~~~
      |                             WC_HASH_TYPE_SHA512

for these here: https://www.wolfssl.com/documentation/manuals/wolfssl/srp_8h.html

But I can't get my head around it.

@dgarske
Copy link
Contributor

dgarske commented Oct 16, 2023

Hi @AchimPieters ,

Have you confirmed you are setting WOLFCRYPT_HAVE_SRP and WOLFSSL_BASE64_ENCODE in your user_settings.h?

Thanks,
David Garske, wolfSSL

@gojimmypi
Copy link
Contributor

Hi @AchimPieters & apologies for my tardy response.

I've taken a look at your latest changes. Looks like you are making good progress,

I saw that you had opened this issue on your esp32-homekit-demo repo, but later deleted it? After typing a reply in #5909 I see you opened this issue. Good.

@dgarske is correct: you are missing some settings. See my commit that adds those settings to your user_settings.h

There's still an error in the crypto.c, specifically this line I believe should be WC_SHA512 like this:

        int r = wc_HKDF(
                WC_SHA512,
                key, key_size,
                salt, salt_size,
                info, info_size,

                output, *output_size
                );

@AchimPieters
Copy link
Author

AchimPieters commented Oct 17, 2023

Hi @gojimmypi my fiend!

No worries! I'm makeing good progress indeed, en these finil changes seem to be te solution to my problem, i'll will give you an update a.s.a.p.

@dgarske Thank you for your swift response, as siad above , it seems to do the trick!

You are bothe awesome!

@AchimPieters
Copy link
Author

unfortunally I was to fast:

>>> crypto_srp_init: Generating salt
>>> crypto_srp_init: Setting SRP username
>>> crypto_srp_init: Setting SRP params
>>> crypto_srp_init: Setting SRP password
>>> crypto_srp_init: Getting SRP verifier
>>> crypto_srp_init: Failed to get SRP verifier (code -1)
!!! HomeKit: [Client 1] Failed to initialize SRP
>>> client_sendv: [Client 1] Sending payload: HTTP/1.1 200 OK\x0D\x0AContent-Type: application/pairing+tlv8\x0D\x0ATransfer-Encoding: chunked\x0D\x0AConnection: keep-alive\x0D\x0A\x0D\x0A
>>> client_sendv: [Client 1] Sending payload: \x36\x0D\x0A\x06\x01\x02\x07\x01\x01\x0D\x0A
>>> client_sendv: [Client 1] Sending payload: \x30\x0D\x0A\x0D\x0A
>>> homekit_client_process: [Client 1] Finished processing
>>> HomeKit: [Client 1] Closing client connection from 192.168.178.73

have too liook into this one...

@gojimmypi
Copy link
Contributor

Hi @AchimPieters

A couple of things come to mind: flash space and stack size.

I see this "Warning: The smallest app partition is nearly full" at build time:

Successfully created esp32 image.
Generated C:/test/esp32-homekit-demo-gojimmypi.17/examples/led/build/VisualGDB/Debug/main.bin
[839/839] cmd.exe /C "cd /D C:\test\esp32-homekit-demo-gojimmypi.17\examples\led\build\VisualGDB\Debug\esp-idf\esptool_py && python C:/SysGCC/esp32/esp-idf/v5.1/components/partition_table/check_sizes.py --offset 0x8000 partition --type app C:/test/esp32-homekit-demo-gojimmypi.17/examples/led/build/VisualGDB/Debug/partition_table/partition-table.bin C:/test/esp32-homekit-demo-gojimmypi.17/examples/led/build/VisualGDB/Debug/main.bin"
main.bin binary size 0xf6520 bytes. Smallest app partition is 0x100000 bytes. 0x9ae0 bytes (4%) free.
Warning: The smallest app partition is nearly full (4% free space left)!
------------------- Memory utilization report -------------------
Used DATA_FLASH: 196KB out of 8192KB (2%)
Used INSTR_FLASH: 695KB out of 3264KB (21%)
Used INSTR_RAM: 85KB out of 128KB (66%)
Used DATA_RAM: 33KB out of 320KB (10%)
Used Unknown: 24 bytes

========== Project Build Summary ==========

Still, only "nearly" full, but...

Try using the Single factory app (large) no OTA

When I run the app, the assert fails to initialize, as MAGIC_OFFSET is zero (the first parameter to esp_partition_write(partition... as I have no HomeKit data:

I (2653) esp_netif_handlers: sta ip: 192.168.1.17, mask: 255.255.255.0, gw: 192.168.1.1
WiFI ready
I (2653) I: >>> HomeKit: Starting server

E (2653) E: !!! HomeKit: HomeKit partition is not found

I (2663) I: >>> HomeKit: Resetting HomeKit storage

image

also consider using ESP_LOGI() (from #include <esp_log.h>) instead of printf in debug.h .

Let me know if you have any tips on initializing with homekit_storage_reset.

@AchimPieters
Copy link
Author

AchimPieters commented Oct 18, 2023

@gojimmypi did you follow the reproduction steps here: https://github.com/AchimPieters/esp32-homekit-demo

stack size.

Keep in mind that 12288 (bytes) is size for ESP32 which is rather small compared to how much memory ESP32 has (512KB, although almost half of that is taken by the system, it's still a decent amount of available memory). For ESP8266 it's only 2048 (words, which is 8192 bytes). Considering a basic ESP8266 example (e.g. an LED) has ~36K of RAM available not being able to generate a 5K response means that you have something else taking a lot of memory. I recommend exploring possibilities to reduce that part. E.g. HomeKit server itself tries to minimize amount of memory it uses when it generates it's responses. E.g. for JSON generation it has a custom manual generation which allows to generate JSON in order as it would appear in output and thus it can send chunked responses as they are being generated.

  • Select Partition table and then Partition Table(Single factory app, no OTA) Set to Custom partition table CSV

partiions.cvs

# ESP-IDF Partition Table
# Name,   Type, SubType, Offset,  Size, Flags
nvs,      data, nvs,     0x9000,  0x5000,
phy_init, data, phy,     0xe000,  0x1000,
homekit,  data, homekit, 0xf000,  0x1000,
factory,  app,  factory, 0x10000, 1M,

also consider using ESP_LOGI() (from #include <esp_log.h>) instead of printf in debug.h . - have to look in too.

PS go to Component config then homekit an select debug output

@gojimmypi
Copy link
Contributor

did you follow the reproduction steps

Basically, yes (although I'm not on a Mac), the instructions at AchimPieters/esp32-homekit-demo but I see there are additional instructions at the recently updated esp32-homekit/components/homekit @ 9acee90 needed for first-time users that include the custom partitions.csv file. Thank you! Apparently that's what I was missing since it's not in the main instructions.

how much memory ESP32 has (512KB, although almost half of that is taken by the system

yes, from the docs:

image

And so for the LED example, I am now much more successful, although I do see these warnings in the code at build time:

[851/866] Building C object esp-idf/main/CMakeFiles/__idf_main.dir/main.c.obj
In file included from C:/test/esp32-homekit-demo.18/components/homekit/include/homekit/homekit.h:4,
                 from ../../../main/main.c:33:
../../../main/main.c:137:44: warning: initialized field overwritten [-Woverride-init]
  137 |         HOMEKIT_ACCESSORY(.id=1, .category=homekit_accessory_category_lighting, .services=(homekit_service_t*[]){
      |                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/test/esp32-homekit-demo.18/components/homekit/include/homekit/types.h:249:20: note: in definition of macro 'HOMEKIT_ACCESSORY'
  249 |                 ## __VA_ARGS__ \
      |                    ^~~~~~~~~~~
../../../main/main.c:137:44: note: (near initialization for '(anonymous).category')
  137 |         HOMEKIT_ACCESSORY(.id=1, .category=homekit_accessory_category_lighting, .services=(homekit_service_t*[]){
      |                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/test/esp32-homekit-demo.18/components/homekit/include/homekit/types.h:249:20: note: in definition of macro 'HOMEKIT_ACCESSORY'
  249 |                 ## __VA_ARGS__ \
      |                    ^~~~~~~~~~~
[852/866] Building C object esp-idf/homekit/CMakeFiles/__idf_homekit.dir/src/server.c.obj

The LED example otherwise builds, flashes, and runs... but I don't see the srp errors mentioned in this issue.

I (2645) esp_netif_handlers: sta ip: 192.168.1.17, mask: 255.255.255.0, gw: 192.168.1.1
WiFI ready
>>> HomeKit: Starting server
>>> HomeKit: Using existing accessory ID: AE:F6:59:80:37:D4
>>> HomeKit: Configuring mDNS
I (7065) wifi:<ba-add>idx:1 (ifx:0, 50:39:2f:80:09:00), tid:4, ssn:0, winSize:64

PS go to Component config then homekit an select debug output

I see a bit more output, but still no SRP errors, as mentioned #6870 (comment):

I (1728) wifi:AP's beacon interval = 102400 us, DTIM period = 3
I (2638) esp_netif_handlers: sta ip: 192.168.1.17, mask: 255.255.255.0, gw: 192.168.1.1
WiFI ready
>>> HomeKit: Starting server
>>> HomeKit: Using existing accessory ID: AE:F6:59:80:37:D4
>>> HomeKit: Configuring mDNS
>>> homekit_setup_mdns: Accessory Setup ID = 1QJ8
>>> homekit_run_server: Starting HTTP server
I (26708) wifi:<ba-add>idx:1 (ifx:0, 50:39:2f:80:09:00), tid:4, ssn:0, winSize:64

Can you please confirm the LED example is where this message originates?

>>> crypto_srp_init: Failed to get SRP verifier (code -1)
!!! HomeKit: [Client 1] Failed to initialize SRP
>>> client_sendv: [Client 1] Sending payload: HTTP/1.1 200 OK\x0D\x0AContent-Type: application/pairing+tlv8\x0D\x0ATransfer-Encoding: chunked\x0D\x0AConnection: keep-alive\x0D\x0A\x0D\x0A

There's mention of an HTTP server, but I am unable to connect to it: in my case: sta ip: 192.168.1.17, connection refused, no ESP32 error message.

As I am otherwise unable to reproduce this problem. Perhaps there's some other client software involved?

@AchimPieters
Copy link
Author

@gojimmypi good to hear you are getting up to date, message originates when trying to add the LED to homekit with the QR-Code or manual by typing the code: 338-77-883

qrcode

@gojimmypi
Copy link
Contributor

message originates when trying to add the LED to homekit

You mean an Apple HomeKit? I don't have one. I have an ESP32 with no camera.

manual by typing the code: 338-77-883

where do I do this?

@AchimPieters
Copy link
Author

@gojimmypi take your iphone:

  1. Power on your accessory* and make sure that it's nearby.
  2. Open the Home app on your iPhone or iPad and tap Add .
  3. Tap Add Accessory.
  4. Add Accessory appears first after tapping Add
  5. Follow the instructions to scan a code or hold your device near the accessory to add it.
  6. Tap Continue, then tap Done.

@gojimmypi
Copy link
Contributor

take your iphone:

I also don't have an iPhone.

@AchimPieters
Copy link
Author

Well that's a bummer....

@gojimmypi
Copy link
Contributor

lol, I was hoping for an answer just a little something better. ;)

Surely there's a way to test this without me needing to have a Homekit device or iPhone?

Otherwise we can get one of the other engineers involved.

@AchimPieters
Copy link
Author

AchimPieters commented Oct 19, 2023

@AchimPieters
Copy link
Author

@gojimmypi do you know where I can find the description crypto_srp_init: Failed to get SRP verifier (code -1)

@gojimmypi
Copy link
Contributor

Hi @AchimPieters I've been unable to get an iPhone emulator running on my Windows machine. I do have a friend with an iPhone, so I hope to try that soon.

where I can find the description crypto_srp_init: Failed to get SRP verifier

That appears to originate in esp32-homekit/src/crypto.c. The wc_SrpGetVerifier is in srp.c and does not immediately appear to have an obvious cause (-1 is the least useful of errors). But for reference the wolfcrypt errors are in wolfssl/wolfcrypt/error-crypt.h.

Just out of curiosity, have you tried increasing the amount of stack space? Also consider sprinkling some stack high water mark checks:

ESP_LOGI(TAG, "Stack HWM: %d\n", uxTaskGetStackHighWaterMark(NULL));

@AchimPieters
Copy link
Author

@gojimmypi the stack size is defined in port.h

#pragma once

#include <stdint.h>

uint32_t homekit_random();
void homekit_random_fill(uint8_t *data, size_t size);

void homekit_system_restart();
void homekit_overclock_start();
void homekit_overclock_end();

#ifdef ESP_OPEN_RTOS
#include <spiflash.h>
#define ESP_OK 0
#endif

#ifdef ESP_IDF
#include <esp_system.h>
//#include <esp_spi_flash.h>
#include "spi_flash_mmap.h" // V5
#include "esp_flash.h" // V5

//https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32c3/migration-guides/release-5.x/storage.html
//https://docs.espressif.com/projects/esp-idf/en/v5.0-beta1/esp32/api-reference/storage/spi_flash.html
#define ESP_FLASH_SECTOR_SIZE SPI_FLASH_SEC_SIZE
//esp_err_t esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint32_t length)
#define esp_flash_read(NULL, buffer, addr, size) (esp_flash_read(NULL, (buffer), (addr), (size)) == ESP_OK)
//esp_err_t esp_flash_write(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length)
#define esp_flash_write(NULL, buffer, addr, size) (esp_flash_write(NULL, (buffer), (addr), (size)) == ESP_OK)
//esp_err_t esp_flash_erase_region(esp_flash_t *chip, uint32_t start, uint32_t len)
#define esp_flash_erase_region(NULL, addr, size) (esp_flash_erase_region(NULL, (addr), (size) / ESP_FLASH_SECTOR_SIZE) == ESP_OK)
#endif


#ifdef ESP_IDF
#define SERVER_TASK_STACK 12288
#else
#define SERVER_TASK_STACK 1664
#endif


void homekit_mdns_init();
void homekit_mdns_configure_init(const char *instance_name, int port);
void homekit_mdns_add_txt(const char *key, const char *format, ...);
void homekit_mdns_configure_finalize();

What stack size would you recommend?

Regarding the stack high water mark checks: on my todo list ;)

@gojimmypi
Copy link
Contributor

What stack size would you recommend?

hm, well, that depends. I wonder how much memory is available? I might try adding another 10K or 20K and see if there's a difference in operation. It's hard to say without seeing the problem myself. We don't even know that it's a stack error.

Speaking of which: I should be able to have an iPhone to test on Saturday. So let me know if there are any other details I might need to be aware of.

btw - now that you are not using a managed component, feel free to clone the latest wolfSSL. If parallel to your project clone, the cmake should find it automatically. Upcoming version will support environment variable for WOLFSSL_ROOT, but you can also set the path manually in the CMakeLists.txt.

I have a more recent CMakeLists.txt if you'd like to experiment with that. It includes the option to assign WOLFSSL_ROOT (i.e. the clone location) in an environment variable.

@AchimPieters
Copy link
Author

@gojimmypi thank you for reply, I personally don't think it's a stack error, but I will give it a try.

That's great news, when you open the HomeKit app, just select the + to add the accessory and follow the steps. The only thing is to open a new terminal window and type screen /dev/tty.usbserial-01FD1166 115200
Replace /dev/tty.usbserial-01FD1166 with your USB port. AND when do the step idf.py menuconfig - component config - homekit select debug output

At this point, we got a "working" setup, I want to follow through until it's fully working, before tweaking and using the latest wolfSSL.

@jucham
Copy link

jucham commented Oct 21, 2023

Hi @AchimPieters , I'm the guy with the iPhone. I can reproduce your error. I also tried to raise the stack size of 20k as suggested by @gojimmypi but it does not solve the problem. @gojimmypi and I are going to do some tests later in the day.

@AchimPieters
Copy link
Author

@jucham thank you for joining and testing!

@dgarske
Copy link
Contributor

dgarske commented Oct 23, 2023

Hi @jucham and @AchimPieters ,

This sounds like a math variable size issue. Based on the code here (https://github.com/AchimPieters/esp32-homekit/blob/main/src/crypto.c#L129) for a 1024 sized verify request I think you need 8192 bit math support.

If you are building with USE_FAST_MATH (tfc.c) then you need #define FP_MAX_BITS (8192 * 2).
If you are building with WOLFSSL_SP_MATH_ALL (sp_int.c) then set #define SP_INT_BITS 8192

If that does not work please share me your current user_settings.h to review.

Thanks,
David Garske, wolfSSL

@jucham
Copy link

jucham commented Oct 23, 2023

Hi @dgarske , thank you very much for the hint, I was a bit lost to be honest.

You're right. In this case it's built with USE_FAST_MATH and if I define FP_MAX_BITS like you said it solves the problem.

@AchimPieters can you confirm that ?

In this file :
(https://github.com/AchimPieters/esp32-homekit-demo/blob/c283a6db79c83fd2d26b1b23a83817ee1c13fabb/components/wolfssl/include/user_settings.h#L117)

/* rsa primitive specific definition */
#if defined(WOLFSSL_ESPWROOM32) || defined(WOLFSSL_ESPWROOM32SE)
    /* Define USE_FAST_MATH and SMALL_STACK                        */
    #define ESP32_USE_RSA_PRIMITIVE
    #define FP_MAX_BITS (8192 * 2)
    /* threshold for performance adjustment for hw primitive use   */
    /* X bits of G^X mod P greater than                            */
    #define EPS_RSA_EXPT_XBTIS           36
    /* X and Y of X * Y mod P greater than                         */
    #define ESP_RSA_MULM_BITS            2000
#endif

@AchimPieters
Copy link
Author

@dgarske thank you for joining the discussion! And providing the missing solution!

@jucham and @gojimmypi the solution provide from @dgarske and presented by you worked. I will update the repo, and then you can test it too. This is awesome, thank you guys.

@gojimmypi Now that the code is working, we can do some fine-tuning ;)

@gojimmypi
Copy link
Contributor

@AchimPieters and @jucham thank you both for all your help. That's so cool it's working!

I put together a little blog to acknowledge your work.

Ok to close this issue, @AchimPieters ?

@AchimPieters
Copy link
Author

Thanks to everybody who help with the issues!
@gojimmypi special thanks, too, you for your long term dedication and the little blog, but you misspelled my name it's Pieters not Peters ;) If you could change that one.

@gojimmypi
Copy link
Contributor

The spelling has been fixed @AchimPieters & we look forward to seeing your ESP32 HomeKit progress!

@AchimPieters
Copy link
Author

AchimPieters commented Jan 21, 2024

Hi @gojimmypi how are you doing? I saw there is a new version 5.6.6 with support for more than only the esp32, but for more esp32 series, but how to incorporate this into my project?

Now compiling only possible for the ESP32, and not for: esp32s2, esp32c3, esp32s3, esp32c2, esp32c6, esp32h2 and esp32p4

the current one gives me errors when compiling, for example esp32c2:

fmacro-prefix-map=/project/examples/led=. -fmacro-prefix-map=/opt/esp/idf=/IDF -fstrict-volatile-bitfields -fno-jump-tables -fno-tree-switch-conversion -std=gnu17 -Wno-old-style-declaration -MD -MT esp-idf/wolfssl/CMakeFiles/__idf_wolfssl.dir/wolfcrypt/src/asn.c.obj -MF esp-idf/wolfssl/CMakeFiles/__idf_wolfssl.dir/wolfcrypt/src/asn.c.obj.d -o esp-idf/wolfssl/CMakeFiles/__idf_wolfssl.dir/wolfcrypt/src/asn.c.obj -c /project/components/wolfssl/wolfcrypt/src/asn.c
In file included from /project/components/wolfssl/wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h:56,
                 from /project/components/wolfssl/wolfssl/wolfcrypt/sha256.h:101,
                 from /project/components/wolfssl/wolfssl/wolfcrypt/random.h:111,
                 from /project/components/wolfssl/wolfssl/wolfcrypt/tfm.h:47,
                 from /project/components/wolfssl/wolfssl/wolfcrypt/integer.h:39,
                 from /project/components/wolfssl/wolfssl/wolfcrypt/asn.h:46,
                 from /project/components/wolfssl/wolfcrypt/src/asn.c:94:
/opt/esp/idf/components/esp_rom/include/esp32/rom/ets_sys.h:15:2: error: #error "This header should only be included when building for ESP32"
   15 | #error "This header should only be included when building for ESP32"
      |  ^~~~~
ninja: build stopped: subcommand failed.
ninja failed with exit code 1, output of the command is in the /project/examples/led/build/log/idf_py_stderr_output_8023 and /project/examples/led/build/log/idf_py_stdout_output_8023
root@22fb157df2ff:/project/examples/led# 

@gojimmypi
Copy link
Contributor

Hello @AchimPieters ! Good to hear from you.

esp32 series, but how to incorporate this into my project?

Great news! The latest code works with all the ESP32 flavors! Please confirm you are using files from the 5.6.6 release (or later).

The examples now contain an updated components/wolfssl directory, in particular a user_settings.h that has a section to detect which ESP32 flavor was selected with the idf.py set-target command. (e.g. CONFIG_IDF_TARGET_ESP32C6, etc)

Hardware acceleration is enabled for all the SoC's supported by ESP-IDF v5.1. Newer chips will work, with HW disabled for any of those undetected.

Note there was a post release change in #7148 that fixed the environment variable option to point to your wolfssl source, if not in the same [workspace]/github clone structure.

If you cannot get it working, please open a new issue so we can track it with visibility. Thanks!

Best Regards.

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

No branches or pull requests

5 participants