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

[TT-1570/TT-12607]document validating client certificates against rootCA #5069

Merged

Conversation

jeffy-mathew
Copy link
Contributor

@jeffy-mathew jeffy-mathew commented Jul 12, 2024

User description

For internal users - Please add a Jira DX PR ticket to the subject!


https://tyktech.atlassian.net/browse/TT-12607 ---

Preview Link

https://deploy-preview-5069--tyk-docs.netlify.app/docs/nightly/basic-config-and-security/security/mutual-tls/client-mtls/


Description


Screenshots (if appropriate)


Checklist

  • I have added a preview link to the PR description.
  • I have reviewed the suggestions made by our AI (PR Agent) and updated them accordingly (spelling errors, rephrasing, etc.)
  • I have reviewed the guidelines for contributing to this repository.
  • I have read the technical guidelines for contributing to this repository.
  • Make sure you have started your change off our latest master.
  • I labelled the PR

PR Type

Documentation


Description

  • Added documentation on using root Certificate Authority (CA) certificates as client certificates for static mutual TLS (mTLS) authentication.
  • Explained the key points and limitations, including the validation process and the incompatibility with dynamic mTLS.

Changes walkthrough 📝

Relevant files
Documentation
client-mtls.md
Document use of root CA certificates for static mTLS authentication

tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md

  • Added a new section explaining the use of root Certificate Authority
    (CA) certificates for static mTLS authentication.
  • Detailed the key points and limitations of using root CA certificates.

  • +11/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Add a warning about the security implications of using a root CA certificate as a client certificate

    Consider adding a warning or note about the security implications of using a root CA
    certificate as a client certificate, as this might expose the system to certain
    risks if not managed properly.

    tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md [108]

    -Yes, you can upload a root CA certificate as a client certificate for static mTLS authentication.
    +Yes, you can upload a root CA certificate as a client certificate for static mTLS authentication. However, be aware of the security implications of this setup, as improper management can expose your system to increased security risks.
     
    Suggestion importance[1-10]: 9

    Why: Highlighting the security implications is crucial for users to understand the potential risks, which significantly improves the security awareness of the documentation.

    9
    Enhancement
    Clarify the distinction between static and dynamic mTLS configurations

    It's recommended to clarify the distinction between static and dynamic mTLS in the
    documentation to avoid confusion. The current explanation might lead users to think
    that dynamic mTLS might work in some scenarios with root CA certificates.

    tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md [114]

    -This approach does not work with dynamic mTLS.
    +This approach works only with static mTLS and is not compatible with dynamic mTLS configurations.
     
    Suggestion importance[1-10]: 8

    Why: The suggestion improves clarity by explicitly stating that the approach works only with static mTLS, which helps prevent potential confusion about the compatibility with dynamic mTLS.

    8
    Link to additional resources for formatting and trusting root CA certificates

    To ensure the information is up-to-date and accurate, consider linking to additional
    resources or documentation that explains how to ensure a root CA certificate is
    "properly formatted and trusted" within a system's certificate store.

    tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md [116]

    -Note: Ensure that your root CA certificate is properly formatted and trusted within your system's certificate store.
    +Note: Ensure that your root CA certificate is properly formatted and trusted within your system's certificate store. For more details on formatting and trusting certificates, refer to [link to relevant documentation].
     
    Suggestion importance[1-10]: 6

    Why: Providing a link to additional resources is helpful for users seeking more detailed information, although it is a minor enhancement.

    6
    Maintainability
    Improve the readability and clarity of the key points section

    To enhance the readability and clarity, consider using a bulleted list instead of
    numbered points for the key points section, as the order of points does not imply a
    sequence.

    tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md [110-114]

     Key points:
    -1. The root CA certificate can be used to be uploaded as a client certificate.
    -2. Clients with certificates signed by this CA will be accepted.
    -3. During verification, Tyk gateway traverses the certificate chain for validation.
    -4. This approach does not work with dynamic mTLS.
    +- The root CA certificate can be uploaded as a client certificate.
    +- Clients with certificates signed by this CA will be accepted.
    +- During verification, Tyk gateway traverses the certificate chain for validation.
    +- This approach works only with static mTLS.
     
    Suggestion importance[1-10]: 7

    Why: Changing to a bulleted list enhances readability and clarity, making it easier for users to understand the key points without implying a sequence.

    7

    Copy link

    netlify bot commented Jul 12, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit 7fddf4d
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/6690ea3a366ea4000860bdea
    😎 Deploy Preview https://deploy-preview-5069--tyk-docs.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.

    Copy link

    netlify bot commented Jul 12, 2024

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit b762796
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/66a0cc756f268c0008d6c2f0
    😎 Deploy Preview https://deploy-preview-5069--tyk-docs.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.

    @jeffy-mathew jeffy-mathew force-pushed the feat/TT-1570/TT-12607/allow-ca-cert-static-mtls-documentation branch from 7fddf4d to 24cce41 Compare July 12, 2024 08:47
    @jeffy-mathew jeffy-mathew requested a review from lghiur July 12, 2024 09:49
    Copy link
    Contributor

    @andyo-tyk andyo-tyk left a comment

    Choose a reason for hiding this comment

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

    Some observations/questions

    @jeffy-mathew jeffy-mathew force-pushed the feat/TT-1570/TT-12607/allow-ca-cert-static-mtls-documentation branch 3 times, most recently from a006fbc to f8ed260 Compare July 19, 2024 16:25
    Copy link
    Contributor

    @andyo-tyk andyo-tyk left a comment

    Choose a reason for hiding this comment

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

    Great, thanks.

    Copy link
    Contributor

    @dcs3spp dcs3spp left a comment

    Choose a reason for hiding this comment

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

    @jeffy-mathew Will approve ready for release after review comments relating to risks of root CA addressed.

    Could you confirm if this PR is targeting release 5.5.0?

    @letzya letzya force-pushed the feat/TT-1570/TT-12607/allow-ca-cert-static-mtls-documentation branch from bfcf10d to b762796 Compare July 24, 2024 09:42
    @letzya letzya merged commit 48d4c54 into master Jul 24, 2024
    9 checks passed
    @letzya letzya deleted the feat/TT-1570/TT-12607/allow-ca-cert-static-mtls-documentation branch July 24, 2024 09:53
    @letzya
    Copy link
    Collaborator

    letzya commented Jul 24, 2024

    /release to release-5.4

    Copy link

    tykbot bot commented Jul 24, 2024

    Working on it! Note that it can take a few minutes.

    tykbot bot pushed a commit that referenced this pull request Jul 24, 2024
    …tCA (#5069)
    
    * document validating client certificates against rootCA
    
    * address review comments
    
    * Update tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md
    
    * Update tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md
    
    ---------
    
    Co-authored-by: Yaara <[email protected]>
    
    (cherry picked from commit 48d4c54)
    Copy link

    tykbot bot commented Jul 24, 2024

    @letzya Succesfully merged PR

    buger added a commit that referenced this pull request Jul 24, 2024
    …certificates against rootCA (#5069)
    
    [TT-1570/TT-12607]document validating client certificates against rootCA (#5069)
    
    * document validating client certificates against rootCA
    
    * address review comments
    
    * Update tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md
    
    * Update tyk-docs/content/basic-config-and-security/security/mutual-tls/client-mtls.md
    
    ---------
    
    Co-authored-by: Yaara <[email protected]>
    @andyo-tyk
    Copy link
    Contributor

    For the record - this feature is arriving in 5.5.0. Don't think it's the end of the world to have merged to release-5.4 docs.

    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.

    4 participants