Insecure credential storage used by drupal_cms_ai recipe as default

I was talking to @japerry about this the other day; he's in the middle of re-architecting the Key module, and I wanted to make sure that Drupal CMS would have some kind of avenue to pre-configure Key in such a way that you could automatically get secure credential storage set up for you, right out of the box, most likely as a part of the drupal_cms_authentication recipe. @pameeela supports the idea, but that is probably blocked on a new major version of Key.

But maybe it's not? I'm not sure. If we can set up encrypted key storage automatically in a recipe, I'd say that's worth adding sooner rather than later. Do you have a specific approach in mind?

Drupal CMS could take a more opinionated stance and require a private file system to be configured, which might serve as one viable default option for storing encryption keys.

We were thinking along these lines as well. A private files directory would be a reasonable default for secure(-ish) key storage, and something Drupal CMS could probably set up.

@mxr576, @longwave, and I discussed this in Slack.

What I think we can try, for the moment, is to update Drupal CMS Helper so that it tries to create ../private, and point the settings to it, unless private files are otherwise configured -- as early as possible, by responding to KernelEvents::REQUEST.

Assigning to myself to see if we can prove the concept.

Okay, that's my proof of concept for what we could do in Drupal CMS. Review!

Took this a step further and created a config action (which we would probably move to the Key module) that exposes a config action to create a key that is stored in a file. And I adapted the AI recipe to use it, right off the bat.

The downside is that this requires Drupal 11.3. We can patch core for the time being, but it means this won't get into 1.2.x in any event. 2.x only, which will require 11.3 or later.

Sounds good to me but we probably can just wait until 11.3 is out? No one is building with 2.x now anyway, so I don't think we need to worry about patching core to get it committed ASAP.

I'm happy with this; I've asked the security team to review it, since it's definitely not something we should commit to Drupal CMS without their explicit sign-off.

Opened #3561380: Expose certain methods of the Key entity as config actions to port the config actions into Key so they can see wider use.

I think you're right. Key needs to be able to store the key file. It has infrastructure for that, but we need to shim in a settable file-based storage and expose Key::setKeyValue() as a config action. I think we can do both.

Okay, well, this took a few interesting turns.

I realized that the original approach was not going to work, because it means that you can never get rid of the shim, as you now rely on it to make all private files work. That goes against the goal of Drupal CMS Helper, which is...to render itself unnecessary.

Better would be for the Key module to be able to handle this intelligently -- write the key to a file if possible. If the key file is using the private:// stream wrapper, that's fine; if it's not configured, fall back to trying to create a private directory on the fly. If that doesn't work, fall back to storing the key in config.

Having this encapsulated in a config action means that Drupal CMS Helper does not need to maintain permanent runtime code for this. Good.

I think this the right balance of convenience and security. I need to write some totally new tests but I feel pretty good about this approach.

This has kept evolving because I keep having better and better ideas for it.

Right now, this actually sets up full AES at-rest encryption for credentials, making them safe to store in config. (The master encryption key is the only one kept in a file.) It does all this transparently. We could merge this as-is, but it took a lot of shims and workarounds to work. I would rather get those workarounds into the relevant modules:

• The AI recipe no longer needs two separate inputs for API keys. This allows me to remove some awkwardness, but it means there needs to be a little bit of refactoring in the AI module's setupAiProvider config action -- which is itself a shim -- so that it can dynamically decide which set of config it should alter. I intend to spend a little time tomorrow showing this to Marcus, in the hopes that he'll merge it into the AI module so I can remove the workaround.
• I had to create a new key provider that transparently encrypts and decrypts a key handled by another provider. In other words, a very simple decorator. I think this key provider should absolutely be in the Encrypt module, as it makes it much easier to securely store keys in config, state, files, or any other mundane location.
• I think the "automatically generate an encryption key and try to save it to a file outside the web root" functionality should be added directly to the Key module. It's straightforward enough, and in the event that the key cannot be saved to a file, the Key module now has an (unreleased) provider that stores the value in state. That, to me, is a reasonable fallback storage spot, because it's internal to the site and never deployed or committed to Git. (Indeed, core stores a private_key hash in state as well, which it uses for its own purposes.) Doing this means it's possible for recipes to take advantage of strong encryption with minimal effort, and that can only be a good thing.

