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

SSL support for MQTT connection #577

Merged
merged 5 commits into from
Aug 10, 2019
Merged

SSL support for MQTT connection #577

merged 5 commits into from
Aug 10, 2019

Conversation

kleini
Copy link
Collaborator

@kleini kleini commented Mar 10, 2019

This is stolen from timpur's v2.1 branch.
I like to encrypt my MQTT connections because passwords are transmitted over MQTT in clear text. Passwords transmitted in clear text should be always avoided.

@timpur
Copy link
Contributor

timpur commented Mar 10, 2019

Just double check my change for the Mac string

const uint8_t MAX_MAC_LENGTH = 6;

:P

@amayii0
Copy link

amayii0 commented Mar 11, 2019

Just a very generic comment, I'm not following heavily the dev but looking here and there.
Would be great if consts are commented with a sample value, from this, how are you storing MAC?

@kleini
Copy link
Collaborator Author

kleini commented Mar 11, 2019

Up to this point I only picked some commits. Will read and try to verify the code now.

@kleini
Copy link
Collaborator Author

kleini commented Mar 11, 2019

@timpur I hope I fixed all char array and string length.

@timpur
Copy link
Contributor

timpur commented Mar 13, 2019

Looks good (I believe these changes were in 2.1) ?

@kleini
Copy link
Collaborator Author

kleini commented Mar 16, 2019

Just to mention it, because I stumbled again over it:
ASYNC_TCP_SSL_ENABLED requires PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH otherwise the SSL connections do not work at all.

@timpur
Copy link
Contributor

timpur commented Mar 17, 2019

Just to mention it, because I stumbled again over it:
ASYNC_TCP_SSL_ENABLED requires PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH otherwise the SSL connections do not work at all.

Maybe @kleini document that ?

@kleini
Copy link
Collaborator Author

kleini commented Mar 17, 2019

@timpur documented SSL connections for MQTT.

@bodiroga
Copy link
Collaborator

Should we merge this PR in the 'develop' branch or the 'develop-v3' branch? IMHO, we should leave 'develop' only for bugfixes and add new features to 'develop-v3' 😉

@kleini
Copy link
Collaborator Author

kleini commented Apr 17, 2019

Yes, will rebase my work on develop-v3 the next days.

@stritti
Copy link
Collaborator

stritti commented Jul 15, 2019

Yes, will rebase my work on develop-v3 the next days.

@kleini could you rebase it to be merged then?

@kleini kleini changed the base branch from develop to develop-v3 July 16, 2019 19:41
@kleini
Copy link
Collaborator Author

kleini commented Jul 16, 2019

I rebased my work on develop-v3. The build is broken because of the resulting image size. Will try to check, why the image size is limited this way.

timpur and others added 5 commits July 17, 2019 21:11
Copy link
Collaborator

@bodiroga bodiroga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work Markus!

@kleini
Copy link
Collaborator Author

kleini commented Jul 18, 2019

@bodiroga This is not my work. @timpur implemented this and I just try to get it merged as I consider encryption for MQTT connections as essential (clear text passwords).

@stritti stritti merged commit 9db189f into homieiot:develop-v3 Aug 10, 2019
@kleini kleini deleted the ssl branch September 25, 2019 16:15
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

Successfully merging this pull request may close these issues.

5 participants