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

[Synthetics] Fix MTLS settings for synthetics service in Kibana #136267

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

lucasfcosta
Copy link
Contributor

Summary

This PR fixes the MTLS settings for the service in Kibana which broke due to recent changes in how we used devUrl and other settings to decide how to send requests.

Now, we'll always rejectUnauthorized certs unless users are sending requests to localhost and in dev mode, it doesn't matter whether they serve their own manifests or use devUrl.

Risk Matrix

  • Please review the certificate settings carefully to see if there's anything I missed here which would allow for unauthorised certs out of dev and localhost.

How to test this PR

  1. Checkout the latest version of the service (make sure it contains the recent changes to DX due to Makefile updates)

  2. Run the service as per the instructions on "how to run it locally" that are present in the docs

  3. Setup Kibana to use the service using devUrl and basicAuth. Ensure you can create and run monitors (try to use run-once as it's easier to test).

  4. Setup Kibana to use the service using devUrl and certificates by copying the crt and key in the service's folder to the Kibana config folder and using it in the Kibana config. For example:

    # Make sure you do NOT have username and password setup for basic auth
    xpack.uptime.service.devUrl: https://localhost:10001
    xpack.uptime.service.tls.certificate: config/123-client.pem
    xpack.uptime.service.tls.key: config/123-client.key

    Ensure you can create and run monitors (try to use run-once as it's easier to test).

  5. Setup Kibana to use a local manifest by creating a manifest file like the one below and serving it on port 8081 using npx http-server --port 8081.

    {
      "throttling": {
        "download": 20,
        "upload": 10
      },
      "locations": {
        "local": {
          "url": "https://localhost:10001",
          "geo": {
            "name": "My Test [local]",
            "location": {"lat": 41.25, "lon": -95.86}
          },
          "status": "ga"
        }
      }
    }

    Then update Kibana configs to use the served manifest:

    # Comment the devUrl!
    # xpack.uptime.service.devUrl: https://localhost:10001
    
    xpack.uptime.service.manifestUrl: http://localhost:8081/manifest.json
    
    # Make sure you do NOT have username and password setup for basic auth
    xpack.uptime.service.tls.certificate: config/123-client.pem
    xpack.uptime.service.tls.key: config/123-client.key

    Ensure you can create and run monitors (try to use run-once as it's easier to test).

For maintainers

@lucasfcosta lucasfcosta added Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes Synthetics labels Jul 13, 2022
@lucasfcosta lucasfcosta requested a review from a team as a code owner July 13, 2022 10:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Tested locally following provided instructions and it worked without a hitch, both run-once and cron monitors. Thanks for the fix. LGTM.

Comment on lines +61 to +62
const rejectUnauthorized = parsedTargetUrl.hostname !== 'localhost' || !this.server.isDev;
const baseHttpsAgent = new https.Agent({ rejectUnauthorized });
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this will prevent us from using basic auth in staging environment against qa or staging env. which is probably fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for adding this. I think it's a good think to clarify:

-> We couldn't use basic auth on those environments anyway because the server itself (service) will refuse basic auth.

The only environment in which basic auth should be enabled is dev.

Therefore this doesn't change the behaviour of the client.

@lucasfcosta
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lucasfcosta lucasfcosta merged commit f9e2ed4 into elastic:main Jul 18, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 18, 2022
@lucasfcosta lucasfcosta deleted the fix-kibana-mtls branch July 18, 2022 10:24
@awahab07 awahab07 self-assigned this Aug 18, 2022
@awahab07
Copy link
Contributor

Post FF Testing

The connection to the service works via basic auth and via MTLS.

@awahab07 awahab07 removed their assignment Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Synthetics Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants