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

Apigee Edge admin page shows password in plaintext within source #415

Closed
akac opened this issue Jun 4, 2020 · 15 comments · Fixed by #457
Closed

Apigee Edge admin page shows password in plaintext within source #415

akac opened this issue Jun 4, 2020 · 15 comments · Fixed by #457
Assignees
Labels
bug Something isn't working
Milestone

Comments

@akac
Copy link

akac commented Jun 4, 2020

Description

As a security concern, the Apigee Edge plugin config should not have the password in plain text within the source.

Steps to Reproduce

Steps to reproduce the behavior:

  1. Login as an admin
  2. Goto the Apigee Edge config
  3. View source

Actual Behavior

Password is shown in plain text within the source

Expected Behavior

No password is ever shown. It may be saved but never retrieved. Or if its retrieved its done via a GET call.

Version Info

Machine name: apigee_edge
Version: 8.x-1.6

@akac akac added the bug Something isn't working label Jun 4, 2020
@cnovak
Copy link
Collaborator

cnovak commented Jun 4, 2020

The Apigee Edge module uses the Drupal Key module to store the Edge credentials. If you install using Kickstart it will use the Configuration Key Provider since no other Key Providers are available.

Check out our FAQ on making the Edge connection more secure for instructions on how to update your site.

This issue should be more clear when installing and reading documentation.

Tasks

@akac
Copy link
Author

akac commented Jun 4, 2020

We are using the Apigee Edge: Private File - but that’s not the problem. The problem is not the key provider. The problem is that when you go to that page, if you view source of the web page - you can see password in plain text.

<div class="js-form-item form-item js-form-type-password form-type-password js-form-item-key-input-settings-password form-item-key-input-settings-password">
      <label for="edit-key-input-settings-password">Password</label>
        <input autocomplete="off" value="**<PLAIN TEXT PASSWORD HERE>**" data-drupal-selector="edit-key-input-settings-password" aria-describedby="edit-key-input-settings-password--description" type="password" id="edit-key-input-settings-password" name="key_input_settings[password]" size="60" maxlength="128" class="form-text" data-drupal-states="{&quot;visible&quot;:[{&quot;:input[name=\u0022key_input_settings[instance_type]\u0022]&quot;:{&quot;value&quot;:&quot;public&quot;}},{&quot;:input[name=\u0022key_input_settings[instance_type]\u0022]&quot;:{&quot;value&quot;:&quot;private&quot;}}],&quot;required&quot;:[{&quot;:input[name=\u0022key_input_settings[instance_type]\u0022]&quot;:{&quot;value&quot;:&quot;public&quot;}},{&quot;:input[name=\u0022key_input_settings[instance_type]\u0022]&quot;:{&quot;value&quot;:&quot;private&quot;}}]}" />

            <div id="edit-key-input-settings-password--description" class="description">
      Organization user's password that is used for authenticating with the endpoint.
    </div>
  </div>

@cnovak
Copy link
Collaborator

cnovak commented Jun 4, 2020

I also added #418 for putting a warning message in when using configuration key provider.

@cnovak
Copy link
Collaborator

cnovak commented Jun 4, 2020

The idea of using the private filesystem is that you have a location outside the Drupal webroot that it should have file permissions set so that only accessible by the system and proper unix users only. If you encrypt the password, then you will also need to have a way to decrypt the password, which means the decryption technique will have to be on the same server also. This would only be a layer of obscurity.

The Key module's project page talks about how you can use different Key providers, if you want added security you can also use a key management solution as they describe on that page.

If neither of those Key providers are what you are looking for, you can also write your own custom module to create your own Key Provider solution, check out this code to see how we wrote our own Key Provider plugins.

@akac
Copy link
Author

akac commented Jun 4, 2020

I don’t know that we’re on the same page. I think you’re focusing on how the key is stored, and I’m focusing on how the rendered web page handles the key.

None of my issues are with how the keys are stored. The key storage is not the issue. The issue is when the web page is loaded in the admin, a person could view the rendered web page source and see the password.

