-
Notifications
You must be signed in to change notification settings - Fork 784
Sim7600 SSL support v.0.12.0 #808
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
base: master
Are you sure you want to change the base?
Conversation
…ficates and private keys
|
First off, thanks for this work. This PR looks like exactly what I was looking for. I'm currently trying to run this code on the sim7600 but it seems like the Currently getting the following error trace: https://registry.platformio.org/tools/platformio/framework-arduino-sam Thanks again |
|
@floBik So I replaced all I use the SIM7600E modem, and these commands seems to fail in the But the Any help will be much appreciated! |
|
Hi everyone, I have just committed a fix for the problem with the .isEmpty() function, using .length() instead. This should solve the problem. @arduino12 If this doesn't solve your problem, post your code so I can have a look. |
|
Hi, Thanks @floBik ! I don't have the SIM7600 modem by now so I can't test your fix.. int8_t send_at_cmd(const char *cmd)
{
modem.sendAT(cmd);
return modem.waitResponse() == 1 ? 0 : 1;
}
int8_t send_https_post_request(const char *ip_address, const char *url, const char *content)
{
int ret;
char cmd[256];
send_at_cmd("+HTTPTERM");
if (send_at_cmd("+HTTPINIT"))
return -1; // Failed starting HTTPS service!
sprintf(cmd, "+HTTPPARA=\"URL\",\"https://%s%s\"", ip_address, url);
if (send_at_cmd(cmd))
return -2; // Failed setting HTTPS URL!
if (send_at_cmd("+HTTPPARA=\"CONTENT\",\"application/json\""))
return -3; // Failed setting HTTPS content-type!
sprintf(cmd, "+HTTPDATA=%d,2000", strlen(content));
modem.sendAT(cmd);
if (modem.waitResponse("DOWNLOAD") != 1)
return -4; // Failed setting HTTPS user-data!
modem.stream.print(content);
modem.stream.flush();
if (modem.waitResponse() != 1)
return -5; // Failed sending HTTPS user-data B!
if (send_at_cmd("+HTTPACTION=1"))
return -6; // Failed sending HTTPS post request!
return 0;
}I think its better to use the AT HTTP commands instead of making HTTP(S) packets using ArduinoHttpClient for this modem- Arad :) |
st31ny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heyy, I just had a look on your code — thank you very much for your contribution!
Your implementation looks indeed pretty good and I tested it successfully. I just found some minor (mostly stylistic) issues.
| return true; | ||
| } | ||
|
|
||
| bool setClientPrivateKey(const String& certificateName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we only need one function to set/reset both client cert and client pk, since setting only the cert or only the pk does not make any sense.
| if (certificates[mux].length() != 0 && | ||
| clientCertificates[mux].length() != 0 && | ||
| clientPrivateKeys[mux].length() != 0) { | ||
| authmode = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can actually be ORed into authmode, so no need for the extra branch.
| if (waitResponse(5000L) != 1) return false; | ||
|
|
||
| sendAT(GF("+CCHOPEN="), mux, ',', GF("\""), host, GF("\","), port); | ||
| // The reply is OK followed by +CIPOPEN: <link_num>,<err> where <link_num> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy/paste?
| */ | ||
| // No functions of this type supported | ||
| public: | ||
| bool addCertificate(const char* certificateName, const char* cert, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, you want to override addCertificateImpl and deleteCertificateImpl resp. here (and have it protected).
| String certificates[TINY_GSM_MUX_COUNT]; | ||
| String clientCertificates[TINY_GSM_MUX_COUNT]; | ||
| String clientPrivateKeys[TINY_GSM_MUX_COUNT]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed?
| at->sendAT(GF("+CIPRXGET=4,"), mux); | ||
| size_t result = 0; | ||
| if (at->waitResponse(GF("+CIPRXGET:")) == 1) { | ||
| at->streamSkipUntil(','); // Skip mode 4 | ||
| at->streamSkipUntil(','); // Skip mux | ||
| result = at->streamGetIntBefore('\n'); | ||
| at->waitResponse(); | ||
| } | ||
| // DBG("### Available:", result, "on", mux); | ||
| if (!result) { | ||
| at->sockets[mux]->sock_connected = at->modemGetConnected(mux); | ||
| } | ||
| return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look like an appropriate error handling (why CIPRXGET here?)… Maybe just check modemGetConnected() like for non-ssl.
| // NOT SUPPORTED | ||
|
|
||
|
|
||
| class GsmClientSecureSim7600 : public GsmClientSim7600 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my understanding, the SIM7600 only supports two SSL connections — so there should probably be a lower limit somewhere here to enforce this.
|
|
||
| return true; | ||
| // We assume this works, so we can do ssh disconnect too | ||
| // stop the SSH client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // stop the SSH client | |
| // stop the SSL client |
|
@floBik Hello, very interested in your project, I saw in another post of yours that you sucessfully implemented MQTT over TLS on the SIM7600 with this code, could you provide me an example code? Thank you very much. |
|
I've incorporated this code on a side fork that I will (hopefully) finish merging here soon. On that fork, I created an example that connected to AWS IoT Core, which is MQTT on TLS: https://github.com/EnviroDIY/TinyGSM/tree/master/examples/AWS_IoTCore One of the reasons I haven't pushed it here yet is that I have made changes to modems that I don't own to test (including the SIM7600), so if you can test it and let me know how it works for you, I'd appreciate it! |
|
Hey @alex-reusables, @SRGDamia1 included most of my changes in her fork. This made most of this review/MR irrelevant. After testing, I only noticed a few bugs, which I addressed in this PR: EnviroDIY#52 |
|
@floBik Thanks for following up! We're using this TinyGSM library in a production context so would prefer to avoid pulling in forks or branches as it gets a bit tricky to manage on our end. |
Same here — really looking forward to get this merged to master! @floBik @SRGDamia1, is there anything we can help on to get this merged soon? |
|
@vshymanskyy Anything we can do to speed this along? The SIM7600 is an extremely popular chip and SSL is a must nowadays - it's been nearly a year now since this was first added, and most of our projects are now using @floBik's fork and you can imagine that's not best practice at all. |
|
Great work @floBik @Matt-Stedman ! SSL support for SIMCOM7600 is a long time overdue. I now have native SSL support between my 7600, TinyGSM and ArduinoHTTPClient working perfectly with FirebaseRTDB for POST and GET requests without the need for SSLClient. Certificate loading and activating is also working. Has anyone been able to get BIN file download working with the forked SSL library? I can obtain the headers and content length ok, but as soon as the BIN file download starts it fails at 6026 bytes. With the AT debugger in play it seems that even the 6026 has data corruption. This is working when SSLClient is installed, but I really want to use your SSL supported library 12:31:00.424 -> Content length: 784496 I am using #define TINY_GSM_RX_BUFFER 1024 and have tried reducing and increasing buffer size with no success. Any comments and suggestions appreciated. Just wanted to check if anyone has tried <1MB BIN file downloading over SSL? |
|
@vshymanskyy Can we please get this merged? Or are you waiting for me to implement/accept @st31ny's changes? I'll see about doing that now, otherwise @floBik it may be you? |
Co-authored-by: @Matt-Stedman
The following PR is an evolution of PR #767 and should resolve issue #786 . A few things have been done:
Feel free to test this and give some feedback if something should be changed.