These all need relevant follow-ups filed in the appropriate contrib modules, plus maintainer buy-in.

In other words, these advancements really take away the pain of getting secure credential storage set up. We should contribute this back. We could keep it all in Drupal CMS, but that's not really the Drupal way, now is it? :)

@pameeela approved this in a Slack DM with me. I'm happy with this approach, and I have buy-in from a maintainer of the Key module to port this feature into Key (there is no need to do anything to Encrypt or Real AES) in follow-up issues, hopefully with the goal of getting this released in the next 1.x version of Key. Security review is unnecessary, as this no longer alters registered stream wrappers over the course of the request. It's about as cleaned-up as it can be for now.

So...auto-merged into 2.x and cherry-picked to 2.0.x. See you in the follow-ups.

I ended up reverting this because I described it to a non-Drupalist friend of mine who is a cryptography nerd, and whose opinion I greatly respect.

He felt that this implementation -- which necessitates storing the encryption key in plaintext, and locally -- is not secure. And he's not wrong; storing an encryption key in plaintext outside of the web root, or in the database, where it could be accessed if either filesystem or database backups are compromised (or, indeed, if the web server is misconfigured), is inherently risky.

But at the same time, the need here is legitimate. It's hard to set up a proper key management and encryption stack in Drupal, if you're not familiar with it. We really should have kind of reasonably secure, zero- or minimal-configuration solution in Drupal CMS, for which the target audience can never be expected to bother setting up proper secret management in Drupal.

And yet, that does run into the reality that no zero-configuration solution is going to be fully secure, and any system is only as secure as its least secure element. So storing an encryption key locally in plaintext could be considered more security theater than effective security measure. Security theater is arguably worse than no security, because of the false sense of security it imparts, by definition.

Striking the right balance is very hard, and frankly it's beyond my pay grade. I can implement stuff, but actually validating a secure system is what the security team is for. We should not be implementing a baseline security improvement without their guidance and review.

So, out of an abundance of caution, I reverted this (and its sister commit in #3561793: Move credential encryption into Drupal CMS Authentication) for now. I think we need to make a clear proposal first, and workshop it with the security team -- ideally multiple members of it -- until we find something that helps Drupal CMS's end users get better security for credentials and meets Drupal's security standards.

@phenaproxima this is a great issue and all the thoughts that are going into it, are outstanding.

Regarding your last comment in #28 is touching on an issue that's real, but the question is how relevant that is for the target audience. For a fully hardened system, i.e. keep your keys secret, even if someone got access to your server, is only solvable with hardware components that keep your secrets. And not even all enterprises are leveraging that.

Look at it the other way round: if your PHP process needs access to the secrets, you've got to store the key to them somewhere the PHP process can access that key. Doing that in a way that somebody with access to the server can't access that key is impossible by definition. Only if the secrets get stored in an HSM (Hardware Security Module) you get a chance to achieve the full security. But that's not for the audience of Drupal CMS. At least not right at the beginning of their journey.

Discussed variously and asynchronously with @xjm (security team member) and @japerry (Key maintainer). Here's what I propose we do:

• Create a completely new key provider, called encrypted.
• When a credential is created using this key provider, it does the following:
  1. Create a deterministic, but hard-to-guess, name for the encryption key -- most likely an HMAC of the key entity's UUID or machine name, and the site's hash salt. This bit is optional, to be honest; it's just obscurity for its own sake, which may help if the list of keys is compromised, but in all likelihood that would also mean the keys themselves were also compromised.
  2. Create a directory called .keychain, one level above the web root, if it doesn't exist already.
  3. Set .keychain's permissions to 0700.
  4. Generate the encryption key with \sodium_crypto_secretbox_keygen().
  5. Store the key in a PHP file, named .keychain/$NAME_OF_KEY.php, which contains <?php return 'PLAINTEXT OF ENCRYPTION KEY';.
  6. Set the key's permissions to 0600.

  If .keychain cannot be created, or the file cannot be written, the plaintext encryption key is stored in the database, in a new keychain key-value collection, identified by the generated name of the encryption key. Because this is a less secure fallback, a warning is logged.

• The original credential is encrypted by sodium_crypto_secretbox() using the key and a nonce generated with random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES). The encrypted credential, and the nonce, are stored in config, in the database. Since the credential is encrypted, it's portable and can be safely kept in exported config. According to Sodium's documentation, the nonce is not considered confidential.

