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

Rover fails to introspect local HTTPS server with self-signed certificate #720

Closed
ZachGoldberg opened this issue Aug 7, 2021 · 8 comments · Fixed by #837
Closed

Rover fails to introspect local HTTPS server with self-signed certificate #720

ZachGoldberg opened this issue Aug 7, 2021 · 8 comments · Fixed by #837
Assignees
Labels
feature 🎉 new commands, flags, functionality, and improved error messages

Comments

@ZachGoldberg
Copy link

ZachGoldberg commented Aug 7, 2021

Description

We test locally using HTTPS with a self signed certificate and I'm basically looking for a ALLOW_INSECURE flag for rover.

Steps to reproduce

openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout server.key -out server.crt

https
  .createServer(
    {
      key: fs.readFileSync(`./ssl/server.key`),
      cert: fs.readFileSync(`./ssl/server.crt`),
    },
    app
  )
  .listen(PORT);

rover subgraph introspect 'https://localhost:4000/graphql

Expected result

Rover produces a schema as usual

Actual result

error[E028]: Could not connect to https://localhost:4000/graphql.
              Make sure the endpoint is accepting connections and is spelled correctly
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Command failed with exit code 1: yarn graph:generate:local-rover
    at makeError (/home/zgoldberg/workspace/growflow/arc-gateway/node_modules/execa/lib/error.js:60:11)
    at handlePromise (/home/zgoldberg/workspace/growflow/arc-gateway/node_modules/execa/index.js:118:26)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  shortMessage: 'Command failed with exit code 1: yarn graph:generate:local-rover',
  command: 'yarn graph:generate:local-rover',
  escapedCommand: '"yarn graph:generate:local-rover"',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}```

### Environment

Run `rover info` and paste the results here 

```Rover Info:
Version: 0.1.10
Install Location: /home/zgoldberg/.nvm/versions/node/v14.15.5/lib/node_modules/@apollo/rover/node_modules/binary-install/bin/rover
OS: Ubuntu 18.04 (bionic) [64-bit]
Shell: /bin/bash```

@ZachGoldberg ZachGoldberg added bug 🐞 triage issues and PRs that need to be triaged labels Aug 7, 2021
@abernix
Copy link
Member

abernix commented Aug 9, 2021

Seems reasonable. Prior art here likely being Python's PYTHONHTTPSVERIFY and Node.js' NODE_TLS_REJECT_UNAUTHORIZED.

Perhaps we would name this ROVER_TLS_VERIFY and have it default to 1 and be set to 0 when ignoring verification is desired?

Alternatively, I could imagine we perhaps want more granularity for this and we want this to be part of either the user profile or a sub-graph specific configuration? (e.g., each subgraph may have different desires to have its validation ignored!)

Quite related, I could also see us needing a ~ROVER_EXTRA_CA_CERTS, with same/similar functionality as NODE_EXTRA_CA_CERTS – and these I could definitely see being different for different subgraphs, though I could also imagine multiple CA Certs could just be concatenated together.

@joeynenni
Copy link

We are in the same situation. A --insecure option like curl would be great!

@EverlastingBugstopper
Copy link
Contributor

Is it possible to add these self-signed certs to OpenSSL's trusted certs in /etc/ssl/certs? Do these same requests work w/curl w/o an --insecure option or do workflows like this typically rely on --insecure options?

@abernix
Copy link
Member

abernix commented Aug 27, 2021

Possibly! I guess on macOS this might require users to add it to their System Keychain. That might be fine, but also might be difficult if the certificate they merely want to support is, for example, checked into a repository. (I'm not sure if this is common, nor am I sure it's particuarly insecure for particular users' cases.)

(Or if they want it to be a transient / non-permanent thing)

@joeynenni
Copy link

@EverlastingBugstopper Adding the cert is non-trivial and something we want to avoid if possible. Our current development environment does require us to use --insecure for much of the local testing. Asking teams to use the old version of Apollo CLI mixed with rover is a bit awkward, any ideas if this feature will happen? Thanks!

@EverlastingBugstopper EverlastingBugstopper added feature 🎉 new commands, flags, functionality, and improved error messages 2021-09 and removed triage issues and PRs that need to be triaged bug 🐞 labels Sep 21, 2021
@EverlastingBugstopper
Copy link
Contributor

I've opened a pull request that adds this capability in case anybody wants to take a look.

@EverlastingBugstopper
Copy link
Contributor

cc @joeynenni and @ZachGoldberg - Rover v0.3.0 is out today which includes flags for disabling hostname validation and/or certificate validation

@joeynenni
Copy link

This is great, thank you @EverlastingBugstopper!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants