Firmware : Adding Authentication gateway library to the mqtt messages (Closes #371)#413
Firmware : Adding Authentication gateway library to the mqtt messages (Closes #371)#413
Conversation
…mware/AUTH_MQTT/MQTT_Gateway/src/gateway_core.cpp, by remove the free heap bug
… devices in the ram and flash
…2 to make it more clear about the callback function
…line 1 of the file
…rmware build and for unit test
platformio.ini, change the build setting for each test
|
|
||
| Device* getDevice(const String& id); | ||
| const std::map<String, Device>& getAllDevices() const { return m_devices.devices; } | ||
| void approveDevice(const String& id, const DevicePerms& perms, const char* psk = nullptr); |
There was a problem hiding this comment.
A lot of these String uses, can they be converted over to std::array<char, size_t>? By requiring Strings that requires dynamic allocation. Unless necessary it's best we don't use dynamic.
There was a problem hiding this comment.
most of strings used for id which is a key at the map for saving devices.
i use it here
std::map<String, Device> devices;
| } | ||
|
|
||
| size_t cipherLen = len + RFC_8439_TAG_SIZE; | ||
| uint8_t* cipher = (uint8_t*)malloc(cipherLen); |
There was a problem hiding this comment.
Please do not use malloc. That will 100% cause memory fragmentation due to dynamic memory allocation and no MMU.
There was a problem hiding this comment.
yes, malloc is not safe to use i will try to use a fixed size buffer for the cipher
the esp32 have embedded MMU but cannot prevent from memory Fragmentation.
| size_t encLen = chacha20_poly1305_encrypt(cipher, dev.enc_key, nonce, | ||
| (uint8_t*)deviceId.c_str(), deviceId.length(), | ||
| plaintext, len); | ||
| if (encLen == (size_t)-1) { |
There was a problem hiding this comment.
There are a large number number of casts in this PR. Would you be able to leverage std::static_cast()?
As well, size_ts cannot be negative since they are unsigned values. What does casting -1 to a size_t accomplish?
There was a problem hiding this comment.
the -1 with casting size_t it to return the maximum size of size_t type
at the library the return of the function chacha20_poly1305_encrypt
can be (size_t)-1 at failure MG_OVERLAPPING
// pointers overlap if the smaller either ahead of the end,
// or its end is before the start of the other
//
// s_size should be smaller or equal to b_size
#define MG_OVERLAPPING(s, s_size, b, b_size) \
(MG_PM(s) < MG_PM((b) + (b_size))) && (MG_PM(b) < MG_PM((s) + (s_size)))
PORTABLE_8439_DECL size_t mg_chacha20_poly1305_encrypt(
uint8_t *restrict cipher_text, const uint8_t key[RFC_8439_KEY_SIZE],
const uint8_t nonce[RFC_8439_NONCE_SIZE], const uint8_t *restrict ad,
size_t ad_size, const uint8_t *restrict plain_text,
size_t plain_text_size) {
size_t new_size = plain_text_size + RFC_8439_TAG_SIZE;
if (MG_OVERLAPPING(plain_text, plain_text_size, cipher_text, new_size)) {
return (size_t) -1;
}
chacha20_xor_stream(cipher_text, plain_text, plain_text_size, key, nonce, 1);
poly1305_calculate_mac(cipher_text + plain_text_size, cipher_text,
plain_text_size, key, nonce, ad, ad_size);
return new_size;
}
There was a problem hiding this comment.
Right. That's in the library. I see you added the OVERLAPPING macro, but again, that raises the question of why have you reimplemented a lot of the functionality of the existing library? This adds a TREMENDOUS amount of code to maintain in our own project. Do you have a justification for why you have essentially just duplicated a library?
|
This PR is As well, the file One way to accomplish including an external git repo as a dependency is add it as a git submodule. Lastly, digging through the linked repo in the |
| @@ -0,0 +1,37 @@ | |||
| // Standalone ChaCha20-Poly1305 AEAD (RFC 8439) | |||
| // Extracted from Mongoose library (Public Domain) | |||
| // Original source: https://github.com/cesanta/mongoose | |||
There was a problem hiding this comment.
If you are going to add in a library yourself and make modifications, please are the version and commit that you are referencing. As well, please state the license the original source is license under, like an example in the project's own source
There was a problem hiding this comment.
okay, but i wil not going to use the chacha20.h
i am going to use only two files of the library and remove all the extracted files
I was tring to not compile all the file of the mongoose library i was need the encryption and hashing function which need to enable the TLS and compile all the TLS stack in the library. using git submodule. is a good point i need to read more about it and how i can do it because i didn't use it before. chacha20.h yes maybe i did some changes because i was facing some problem when receiving message and decrypt the message i got a bug that's memory access violation and the esp32 was rebooting. but the changes didn't solve it and found the problem with calling free() function. but i didn't turn the changes back because i was going to use the two files from mongoose library. |
2 participants
Links
What & Why
-what it do is checking if the device sending message to krake is authenticated by the user or not by checking a preshard password of the device to the Krake using a webpage dashboard.
-why we doing this, to be able to control which device can publish Alarms to Krake and we can have some informations about the device eg(what is this device type, that it's status , and permissions, ...).
Validation / How to Verify
there's two steps to Verify:
Checklist