When the credential is needed, the encryption key is either read from the file (if the file exists) or from the keychain key-value store if the file doesn't exist. The key is then used, along with the stored nonce, to decrypt the credential with sodium_crypto_secretbox_open().

If the credential is changed, the nonce is changed. The encryption key doesn't need to be, but maybe we can offer that as an option for the truly paranoid.

This provides no defense against a compromised server; if the encryption keys can be extracted from the filesystem or the database, the credentials are compromised, full stop. But that's also true of many things, like SSH keys. As I understand it, Drupal's threat model concerns ways that Drupal itself can be used as an attack vector, and from that perspective, I don't see how this scheme would be problematic. Writing the encryption keys as PHP files means that even if you're able to access a key file over the web, it will produce no output (the same protection you get with settings.php, which contains the security-sensitive hash salt). So I think the biggest risk would be posed by path traversal vulnerabilities. To help mitigate these things, we could have a simple button in the UI which, when clicked, re-encrypts all encrypted credentials using freshly generated unique keys.

This would be a zero-configuration option that, although certainly not perfect, is several levels better than what the default in Key is now, which is completely unencrypted plaintext storage of credentials in exported config.

Next step: security team validation and sign-off. I'd ideally like two members of the security team to weigh in and approve this (or not).

Linking #3193591: Add a secrets store to Drupal core as related issue requesting secure storage be included in core itself instead of the contrib ecosystem.

I definitely like Symfony's approach, but it's aimed squarely at developers. One of the constraints we face is that we can never, ever expect Drupal CMS's target audience to understand this stuff or even care about it. It needs to be completely transparent to them. This is not the kind of thing we can tout, because our intended end users will greet it with a yawn.

From a security perspective, I definitely think we need something, but it's just not entirely clear what the best option is, given the technical configuration requirements of better solutions.

By "aimed squarely at developers", I just meant we can't expect end users of Drupal CMS to touch the command line or fire up a text editor to edit .env files. :) But I can definitely be persuaded to switch to a sealed-box scheme! If it doesn't really move the needle in terms of securing the encryption key, but does assist the deployment and DevOps processes, that's a strong point in its favor.

I don't think drupal_cms_helper is the right place for this to live. Maybe it's possible to add a different storage plugin for the key module, or possibly this could be part of moving parts of key module into core. Even if it's done correctly, it's very complex and sensitive code to maintain in a project that is only supposed to be a recipe.

I am certainly open to doing it in Key; if @rlhawk or @japerry agree, I can move it. I have no strong opinion about where it lives.

@catch, I opened a sandbox project to put this into its own module (potentially): https://www.drupal.org/sandbox/phenaproxima/3562833.

@mxr576, @japerry, and yourself have commit access. Let's see what happens.

This looks pretty close to ready, but it is blocked on a bugfix (and new release) of Key. We cannot patch dependencies.

Self-assigning to go over this with @pameeela -- she's out this week so it'll probably be next week.

I just gave this a quick test and I think I like it. In the context of DDEV, it does nicely set up everything for you transparently. You don't have to think about it. And there's a way to import and export the encryption keys.

Flaws but not dealbreakers/blockers:

• The import/export stuff is important, but it's not documented or explained at all and that will make it hard for people, even technical people, to use and understand. It's completely disconnected from the key UI, which is not helpful. This probably needs a documentation page, in our user guide (docs directory) which explains what's going on and what to do when you want to move your site to hosting, and how to handle the keys in that situation. We could add that in a follow-up.
• The UX could use some refinement (the "key ID" is completely opaque, it's not remotely clear where a key is used or why it's important), but I don't think that is a blocker because this is not a UI we would expect our target audience (marketers) to even think about, much less look at.

I will try to walk through this with @pameeela and seek her approval. The documentation is probably the biggest sticking point, and the UX needs some love, but this would not be the only dependency that could benefit from UX improvements. The fact that this is a very technical piece of the puzzle means that the UX issues are probably somewhat less problematic than they would otherwise be.

But from my perspective as Drupal CMS's Architecture Lead, it passes muster.

So storing an encryption key locally in plaintext could be considered more security theater than effective security measure. Security theater is arguably worse than no security, because of the false sense of security it imparts, by definition.

