[meta] Decide on and implement additional security hardenings for JSON:API and other HTTP APIs

I went ahead and parented all the existing issues (although a couple of them look to be sort-of-duplicates of each other). I think there's also several to file yet before we mark #3035979: [DISCUSSION] Should a facility be added to JSON:API which limits automatic exposure of all entities fixed.

This was not proposed in the other issue because that issue was JSON:API specific and time-constrained.

I'm very interested in exploring ways that we can force entity type and field implementations to consider their security in the abstract, not tied to any particular representation (via Form API, JSON:API, config export, etc.).

I have lots of ideas for how we might do that:

1. I think it would be good to sacrifice some DX in exchange for stricter security by returning AccessResult:: forbidden('Access logic has not yet been implemented for this (entity|field), follow the guidelines at <link> to implement a secure control.') from EntityAccessControlHandler. Currently, checkFieldAccess returns allowed by default (I'm not sure what it does at the entity level). This would be limited to Drupal 9 of course.
2. For Drupal 8, a compromise could be to create a special subclass of AccessResult called UnconfirmedAccessResult and use that in the base implementation. We could then throw some deprecation notices when these are encountered as an end result. This object could have a ->confirm() method on it to make it easy to affirm the default behavior in the base class, if it doesn't need to be overriden.
3. Following up on the last idea, we could have a "safe mode" that treats all unconfirmed results as forbidden. Toggling this behavior would be a way to track down vulnerabilities.
4. Another useful tool would be an auditing mechanism which would provide security hardening suggestions/recommendations. I saw a blog post about an entity level tool that already exists, do we know if there's a field level tool?
5. A really nice low-hanging thing we could do is have a flowchart for implementors that helps them evaluate their access controls and ensure they've thought of everything. This would also be a way for the security team to audit and approve entity types that could be considered safe to enable write operations on by default.
6. One thing that has bitten us before is that access operations (view, create, edit, update, ...) are not clearly defined anywhere. We have loose conventions for what they mean, but no standardization. I know that changing them from strings would be highly disruptive, but what about creating a constant like EntityAccessControleHandlerInterface::VIEW = 'view' with a clear definition of what it means, then making it a code smell to use raw strings?
7. We're missing access check methods for both entities and fields that do not rely on having an instance of the object for which they check access. This has caused us a lot of pain WRT API-driven entity listings and filtering. Having these would also save ourselves a lot of DB round trips if we could check access prior to executing a query or entity loads.

Finally, I'd like to suggest that we view API-First Drupal as an opportunity to make all of Drupal more secure...

There are far fewer moving parts to consider in the API-First world. There are no form alters, no Form API (no magic array keys!), no Twig templates, no ways to execute a DB query or render a View in a template, no places an entity might accidentally be exposed on a forgotten page via a custom block, ... the list goes on.

JSON:API exposes a Drupal site's data in such an unadulterated fashion, it provides a great mental exercise to gut-check your access results and it encourages better organized code. An implementor doesn't have to wonder if it's better to add an #access render array key value or implement a field access hook. There's just one way to do it!

In short, while it's a bit scary and opens up another surface to attack, the API-First architecture can bring a lot of clarity to a significantly muddied space.

</pontification> 😉

Lots of great stuff in #3, thanks @gabesullice!

Adding a few stragglers from #3035979: [DISCUSSION] Should a facility be added to JSON:API which limits automatic exposure of all entities that don't have issues yet to the remaining tasks.

Then, two more things that I think we should add are:

1. Ideas for promoting further security research on contrib code with JSON:API. Many contrib vulnerabilities continue to be found with REST, and if they also exist for JSON:API they become more severe because of JSON:API's more permissive configuration.
2. Ideas for educating developers about writing secure code for an API-first Drupal. (Similar to what @gabesullice alludes to at the end of #3, I think.)

Another thing we should look at specifically is how to further harden the default "read everything" behavior. Most security team members were comfortable with our MVP in #3039568: Add a read-only mode to JSON:API, but a few raised concerns that this default behavior was still too permissive in terms of the risk of information disclosure.

One particular topic that came up is config entities, which many developers might not think of as being potentially available outside their modules' admin UIs. There's also the added risk that info disclosures with config entities might contain additional data that could be used to attack the site.

So adding those to the IS bullets as well!

@effulgentsia, I think #3040372: Allow for API-First content without requiring all configuration to also be API-first is great, but I think there might be additional things beyond #3040364: Allow modules to declare the entity types they want to consume with HTTP APIs that we should pursue to harden the read-everything behavior.

@Wim Leers added #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs) for an issue we encountered for (un-)exposing Layout Builder defaults data, which adds data to the third-party settings of bundle config entities.

Whoa! Finally ready to get this going again.

I'd like for all of us to come to a consensus on what does and does not make sense to implement in order to further harden JSON:API from a security standpoint. In undertaking that process, I think we can also prioritize those things that we do want to tackle.

To help us accomplish that, I went through all of the child issues and aggregated them into a matrix of sorts. This matrix was useful to me because I found it pretty difficult to keep the many child issues of this discussion in my head at once and even harder to consider them relative to one another. Hopefully you all find it helpful as well.

For each issue, I created number of axes by which to evaluate the proposal:

• Does the proposal require contrib code to be written/changed?
• Does the proposal require require custom code to be written/changed?
• Does the proposal require config/configuration pages?
• Does the proposal make JSON:API safer by default?
• Is the proposal actionable by the end-user?
• Does the proposal make JSON:API easier or harder for a newcomer to use?

Each proposal was ranked -1, 0 or 1, depending on whether the proposal had a negative outcome, no effect or a positive outcome.

Finally, I summed these values up and also ranked each proposal with a very rough level-of-effort (LOE). LOE is ranked 0 if it the work has already been done, 1 if it is something fairly straightforward, 2 if it is something that will take a moderate amount of work and 3 for the proposal that seemed most complicated to implement.

I think the two biggest shortcomings with this matrix approach is that it is fairly subjective and all of the axes are equally weighted. I've made the document open for public commenting. If you feel I've misrepresented whether something is negative, neutral or positive just comment on the cell. Same if you have a question. If I've missed an axis completely, just add a comment on this issue with the axis label and your ranking for each issue and I'll add it to the matrix.

Matrix available here.

Early conclusions:

There's a really nice correlation between things which are easier to implement and that have a positive impact. The two issues with the highest LOE are also among the issues that have the least positive outcome:

I propose we close these two issues:

• #3037534: Add a mechanism to mark TypedData (entities and fields) internal via settings.php would be tremendously complicated to implement, complicated to configure and does not make JSON:API safer by default. While it is the proposal that offers the most granular tool for securing JSON:API, that pro doesn't seem to outweigh the costs IMO.
• #3040364: Allow modules to declare the entity types they want to consume with HTTP APIs this proposal would be a little less complicated to implement than the former, but it would require contrib and custom code and it would make JSON:API harder to use. JSON:API has strived to be very easy to use adopt even for non-Drupal developers. This would reverse that. It would make JSON:API safer-by-default and wouldn't require contrib though. Perhaps the idea could be refined to require less contrib/custom code (i.e. via usage detection), but I think that would only increase the LOE further.

On a more positive note, there is some low-hanging fruit:

I think we should make this one our first priority:

• #3037942: Add facility to allow security advisories to advise to disable certain routes to mitigate security risk temporarily has the second highest positive outcome and falls into the easier-to-implement LOE. It could benefit all of Drupal, not just JSON:API and would provide a new tool for the security teams belt.

The issue with the highest positive outcome is thankfully among the middle LOEs.

I think these issues should be our second highest priority:

1. #3040372: Allow for API-First content without requiring all configuration to also be API-first would make JSON:API easier to use while at the same time making it safer by default. It isn't end-user actionable, but I think it would dramatically reduce the attack surface area of JSON:API. I think its one drawback is that it is quite blunt. I suspect that it will lead us directly to...
2. #3040354: Introduce entity type annotation key for indicating that an entity type is *safe* for HTTP API usage should be merged with the previous issue or made a required followup to it.

What about the rest?

• #3039712: Discuss whether we want an entity access audit status report is just about discussion. I think we should have the discussions in parallel to the prioritized issues, then reevaluate after they're finished. The prioritized issues will make an audit report actionable, which would make the report more useful.
• #3039715: Discuss whether we want an API routes audit status report is the same as the above.
• #3039687: Add an API read-only mode to Drupal core is about porting the JSON:API read-only mode to REST. Personally, I'm fine putting this on the back-burner. I don't see a lot of value in making it generic. @Wim Leers may disagree.
• #3037039: Create a public API for indicating resource types should not be exposed this one does not provide a ton of value from a security hardening perspective relative to the other issues IMO. That's because it isn't safer by default and it requires custom code to be used. That said, I think it has lots of value from a non-security perspective. I propose we move it under #3032787: [META] Start creating the public PHP API of the JSON:API module since JSON:API Extras is already using an @internal API to accomplish the same thing. This will make Drupal more stable regardless of its security outcomes so it doesn't need a priority under this meta issue IMO.
• #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs) wouldn't provide a lot of extra value if we implement #3040372: Allow for API-First content without requiring all configuration to also be API-first. There's also some really unfortunate confusion about the meaning of internal that muddies the water around this issue. I think we should leave it on this list of issues, but at the lowest of priorities.

#8 🥳 👏👏👏👏👏👏

I am missing one axis: Does the proposal make other HTTP APIs safer by default?.

I transplanted your feedback/analysis of every issue onto the corresponding core issue, to ensure that people reading those respective issues and not this particular issue are also awrae.

RE: closed.

• #3037534. I too have concerns. I agree it would not make JSON:API safer by default. But like you said, it'd allow a site with a very knowledgeable site owner to tweak what is exposed via HTTP APIs for an increased level of safety. Posted comment: #3037534-10: Add a mechanism to mark TypedData (entities and fields) internal via settings.php.
• #3040364. I share the strong concerns. Posted comment: #3040364-6: Allow modules to declare the entity types they want to consume with HTTP APIs.

RE: first priority. Agreed.

RE: second priority:

• #3040372. Agreed.
• #3040354. I am not sure that this should be merged. I think both approaches can be considered orthogonal. Because for #3040372, it's possible to add a reference to a config entity type that was not previously reachable from content, and that should cause it to be exposed automatically.

• #3039712: +1
• #3039715: +1
• #3039687: heh, I indeed disagree, but not strongly: see my comment at #3039687-5: Add an API read-only mode to Drupal core.
• #3037039: +1!!!
• RE: #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs) not providing a lot of value if we do #3040372: Allow for API-First content without requiring all configuration to also be API-first: I tentatively disagree here. See #3043245-7: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs).

I added your axis. I don't think it really changes my conclusions on its own (I have yet to read your comments on specific issues though). Notably, it pushed #3039687: Add an API read-only mode to Drupal core and #3037534: Add a mechanism to mark TypedData (entities and fields) internal via settings.php out of the "neutral" bucket into a slight positive. That makes sense to me.

Thanks for transplanting my comments! I'm going to go read all of your replies (and possibly reply to you). Then I'll post another comment back here summarizing any noteworthy updates to our thinking.

Okay, status update as promised:

• #3037534: Add a mechanism to mark TypedData (entities and fields) internal via settings.php: @Wim Leers agreed w/ my initial concerns and added new ones. I proposed that we keep the issue open 2 more weeks for additional input. If no one argues for keeping it open in that time, let's close it.
• #3040364: Allow modules to declare the entity types they want to consume with HTTP APIs: Same as above.
• #3040354: Introduce entity type annotation key for indicating that an entity type is *safe* for HTTP API usage: I now agree with @Wim Leers. This issue should not be merged. My reasoning is different than @Wim Leers's, but the conclusion is the same (see #3040354-6: Introduce entity type annotation key for indicating that an entity type is *safe* for HTTP API usage).
• #3039712: Discuss whether we want an entity access audit status report: I proposed that this be moved to the core ideas queue so that it can get some more traction. I think that's the right process for it. Everyone involved thinks it's a good idea. So I think that discussion has concluded and its time for a broader, more formal review.
• #3039715: Discuss whether we want an API routes audit status report: Whoa! @Wim Leers created a followup that @xjm requested on this issue. I added it to my matrix and I it sky-rocketed to the top of the list of useful things to do. I think we should close this issue and instead focus on the followup, I think @xjm will need to agree though.
• #3039687: Add an API read-only mode to Drupal core: Added my thoughts and reply to @Wim Leers in #7 of that issue. Still needs more discussion.
• #3037039: Create a public API for indicating resource types should not be exposed: Unless someone disagrees in a comment on this issue in the next 2 weeks, I'll change its parent as suggested.
• #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs): replied to @Wim Leers, we're still not on the same page about this.
• #3068275: Add status report message about JSON:API's read-only mode: this is new and I think it should be concurrent with our first priority.

Patch created for #3068275: Add status report message about JSON:API's read-only mode.

#11 was a great status update. But that's 10 weeks ago. What's the status today?

1. #3037534: Add a mechanism to mark TypedData (entities and fields) internal via settings.php was closed after 10 weeks of waiting for feedback.
2. #3040364: Allow modules to declare the entity types they want to consume with HTTP APIs was also closed after 10 weeks of waiting for feedback.
3. #3040354: Introduce entity type annotation key for indicating that an entity type is *safe* for HTTP API usage has a high-level plan.
4. #3039712: Discuss whether we want an entity access audit status report boils down to "add https://www.drupal.org/project/entity_access_audit to Drupal core" — it has value outside of API-First Drupal too. In other words: this also has a plan.
5. #3039715: Discuss whether we want an API routes audit status report is blocked on @xjm agreeing that it can be closed.
6. #3039687: Add an API read-only mode to Drupal core has been RTBC for nearly two months.
7. #3037039: Create a public API for indicating resource types should not be exposed has been RTBC for nearly one month.
8. #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs) was deprioritized. Blocked on further feedback, but the other issues in this list are deemed more important.
9. #3068275: Add status report message about JSON:API's read-only mode has been RTBC for exactly one month.

That leaves only #3039715: Discuss whether we want an API routes audit status report and #3043245: Allow marking a config schema types to be marked as "internal" (to avoid exposing them via HTTP APIs). Everything else is either closed, RTBC or already has a plan.

Thanks for the update @Wim Leers!

To clarify, nothing should be blocked on or assigned to me -- these are all framework decisions, not release management decisions and way outside my personal expertise. I was posting a whole lot of issue comments summarizing discussions that I attended, without them being my specific requests personally. :)

Probably should have done this awhile ago as well, but tagging for framework management feedback on the plan(s) as summarized in #11 and #14.

So the status on #3039687: Add an API read-only mode to Drupal core keeps confusing me; it's marked RTBC, but the request there is to wontfix it.

#17 because it needs a committer to sign off on it. At Needs review it would never get looked at. Perhaps I'm wrong there?

@xjm just opened #3087525: [meta] Determine which core entity types should not expose their data to JSON:API (for DX or security reasons). I like its direction and I think it belongs as a child of this issue (so I made it one). It's an obvious companion to #14.1.