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

DOCSP-41508: TLS #37

Merged
merged 5 commits into from
Aug 21, 2024
Merged

DOCSP-41508: TLS #37

merged 5 commits into from
Aug 21, 2024

Conversation

mcmorisi
Copy link
Collaborator

For the most part, completely lifted from the Coroutine version.

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41508
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/kotlin-sync/DOCSP-41508-TLS/connect/tls/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link

netlify bot commented Aug 14, 2024

Deploy Preview for docs-kotlin-sync ready!

Name Link
🔨 Latest commit a315380
🔍 Latest deploy log https://app.netlify.com/sites/docs-kotlin-sync/deploys/66c5e4712d873b0008007b44
😎 Deploy Preview https://deploy-preview-37--docs-kotlin-sync.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

source/connect.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
Comment on lines 218 to 223
.. warning::

Disabling hostname verification can make your configuration
`insecure <https://tlseminar.github.io/docs/mostdangerous.pdf>`__.
Disable hostname verification only for testing purposes or
when there is no other alternative.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Specifying this option in a production environment makes your application insecure and potentially vulnerable to expired certificates and foreign processes posing as valid client instances.

Copy link
Collaborator

Choose a reason for hiding this comment

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

from the GO copy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will modify the warning.

source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
source/connect/tls.txt Outdated Show resolved Hide resolved
@mcmorisi mcmorisi requested a review from rustagir August 15, 2024 15:37
Copy link
Collaborator

@rustagir rustagir left a comment

Choose a reason for hiding this comment

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

a few more small things, but approved!

Comment on lines 27 to 33
.. note:: Debugging TLS

If you experience trouble setting up your TLS connection, you can
use the ``-Djavax.net.debug=all`` system property to view helpful
log statements. See `Debugging SSL/TLS connections
<https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/ReadDebug.html>`__
in the Java language documentation for more information.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S; i think this note is coming too early in the page. Could move to the end of the Enable TLS section?

Comment on lines 62 to 64
To enable TLS on a connection by using a connection string, set the connection string
parameter ``tls`` to ``true`` in the connection string passed to
``MongoClient.create()``, as shown in the following code:
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: reduce the number of times "connection string" is mentioned:

Suggested change
To enable TLS on a connection by using a connection string, set the connection string
parameter ``tls`` to ``true`` in the connection string passed to
``MongoClient.create()``, as shown in the following code:
To enable TLS on a connection by using a connection string, set the
``tls`` option to ``true`` in the options parameter and pass the string to
``MongoClient.create()``, as shown in the following code:

Comment on lines 109 to 112
<https://letsencrypt.org/>`__. As a result, you can connect to a
:atlas:`MongoDB Atlas </>` instance, or any other
server whose certificate is signed by an authority in the JRE's default
certificate store, with TLS enabled without configuring the trust store.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<https://letsencrypt.org/>`__. As a result, you can connect to a
:atlas:`MongoDB Atlas </>` instance, or any other
server whose certificate is signed by an authority in the JRE's default
certificate store, with TLS enabled without configuring the trust store.
<https://letsencrypt.org/>`__. As a result, you can enable TLS when connecting to a
:atlas:`MongoDB Atlas </>` instance, or any other
server whose certificate is signed by an authority in the JRE's default
certificate store, without configuring the trust store.


An application that initiates TLS/SSL requests needs to set two JVM system
properties to ensure that the client presents a TLS/SSL certificate to
the MongoDB server:
Copy link
Collaborator

Choose a reason for hiding this comment

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

lgtm

Comment on lines 150 to 153
The JVM key store saves certificates that securely identify your {+language+}
application to other applications. By using these certificates, other
applications can prove that the connection to your application is
genuine and secure from tampering by third parties.
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: I think this is a duplicate paragraph from the prev section. Consider removing

Comment on lines 182 to 183
Find an example showing how to configure a client to use an ``SSLContext``
instance in the :ref:`Customize TLS Configuration with an SSLContext section of this guide <kotlin-sync-tls-custom-sslContext>`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Find an example showing how to configure a client to use an ``SSLContext``
instance in the :ref:`Customize TLS Configuration with an SSLContext section of this guide <kotlin-sync-tls-custom-sslContext>`.
Find an example showing how to configure a client to use an ``SSLContext``
instance in the :ref:`kotlin-sync-tls-custom-sslContext` section of this guide.

Comment on lines 212 to 213
To restrict your application to use only the TLS 1.2 protocol, set the
``jdk.tls.client.protocols`` system property to "TLSv1.2".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To restrict your application to use only the TLS 1.2 protocol, set the
``jdk.tls.client.protocols`` system property to "TLSv1.2".
To restrict your application to use only the TLS 1.2 protocol, set the
``jdk.tls.client.protocols`` system property to ``"TLSv1.2"``.

@mcmorisi mcmorisi requested review from a team, stIncMale and rozza and removed request for a team August 15, 2024 18:53
@rozza rozza removed the request for review from stIncMale August 21, 2024 09:06
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM

@mcmorisi mcmorisi merged commit 626ef2e into mongodb:master Aug 21, 2024
5 of 6 checks passed
@mcmorisi mcmorisi deleted the DOCSP-41508-TLS branch August 21, 2024 12:59
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.

3 participants