The point about security theater feels like the right north star. The current approach creates a false sense of safety because credentials live in config. We can reduce that risk, but we cannot replace it with a perfect solution.

I strongly suggest the drupal_cms team looks at those statements closely.

Easy Encryption fallback of storing the decryption key along side the data to be protected (in the same database as the encrypted keys) is still theater, the warnings are almost non-existent, and would be very easy to ignore. Furthermore there is no ongoing warning when creating new keys to indicate that they are being stored insecurely (at least with RC5).

Easy Encryption goal of storing the encryption keys on disk is reasonable, the keys are not necessarily stored with the data they are protecting (API keys in database) requiring a dual compromise (there are of course cases where this wouldn't be the case, however at the lowest level a successfull breach needs access to both keys and the protected data).

Drupal CMS and Easy Encryption can insist that the folder exists and is writable as part of the install requirements or fail to allow the install to proceed. I strongly recommend doing so, otherwise your just storing the key insecurely in the database with added complexity and less transparency regarding insecurity, especially when Easy Encryption currently asserts that it will securely store the data.

A large portion of new deployments may indeed be made more secure by the easy encryption module being able to create the keys on disk, it is however when that fails that the false impressions become a significant concern.

Drupal CMS and Easy Encryption can insist that the folder exists and is writable as part of the install requirements or fail to allow the install to proceed.

How?

Easy Encryption, in this scheme, is installed by a recipe. Recipes have no way to check preconditions before being applied. (There is a core issue to work on this, but it has not seen much action and I've got a lot of other things I need to work on.) Even if they did, I'm not sure we would want to block applying the AI recipe on the lack of a writable filesystem, because a lot of hosting platforms don't give you a writable filesystem.

This MR, as implemented, improves the status quo -- which, remember, is storing sensitive credentials not just in plain text, but in plain text that gets exported in configuration and into (presumably) source control! That's the worst possible situation. Surely, storing the encryption keys in a non-exportable and non-exposed database table (state) is better than that. It is not the best possible security, and calling it "secure" is probably misleading, but from a nuts-and-bolts perspective it is unambiguously an improvement.

I do think that it is a great idea to flag a warning in the status report if the database storage is being used. If Easy Encryption is able to transfer the encryption key automatically from the database into the filesystem, or have some kind of button/Drush command to do it, even better! I would not block on that, though; I would instead call it a high-priority follow-up, and I have complete confidence in @mxr576's ability and willingness to follow through.

I also completely agree that the messaging could be improved and made clearer that "better than plaintext" is not the same as "secure". That, too, feels like a follow-up rather than a blocker.

Easy Encryption, in my view, is a deeply pragmatic module. It recognizes that Drupal CMS's target audience of marketers does not, by and large, give a single 💩 about security until they get burned. Given that sad reality, it does what it can realistically do to help the cause. The fact that it can, in certain situations, be considered "security theater" is less important, IMHO, than the fact that it is an improvement over having no protection at all.

Shorted answers to avoid this becoming a novel:

Better security doesn't necessarily mean acceptable security is how I suggest looking at my statements. You do bring up a good point regarding config exports for advance site owners utilizing config management (not likely a drupal_cms target user IMO). Any hard statistical on Database breaches vs Config export breaches? Perhaps I'm vastly miscalculating the proportions of each breach type.

Drupal Core already requires some form of writable filesystem to even function, some form of persistent storage to be practical for normal usage. Adding another persistent storage doesn't seem to be a large ask.

There is also a support workload to consider for all sites that fail to export keys that were silently setup for them. Not a big issue for API keys, significant problem if a module like field_encrypt is added where it can result in actual data loss. Sometimes the best action is to guide users to setup (and backup the important information) rather than making it magically work for them.

How?

Drupal CMS installer pages looking at the recipes selected, decorating the Module Installer services, aliasing a core class to a custom version to handle the changes are thoughts that come to mind. I leave the exact specifics to someone who works with the code base daily on where they would want to solve such an issue.

I would not block on that, though; I would instead call it a high-priority follow-up, and I have all confidence in @mxr576's ability and willingness to follow through.

I have no doubt he will either. I do (as I have in the past) question what that says about drupal_cms acceptance standards, considering drupal_cms intended to be new site owners first look at the Drupal ecosystem I would hope it is evaluating applications using a high evaluation standard, expecting polish and concerns to be sorted before they are added rather than in followup issues.

Better security doesn't necessarily mean acceptable security

"Acceptable" is a completely subjective judgment call that depends on who you are, what you're trying to do, and what your threat model is. Drupal CMS is not aimed at developers or security-conscious people. It is aimed at people who don't know or care anything about security. So in this case, I would argue that better security is an entirely reasonable thing to aim for.

For the record, I asked the security team to explicitly review Easy Encryption's approach before we implemented anything. I waited for a month, then was told in Slack (and only after DMing a team member) that the security team does not review proposed module designs, but only reacts to actual security problems as they are discovered.

If there was a workable definition of "acceptable" security, I would imagine that its criteria and patterns would be documented somewhere for the benefit of people who want to write security improvement modules.

There is also a support workload to consider for all sites that fail to export keys that were silently setup for them.

Agreed; I think this piece of it needs documentation, and that is absolutely follow-up material.

significant problem if a module like field_encrypt is added where it can result in actual data loss

That's a good point, and it should be answered before we commit this. @mxr576, how well would Easy Encryption play with something like Field Encrypt? Is there a risk of data loss? If so, what could we do to mitigate it? (From Drupal CMS's perspective, adding a conflict with Field Encrypt in composer.json would be an acceptable workaround in the short term.)

Drupal CMS installer pages looking at the recipes selected, decorating the Module Installer services, aliasing a core class to a custom version to handle the changes are thoughts that come to mind.

These aren't realistic options in a recipe-based system, I'm afraid. Drupal CMS's installer does not apply the AI recipe in the first place (and it never has), and we are not going to add module installation guardrails to our polyfill module.

I do (as I have in the past) question what that says about drupal_cms acceptance standards, considering drupal_cms intended to be new site owners first look at the Drupal ecosystem I would hope it is evaluating applications using a high evaluation standard

We don't have a formal set of standards for adding dependencies. What it largely comes down to, in practice, is what our product leadership feels will be most beneficial for our target users, and whether we can easily work with the maintainers. @pameeela has yet to weigh in here from the product perspective, but @mxr576 has proven to be a solid, responsible maintainer.

It's certainly a much less formal approach than core takes, but that's how Drupal CMS does things -- I think it's one of our strengths, but your mileage may of course vary. If that's uncomfortable for you, it's perfectly fine if you choose not to use or recommend Drupal CMS.

"Acceptable" is a completely subjective judgment call that

I'm a bit surprised this doesn't have a clear CWE category, though that could be because key storage is usually the responsibility of the server not the software itself, or if it is the software (like a KMS) usually doesn't store the data in plainttext. Unfortunately for drupal_cms, it would be the party storing the data not the the site owner.

https://www.drupal.org/project/drupal/issues/590656 is somewhat relevant, The database backup contained all the information necessary to bypass a security protection, similar would be happening here if the keys are stored in the same location as the data they protect. It is why hash_salt is in settings.php and not the database.

Drupal CMS is not aimed at developers or security-conscious people. It is aimed at people who don't know or care anything about security.

Hence drupal_cms greater responsibility to avoid storing the plaintext keys in the same location as the ciphertext they protect. I do agree drupal_cms can be argued to have a responsibility to protect the export files at the same time, that almost is its own distinct issue, separate from storage on the running site (adding passphrase based encryption to the exports can protect export files)

from Drupal CMS's perspective, adding a conflict with Field Encrypt in composer.json would be an acceptable workaround in the short term

The drupal/encrypt module is what provides most of the work there as the framework, Field Encrypt is just an easy to see example of where loss of keys is data loss.

For the record, I asked the security team to explicitly review Easy Encryption's approach before we implemented anything. I waited for a month, then was told in Slack (and only after DMing a team member) that the security team does not review proposed module designs, but only reacts to actual security problems as they are discovered.

Indeed, I saw the original request and delayed any comments at that time. Hopefully the DA will look into assisting Drupal Core and drupal_cms receive security advice before code is deployed to production sites not after.

Considering how often I disagree with the Core devs and the Security team I almost didn't speak up, I was hoping someone other than me would be the one to provide the architectural review.

I think that's an excellent start; ideally we could also link to documentation explaining how to move the key. If Drupal CMS adds this module and the relevant documentation, then linking to our docs temporarily would be acceptable.

By the way, I spoke with @pameeela yesterday and told her my thoughts on Easy Encryption. My summary was basically what I've been saying: useful module, makes things a little more secure out-of-the-box, the UI is rough around the edges, and aspects of it need documentation. But I endorsed it. Pam wants to test it out personally (hopefully today, her time) before signing off.

it recognizes that Drupal CMS's target audience of marketers does not, by and large, give a single 💩 about security until they get burned.

I view two extreams on how that can be treated, the users don't care so neither does the software, or the user doesn't care so the software operates under the assumption it has to protect itself from the user.

As is this is probably somewhere in between, when local storage exists its leaning strongly towards protecting the software from the users (which I greatly respect as a net positive) and when no writable storage exists it starts to lean towards leading to situations where the user burn themselves.

Developers often choose default values that leave the product as open and easy to use as possible out-of-the-box, under the assumption that the administrator can (or should) change the default value. However, this ease-of-use comes at a cost when the default is insecure and the administrator does not change it.

CWE-1188: Initialization of a Resource with an Insecure Default

This part of the discussion is as much about protecting the site owner and their users, as it is about protecting drupal_cms and Drupal Core's reputation. Someday something is going to happen and an insecure default will be questioned.

do you think it would make sense to open a follow-up to extend the fallback strategy so that, if writing outside the docroot fails, we first attempt to store encryption keys in the private filesystem before falling back to state?

Similar risk to storing them in the .easy_encryption folder with the previously noted risk of private:// under public:// or otherwise having the path be served by the server which may not execute the PHP interpreter. Site owners very much need to keep private:// safe and any faults in doing so would be out of scope to easy_encryption.

I was going to put a comment in the original post that easy_encryption could consider gathering data for well-known hosts and any that do have a writable persistent storage the module could determine the provider and utilize the correct path (obviously a follow-up not needed for this in its root form).

Going back to #32 regarding <?php protection of the files, that assumption starts to fail when one includes streamWrappers. Still a good protection, just I'm use to working with streamWrappers where there is no interpreter and the file will be served raw (though private:// isn't public so no real increase in risk).

As an aside:
I recall the posts that Startshot would be a polished out of the box experience and that it wouldn't be delayed by core, I don't necessarily consider "less than ideal security" to be polished. I look at this very much as "core may not have it, that makes it this projects responsibility to implement and later migrate into core" in the spirit of not being a fork, yet at the same time still being a parallel version of Drupal. (To be fair, i do believe Starshot/drupal_cms had an unacceptably short window to develop itself and that contributes to situations like this where something needs to be doable yet isn't as the frameworks were not created for them in the beginning)

Many thanks for all of the thought and work that has already gone into this! I agree with everyone that this is an important issue and the current state is far from ideal.

My main concern about this approach is that it seems fairly complicated to understand. That is greatly mitigated by the recipe doing the setup, but I don't know how all of the config here fits together and I'd like to see it documented. If the IS can be updated to reflect the current state of the MR, and describe how the pieces fit together, that would be really helpful.

I do agree that most users don't care about this, and also that we have a responsibility to provide some elevated concern as a result. But I'm thinking about a few scenarios where users need to add a key after applying the recipe, for example, if they chose amazee.ai for the trial and then signed up for an account. I assume they would be expected to set this up themselves, and is it reasonable to expect they would be able to figure that out? At least we could provide documentation for how to do it, even if they are likely to do it the easy way.

@mxr576 sorry, I have had trouble finding the proper time to respond! Thank you very much for your thorough response.

Could your please further elaborate on this statement, fairly complicated from the point of view of who (which persona) and why?

So this is what I see after applying the AI recipe and selecting Anthropic:

I think I can make sense of what is going on here, but it's not very obvious or intuitive, and it's also not obvious that you have to take action in order for this to work across environments. Not that we would be able to convey this in the UI; but we should at least have documentation about it in the user guide. I would be OK to do that as a follow up but I would consider it a release blocker and assume this will only go into 2.1.x?

Yes, this is a 2.1.x-only feature.

I think the “it” in the sentence refers to a guide on how to move Drupal CMS–based sites between hosting environments

Yes, exactly -- we don't need to explain the ins and outs of everything, just what the steps would be for this to work.

Merged into 2.x and cherry-picked to 2.1.x.

Leaving at "needs work" for documentation follow-up.

Per @pameeela's request, changing status but we still need a docs follow-up here.