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

document tls.cert.auth #3010

Merged
merged 5 commits into from
Feb 1, 2020
Merged

document tls.cert.auth #3010

merged 5 commits into from
Feb 1, 2020

Conversation

HHHartmann
Copy link
Member

Fixes #2578.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

<Description of and rationale behind this PR>
Trying to add documentation for tls.cert.auth
Please review and tell me if it is nonsens as I don't really know much about tls.

@HHHartmann HHHartmann requested a review from nwf January 15, 2020 20:44
@nwf
Copy link
Member

nwf commented Jan 15, 2020

Do we know that this actually works?

@dtran123
Copy link

Yes this works. If your TLS handshake doesn't take too much RAM. In my case, I have solid connections to an MQTT broker with full certification + key authentication (which I control).

@marcelstoer marcelstoer added this to the Next release milestone Jan 16, 2020
docs/modules/tls.md Outdated Show resolved Hide resolved
docs/modules/tls.md Outdated Show resolved Hide resolved
app/modules/tls.c Show resolved Hide resolved
Copy link
Member

@nwf nwf left a comment

Choose a reason for hiding this comment

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

Please address comments above. This is a NodeMCU TLS client API, and the documentation must clearly reflect that, and the examples should involve client private keys. Be careful when spelling out who's doing the connecting and proving and verifying and all that.

Hope is gets better ...
docs/modules/tls.md Outdated Show resolved Hide resolved
@HHHartmann
Copy link
Member Author

Hope it is good to go now. If not please let me know.

Copy link
Member

@marcelstoer marcelstoer left a comment

Choose a reason for hiding this comment

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

GitHub didn't allow me to edit this directly.

docs/modules/tls.md Outdated Show resolved Hide resolved
docs/modules/tls.md Outdated Show resolved Hide resolved
docs/modules/tls.md Outdated Show resolved Hide resolved
@HHHartmann
Copy link
Member Author

@marcelstoer found the checkbox. You should now be able to edit.

@marcelstoer marcelstoer merged commit 76e9f1a into nodemcu:dev Feb 1, 2020
@HHHartmann HHHartmann deleted the tls.cert.auth branch May 1, 2020 16:40
marcelstoer pushed a commit that referenced this pull request Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants