JSONFormatter logs auth_token

Bug #1866705 reported by Ben Nemec
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
oslo.context
Fix Released
High
Unassigned
oslo.log
Invalid
High
Unassigned

Bug Description

As reported here: https://review.opendev.org/#/c/605342/ so I'm not sure whether this needs to be private. I figure we can always make it public later but we can't go the other way.

It looks like we took steps to sanitize the token from the JSON output[0], but we missed a nested instance in the context auth_token_info. It sounds like we probably need to start stripping some of the context out in the JSONFormatter anyway because logging the full catalog in every message is unreasonable. Need to figure out if we just remove the catalog and auth_token or if we can do away with the whole auth_token_info field.

0: https://opendev.org/openstack/oslo.context/src/branch/master/oslo_context/context.py#L368

Revision history for this message
Ben Nemec (bnemec) wrote :

We should also update oslo.context to strip the nested auth_token field from the dict it returns. Strictly speaking, the security side of this could be taken care of by that alone, but I still think we should make the change in oslo.log too as both a belt-and-suspenders thing and because of the semi-related catalog issue.

Changed in oslo.context:
status: New → Triaged
importance: Undecided → High
Revision history for this message
Ben Nemec (bnemec) wrote :

Looking a bit deeper, it seems that the problematic field is a Heat-specific thing. We can't really fix that in oslo.context, but we may still need some way to filter out the field in oslo.log.

Changed in oslo.context:
status: Triaged → Invalid
Ben Nemec (bnemec)
Changed in oslo.context:
status: Invalid → Triaged
Revision history for this message
Ben Nemec (bnemec) wrote :

Okay, changing my mind again. It turns out that auth_token_info is a common pattern across a number of projects, so IMHO it would be appropriate to handle it in oslo.context.

information type: Private Security → Public Security
Revision history for this message
Ben Nemec (bnemec) wrote :

Moving to public since I pushed the patch to oslo.context (and as noted above there was already public discussion of the bug).

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo.context (master)

Reviewed: https://review.opendev.org/712146
Committed: https://git.openstack.org/cgit/openstack/oslo.context/commit/?id=1dd72d1d209e699efc360ff99a20166aac831939
Submitter: Zuul
Branch: master

commit 1dd72d1d209e699efc360ff99a20166aac831939
Author: Ben Nemec <email address hidden>
Date: Tue Mar 10 17:55:16 2020 +0000

    Filter out auth_token_info from logging values

    auth_token_info is a common field that subclasses of RequestContext
    add. It contains things like the token itself and the entire catalog,
    both of which are undesirable to log. The token is a security concern
    and the catalog is huge, which bloats the logs an unacceptable amount.

    This change removes the auth_token_info key from the logging dict
    that we return to the log formatter, which eliminates both problems.

    Change-Id: If5ebaa3c1859d32cd05f51defe173fc625b21af5
    Closes-Bug: 1866705

Changed in oslo.context:
status: Triaged → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to oslo.context (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/713738

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo.context (stable/train)

Reviewed: https://review.opendev.org/713738
Committed: https://git.openstack.org/cgit/openstack/oslo.context/commit/?id=ab17aef5735bcb242889f610d2165d46b380f7cc
Submitter: Zuul
Branch: stable/train

commit ab17aef5735bcb242889f610d2165d46b380f7cc
Author: Ben Nemec <email address hidden>
Date: Tue Mar 10 17:55:16 2020 +0000

    Filter out auth_token_info from logging values

    auth_token_info is a common field that subclasses of RequestContext
    add. It contains things like the token itself and the entire catalog,
    both of which are undesirable to log. The token is a security concern
    and the catalog is huge, which bloats the logs an unacceptable amount.

    This change removes the auth_token_info key from the logging dict
    that we return to the log formatter, which eliminates both problems.

    Change-Id: If5ebaa3c1859d32cd05f51defe173fc625b21af5
    Closes-Bug: 1866705
    (cherry picked from commit 1dd72d1d209e699efc360ff99a20166aac831939)

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to oslo.context (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/714717

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/oslo.context 3.0.1

This issue was fixed in the openstack/oslo.context 3.0.1 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to oslo.context (stable/stein)

Reviewed: https://review.opendev.org/714717
Committed: https://git.openstack.org/cgit/openstack/oslo.context/commit/?id=445de77bf7e6c57b3b4bd4097859075920b0b042
Submitter: Zuul
Branch: stable/stein

commit 445de77bf7e6c57b3b4bd4097859075920b0b042
Author: Ben Nemec <email address hidden>
Date: Tue Mar 10 17:55:16 2020 +0000

    Filter out auth_token_info from logging values

    auth_token_info is a common field that subclasses of RequestContext
    add. It contains things like the token itself and the entire catalog,
    both of which are undesirable to log. The token is a security concern
    and the catalog is huge, which bloats the logs an unacceptable amount.

    This change removes the auth_token_info key from the logging dict
    that we return to the log formatter, which eliminates both problems.

    Change-Id: If5ebaa3c1859d32cd05f51defe173fc625b21af5
    Closes-Bug: 1866705
    (cherry picked from commit 1dd72d1d209e699efc360ff99a20166aac831939)
    (cherry picked from commit ab17aef5735bcb242889f610d2165d46b380f7cc)

tags: added: in-stable-stein
Revision history for this message
Ben Nemec (bnemec) wrote :

This was fixed on the oslo.context side. We don't need anything in oslo.log.

Changed in oslo.log:
status: Triaged → Invalid
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.