Nothing in the docs I’ve read tell me that how the key is stored would change that.

So my questions: Does changing the key storage option affect how the key is rendered in the HTML source code?

@cnovak
Copy link
Collaborator

cnovak commented Jun 4, 2020

Sorry @akac my fault for not reading carefully. I now correctly understand your issue. We will work on a way to fix this.

@cnovak cnovak added this to the 8.x-1.12 milestone Jun 4, 2020
@mxr576
Copy link
Contributor

mxr576 commented Jun 8, 2020

This is an admin form so only a small group of ppl should have access to it. I can only imagine this as a security issue if the page can be cached on the client side, otherwise it is as insecure as as any admin pages. If an admin user leaves his/her machine unattended and unlocked then this is only one of the places that a malicius user can leverage to do bad things. We usually install Autologout module as a workaround which logs out users after X amount of innactivity automatically.

@akac
Copy link
Author

akac commented Jun 8, 2020

Admin forms are vectors for attacks. Someone who breaks into the portal admin page would now also be able to break into Apigee. The purpose is to diminish the attack vector. Either way, this was identified by a penetration test team as a vulnerability and we were asked to raise the problem with this project.

@mxr576
Copy link
Contributor

mxr576 commented Jun 9, 2020

Suggestion: with a custom access checker in your project, disable the admin form of the module completely on production if this is a hard requirement on the project.

@cnovak cnovak modified the milestones: 8.x-1.12, 8.x-1.11 Jun 23, 2020
@minnur
Copy link
Contributor

minnur commented Jun 28, 2020

I am modifying app credentials view to use AJAX to dynamically pull credentials when pressed the eye icon.

@minnur
Copy link
Contributor

minnur commented Jul 6, 2020

The kickstarter modifies the app page with the toggle that shows/hides credentials. I think got it working with the new endpoint for credentials. Making the change in the edge module to support the same toggle.

@minnur
Copy link
Contributor

minnur commented Jul 6, 2020

@arunz6161
Copy link
Collaborator

Above PRs dont apply to this bug

@arunz6161 arunz6161 modified the milestones: 8.x-1.12, 8.x-1.13 Jul 21, 2020
minnur added a commit to minnur/apigee-edge-drupal that referenced this issue Jul 30, 2020
@minnur
Copy link
Contributor

minnur commented Jul 30, 2020

@arunz6161 I have a possible solution in this PR #457 so the idea is to make password field empty and always required. Whenever admin making changes to the connection settings they would have to provide the password. What do you think about this approach @arunz6161 @arlina-espinoza @cnovak

@cnovak
Copy link
Collaborator

cnovak commented Jul 31, 2020

I would rather follow the way SMTP module does the password where if you don't put it in it is left unchanged. This way if someone submits a change to the form they do not loose the current password. This could cause an outage on a production site.

minnur added a commit to minnur/apigee-edge-drupal that referenced this issue Jul 31, 2020
minnur added a commit to minnur/apigee-edge-drupal that referenced this issue Aug 1, 2020
minnur added a commit to minnur/apigee-edge-drupal that referenced this issue Aug 3, 2020
minnur added a commit to minnur/apigee-edge-drupal that referenced this issue Aug 3, 2020
arlina-espinoza added a commit to arlina-espinoza/apigee-edge-drupal that referenced this issue Aug 4, 2020
arlina-espinoza added a commit to arlina-espinoza/apigee-edge-drupal that referenced this issue Aug 4, 2020
arlina-espinoza added a commit that referenced this issue Aug 4, 2020
… form (#457)

* [#415] Always require password when making connection changes.

* [#415] Make sure password is set only when the field is not empty. (Similar to what SMTP module does).

* [#415] Troubleshoot failed tests. We no longer need to test password field because it is always empty.

* [#415] Refine authentication form.

* [#415] Fix WSOD when saving credentials.

* [#415] Fix tests.

* [#415] Fix code sniffer.

Co-authored-by: Arlina Espinoza <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants