Opened 4 weeks ago
Last modified 5 days ago
#64789 new task (blessed)
Security audit for API key storage on the Connectors screen
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | 7.0 | Priority: | normal |
| Severity: | normal | Version: | trunk |
| Component: | Security | Keywords: | |
| Focuses: | Cc: |
Description
The Connectors screen introduced in #64730 stores AI provider API keys using the WordPress options API. Several security concerns were discussed during the review that need to be verified.
Plaintext storage in the database
API keys for AI providers are currently stored as plaintext option values. These are sensitive credentials that, in the perfect world, should be encrypted. WordPress ships with libsodium (via sodium_compat), which could be used for authenticated encryption.
Masking bypass via /wp-admin/options.php
The current masking approach uses a filter on option_{name} to redact the key value in REST responses and on the Connectors page. However, the key is still visible in plaintext when visiting the hidden options.php screen in WP Admin. The masking filter is also temporarily removed when passing the key to the AI provider class for authentication, which may create additional windows of exposure.
Key portability concerns
As noted in review, just because a user is authorized to connect a provider on a site doesn't mean the key should be extractable. Encryption at rest (where only the site itself can decrypt for authenticated API calls) would mitigate this.
Proposed actions
- Evaluate using Sodium-based encryption for API key values stored in the options table.
- Ensure masking is applied consistently across all surfaces (REST API, options.php, wp_options database queries) or that encryption makes masking less critical.
- Document the threat model: who are we protecting against (other admins, database access, plugins reading options)?
Change History (11)
#2
@
4 weeks ago
+1 to everything that @johnbillion says. While it would be great to have encryption, the implications for Core are different than for a plugin.
The challenge is indeed that we need to rely on a secret, and some sites are configured to rotate these (which would then lead to the data no longer being decryptable).
So at the minimum, we would need a new constant for an encryption key, and that would need to be explicitly documented as to never rotate its value. Certainly not trivial to pull off at the scale of WordPress, and certainly not this late in the release cycle.
It's also worth noting that, if a malicious plugin gets access to the site, none of it helps. So I think it's fair to proceed with the current baseline of no encryption, and explore support for external two-way encryption systems in the future.
The only thing that I think we should fix here is the display in plaintext in wp-admin/options.php. That's of course not encryption, but a small tweak we should add in for parity with how these values are displayed elsewhere.
This ticket was mentioned in Slack in #core-passwords by johnbillion. View the logs.
4 weeks ago
#4
follow-up:
↓ 5
@
4 weeks ago
The only thing that I think we should fix here is the display in plaintext in wp-admin/options.php. That's of course not encryption, but a small tweak we should add in for parity with how these values are displayed elsewhere.
@jorgefilipecosta is working on the fix in https://github.com/WordPress/wordpress-develop/pull/11158. He even created a dedicated ticket #64793 for that, as it looks like we will move this one to the 7.1 release.
#5
in reply to:
↑ 4
@
4 weeks ago
Replying to gziolo:
The only thing that I think we should fix here is the display in plaintext in wp-admin/options.php. That's of course not encryption, but a small tweak we should add in for parity with how these values are displayed elsewhere.
@jorgefilipecosta is working on the fix in https://github.com/WordPress/wordpress-develop/pull/11158. He even created a dedicated ticket #64793 for that, as it looks like we will move this one to the 7.1 release.
An alternative to this would be to store all the keys in a single connectors_api_keys option, which would result in them getting stored as a serialized array, and thus appear as SERIALIZED DATA on the options.php screen.
This ticket was mentioned in Slack in #core-ai by justlevine. View the logs.
3 weeks ago
#7
@
3 weeks ago
This came up the other day in a discussion about the Two Factor plugin and how we'd store a key for encrypting TOTP tokens in the database. It got me thinking, and I started working on a potential option to add a get_secret() function (and related utility) to WP.
I spun this up as a plugin (https://github.com/ericmann/secrets-manager/) and submitted it to the .org repo for review. It's still a first pass and needs some more attention, but provides similar utility to Kubernetes' secrets system, just in WordPress. Secrets are automatically encrypted either with a fixed key set in wp-config.php or using the Site Kit approach of concatenating existing salts from the config. It also has utility for rotation and WP CLI integration to make it more usable.
There's currently no way to store a two-way encrypted value in WordPress without tying it either to one of the built-in secret keys in wp-config.php or tying it to a new secret. Tying it to a key in this file risks data loss if it gets rotated or a user migrates/exports/copies their data to another environment that uses different keys.
Felix has written about how the Google Site Kit plugin encrypts API keys, but it still relies on a secret key in wp-config.php.
There's nothing that can be done about this in time for 7.0. In the past I've floated the idea of a two-way encryption API in WordPress which uses no encryption by default but allows site owners and hosts to connect it to an environment variable for local two-way encryption, a KMS, a secret store, or any other opt-in means of encryption that reduces the opportunity for data loss.
CC @kasparsd @snicco @rmccue @flixos90