User sensitive info is not protected properly

Bug #1337268 reported by Anastasia Kuznetsova on 2014-07-03
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mistral
High
Unassigned

Bug Description

Now:

If user passes security sensitive information to a workflow it won't be protected properly: it wil may appear in Mistral logs, it won't be encoded before transferred over the network etc.

The goal:

We need a mechanism that allows to protect sensitive user data used by workflows.

Solution ideas:

* Client doesn't need to encode sensitive data before sending it to a server. It can use HTTPS.
* Create the special section "secret" in the workflow language to let Mistral know that this date must be protected
* Create a special data type, for example class "Secret", with string representation "******" so that if something is wrapped into it we'll never see it in logs in its initial form. All variables marked in "secret" should be internally wrapped by instance of "Secret".
* Store Secret instances in encoded form in DB, decode them when fetched from DB

Syntax ideas:

--------- Section "secret" under workflow ---------

[renat: Everybody liked this idea at the PTG]

version: "2.0"

wf:
  input:
   - username
  secret:
   - keyA

  tasks:
   taskA:
     action: my_action
      publish:
       keyA: <% task(taskA).result.foobar %>
      on-success:
        - taskB

   taskB:
      action: my_action2
      input:
        arg1: <% $.keyA %>

--------- Encrypt only part of the structure ---------

[michal: we need to encrypt only keyA, and not the whole headers section]

version: "2.0"

wf:
  input:
   - username
  secret:
    - keyA

  tasks:
   taskA:
     action: std.http
      input:
       url: some.url
        headers:
         accept: text
          authorization: keyA

--------- Wrapping sensitive data using a function ---------

  tasks:
    taskA:
     action: my_action username=<% ... %> password=<% secret(...) %>

--------- Using decorator to protect from logging etc. ---------

# In this example, the argument "password" will never be logged by Mistral
# in its initial form.

from mistral_lib.secret import secret

@secret(['password'])
class MyAction(Action):

    def init(self, password):
        # do something

Changed in mistral:
status: New → Confirmed
importance: Undecided → Critical

Passwords or any other security information that can be imported in context via CLI are showing in mistral log and mistral dashboard too.

Changed in mistral:
importance: Critical → High
Changed in mistral:
milestone: none → 0.2
Changed in mistral:
milestone: 0.2 → 0.1.1
Changed in mistral:
milestone: 0.1.1 → 0.2
Changed in mistral:
assignee: nobody → Sirisha Devineni (sirisha-devineni)

Fix proposed to branch: master
Review: https://review.openstack.org/139936

Changed in mistral:
status: Confirmed → In Progress
Changed in mistral:
milestone: kilo-1 → kilo-2

Renat said on the comment:
>>>
Honestly, what mask_password() method does doesn't seem 1) obvious because there's no clear documentation on it 2) reliable enough for all situations we may have. I also would like to see some testing for that (I think it's possible to do if we apply some mocking testing etc.)
Generally, I would suggest that we create a special utility class Password with string representation like "****". And whenever we need to get a real value we will call a method get_value() or something like that.
Thoughts?
>>>

The other use case for passwords is passing them as parameters via environment __env, or workflow parameters.
We may also want to mask them in the DB: else the task context will be inspected and passwords exposed via API.

Changed in mistral:
milestone: kilo-2 → kilo-3

Change abandoned by Renat Akhmerov (<email address hidden>) on branch: master
Review: https://review.openstack.org/139936
Reason: Obsolete and almost impossible to rebase.

no longer affects: mistral/kilo

Fix proposed to branch: master
Review: https://review.openstack.org/207348

Changed in mistral:
status: Confirmed → In Progress

Here's my strong opinion about this ticket and some ideas of how we could address it.

* Any solutions based on guessing (any kinds of patterns etc.) what is secure information and what's not won't work out. Just because it's security and this is not a secure approach by definition.

Below are my ideas of how we could address it.

Client-side

An example of input data that we provide to Mistral in JSON format:
{
    "secure_key^&": {
 "key1": { "_secured": 1 },
 "key2": 2,
 "key3: 3
    }
}

So whenever we need to make something secure we just wrap a corresponding value with {"_secure": value} (we can use a different marker if needed). So the client knows that it needs to be passed to the server as a secure piece of information.

Server-side:

* We create a special data type (class SecuredData or Password) that has a string representation "*****"
* JSON SQLAlchemy type must account for this class and encode/decode info behind this class instances so that we don't keep passwords and other secure things in their original form in DB
* Every time we request a info via API we give only its string representation which is "*****"
* Every time in the server when we need to get a real value (to pass it to executor etc.) we extract a real value (i.e. using get_value() method)

summary: - Security issue: passwords are not hidden in logs
+ Security issue: user secure info is not protected properly (logs, API,
+ DB)
no longer affects: mistral/liberty
Changed in mistral:
importance: Critical → Undecided
milestone: liberty-rc1 → none
assignee: Lingxian Kong (kong) → nobody

Change abandoned by Lingxian Kong (<email address hidden>) on branch: master
Review: https://review.openstack.org/207348
Reason: Should find a generic way to fix the security problem, need more discussion with mistral team.

no longer affects: mistral/mitaka

We can work with the ossp (openstack security project) to get a ossn security note, such as https://wiki.openstack.org/wiki/OSSN/OSSN-0052

Changed in mistral:
status: New → Confirmed
importance: Undecided → High
milestone: none → pike-1
Renat Akhmerov (rakhmerov) wrote :

this is the nova attempt at fixing a similar issue: https://review.openstack.org/#/c/220622

Summer Long (slong-g) wrote :

Heat's patch for the same issue (at debug level): https://bugs.launchpad.net/heat/+bug/1664792

description: updated
summary: - Security issue: user secure info is not protected properly (logs, API,
- DB)
+ User sensitive info is not protected properly
description: updated
description: updated
Changed in mistral:
milestone: pike-1 → pike-2
Dougal Matthews (d0ugal) on 2017-04-18
Changed in mistral:
assignee: nobody → Brad P. Crochet (brad-9)
Changed in mistral:
milestone: pike-2 → pike-3
Changed in mistral:
milestone: pike-3 → queens-1
Brad P. Crochet (brad-9) on 2017-08-23
Changed in mistral:
assignee: Brad P. Crochet (brad-9) → nobody
Changed in mistral:
milestone: queens-1 → queens-3
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Related blueprints