silverstripe / silverstripe-graphql

Serves Silverstripe data as GraphQL representations
BSD 3-Clause "New" or "Revised" License
52 stars 61 forks source link

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

Open madmatt opened 2 years ago

madmatt commented 2 years ago

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 commented 2 years ago

Relevant: https://github.com/silverstripe/silverstripe-graphql/pull/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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

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

baukezwaan commented 1 year ago

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