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

[Security] Disable introspection in test and live modes #420

Open
madmatt opened this issue Dec 16, 2021 · 5 comments
Open

[Security] Disable introspection in test and live modes #420

madmatt opened this issue Dec 16, 2021 · 5 comments

Comments

@madmatt
Copy link
Member

madmatt commented Dec 16, 2021

I'm a GraphQL newbie, so I might be missing something here, but is there any reason why you might want introspection enabled in production for the /admin/graphql admin GraphQL endpoint? During development it's obviously helpful, and if you're shipping frontend, non-CMS GraphQL APIs you might want this, but by default I think it makes sense to disable this for the /admin/graphql endpoint.

Thoughts? Am I way off base with this one?

edit: The module README talks about this, and mentions that Apollo needs some amount of introspection enabled, but it sounds like disabling introspection is quite a common security requirement for GraphQL endpoints, so I'm not sure which is 'correct' here.

Context: This has come up in quite a few penetration tests that we've had done internally at Silverstripe Ltd. so I'm keen to get some thought from the core team on whether this is possible to do or not.

@unclecheese
Copy link

Relevant: #400

We're also tracking this as a security issue. Gets a very low score because by default we only expose the admin schema, which is already behind auth.

Still, it's a footgun, and we should provide sensible defaults.

@maxime-rainville
Copy link
Contributor

OWASP GraphQL cheat sheet treats it more as a best practice.

Just looking at the CMS graphql schema, that's entirely public. All you would need to do to find it is installed the CMS in and look at it.

If you had your own schema and we're developing an application targeted at other devs, there's a perfectly valid use case where you would want to leave introspection on to make it easy for them to debug their app.

Can't imagine we want to backport this to GraphQL v3.

It's just a question of adding an option to disable introspection to GraphQL v4. The key question now is do we want introspection ON or OFF by default on TEST/LIVE?

@baukezwaan
Copy link

It's just a question of adding an option to disable introspection to GraphQL v4. The key question now is do we want introspection ON or OFF by default on TEST/LIVE?

Best practise would be to turn it off on production- and test-environments. And you can leave it open on dev-environments.

.

Disables schema introspection outside of dev

For anyone bumping into this thread, looking for how to disable introspection on GraphQL 4v. This is what you can do:

Create a middleware PHP-file:

    namespace MyApp\GraphQL\Middleware;

    class IntrospectionMiddleware implements QueryMiddleware
    {
        public function process(Schema $schema, string $query, array $context, array $vars, callable $next)
        {
            DocumentValidator::addRule(new DisableIntrospection());
            
            return $next($schema, $query, $context, $vars);
        }
    }

And a yml-config file, to set the middelware for everywhere except dev:

---
Name: 'app-graphql-middlewares'
Except:
  environment: dev
---
SilverStripe\Core\Injector\Injector:
  # use SilverStripe\GraphQL\QueryHandler\QueryHandlerInterface.default or .admin if you just want to apply to one schema
  SilverStripe\GraphQL\QueryHandler\QueryHandlerInterface:
    properties:
      Middlewares:
        disableIntrospection: '%$MyApp\GraphQL\Middleware\IntrospectionMiddleware'

Thanks to @unclecheese for the help on this

@sunnysideup
Copy link
Contributor

sunnysideup commented Oct 19, 2022

@baukezwaan - does this work for you? I dont get any errors, yet to test the outcome.

@baukezwaan
Copy link

Yes, it worked for us. I do remember having some issues locally with cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants