[OSSA 2015-007] S3token incorrect condition expression for ssl_insecure (CVE-2015-1852)

[Brant Knudson]

Remember bug 1353315? The auth_token middleware would not verify the server cert when insecure=false in api-paste.ini because it passes the value as a string rather than a Boolean. Turns out he s3_token middleware has the same code.

http://git.openstack.org/cgit/openstack/keystonemiddleware/tree/keystonemiddleware/s3_token.py#n119

 insecure = conf.get('insecure', False)

 if insecure:
     self._verify = False

conf is a dict of strings, so if you set insecure=false, then insecure here gets set to "false", which evaluates to True since it's not a zero-length string.

[Morgan Fainberg (mdrnstm)]

Yep. Good catch, classified this as appropriate.

[Jamie Lennox (jamielennox)]

Appears that s3token middleware doesn't support using oslo.config at all so the patch for auth_token is not useful.

[Grant Murphy (gmurphy)]

Since we issued an advisory for bug 1353315 we will probably do the same for this one. Adding OSSA task to the bug and marking incomplete pending discussion with VMT.

[Brant Knudson (blk-u)]

Jamie's patch didn't apply for me... also, seems like overkill.

[Morgan Fainberg (mdrnstm)]

Added coresec here since they should have been added when i did the cleanup of security review permissions.

[Tristan Cacqueray (tristan-cacqueray)]

So the middleware is still present with python-keystoneclient, so I guess it should also be fixed there.

Here is a first impact description draft:

Title: S3Token TLS cert verification option not honoured in paste configs
Reporter: Brant Knudson (IBM)
Products: keystonemiddleware
Affects: versions up to 1.5.0 (keystonemiddleware),
         versions up to 0.11.2 (python-keystoneclient)

Description:
Brant Knudson from IBM reported a vulnerability in keystonemiddleware
(formerly shipped as python-keystoneclient). When the 'insecure' option
is set in a S3Token paste configuration file it is effectively ignored,
regardless of its value. As a result certificate verification will be
disabled, leaving TLS connections open to MITM attacks. All versions of
s3_token middleware with TLS settings configured via a paste.ini file are
affected by this flaw.

[Brant Knudson (blk-u)]

comments on the impact description in comment 7 :

in Title, change "honoured" to "honored" for US spelling. Also, can remove "in paste configs", since the S3Token only supports paste config (unlike auth_token middleware that supports config file, too).

Seems like Products should also have python-keystoneclient ?

In the Description, final sentence should be "All versions of
s3_token middleware with TLS settings configured are
affected by this flaw." (remove the section about paste.ini since s3token can only be configured via paste.ini).

[Brant Knudson (blk-u)]

The code is the same in keystoneclient.middleware: http://git.openstack.org/cgit/openstack/python-keystoneclient/tree/keystoneclient/middleware/s3_token.py#n118 , so the problem needs to be fixed there too.

Not sure if it's worth it to bother doing the review in the issue rather than in gerrit... the fix should be the same.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks for the quick feedback. Here is the updated impact description draft:

Title: S3Token TLS cert verification option not honored
Reporter: Brant Knudson (IBM)
Products: keystonemiddleware, python-keystoneclient
Affects: versions up to 1.5.0 (keystonemiddleware),
         versions up to 0.11.2 (python-keystoneclient)

Description:
Brant Knudson from IBM reported a vulnerability in keystonemiddleware
(formerly shipped as python-keystoneclient). When the 'insecure' option
is set in a S3Token paste configuration file it is effectively ignored,
regardless of its value. As a result certificate verification will be
disabled, leaving TLS connections open to MITM attacks. All versions of
s3_token middleware with TLS settings configured are affected by this
flaw.

[Brant Knudson (blk-u)]

The impact description in comment 10 looks good to me.

[Morgan Fainberg (mdrnstm)]

I concur, impact description looks good.

[Thierry Carrez (ttx)]

impact desc +1

[Dolph Mathews (dolph)]

+1 for the patch to keystonemiddleware in comment #4 and the impact description in #10.

[Guang Yee (guang-yee)]

+1 for both the patch and impact desc.

For the tests, 'someweirdvalue' is interpreted as False sound too lenient. I think we should tighten it up a bit in the future.

btw, I haven't came across a deployment where the insecure flag, if set, is meant to "enable" cert validation. Its always the other way around. If "insecure" is set, its most likely meant to turn off cert validate regardless of its value. So the impact may not as bad as we think.

[Tristan Cacqueray (tristan-cacqueray)]

This is the patch needed for keystoneclient. Besides paths change, the tests needed a minor tweak to work, in the keystoneclient, s3_token use self.verify instead of self._verify.

As it will be shipped with the advance notification, can someone make sure this is correct ?

[Tristan Cacqueray (tristan-cacqueray)]

The keystoneclient patch.

[Brant Knudson (blk-u)]

The patch in comment 17 looks correct to me. It passed tox -e py27,pep8.

[Brant Knudson (blk-u)]

Also passes tox -e py26,py33.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks Brant,

so proposed public disclosure date/time:
2015-04-14, 1500UTC

[Morgan Fainberg (mdrnstm)]

+1 for the report date etc.

[Jeremy Stanley (fungi)]

On rereading the impact description, I think for clarity we should have something more like:

... When the 'insecure' option is set in a S3Token paste configuration file its value is effectively ignored. Note that it's unusual to explicitly add this option and then set it to false, so the impact of this bug is thought to be limited. ...

[Jeremy Stanley (fungi)]

Or better still:

...its value is effectively ignored and instead assumed to be true. ...

[Guang Yee (guang-yee)]

++ to Jeremy's edition on comment #23

[Tristan Cacqueray (tristan-cacqueray)]

Thanks fungi, so for completeness, the revised impact description:

Title: S3Token TLS cert verification option not honored
Reporter: Brant Knudson (IBM)
Products: keystonemiddleware, python-keystoneclient
Affects: versions through 1.5.0 (keystonemiddleware),
         versions through 0.11.2 (python-keystoneclient)

Description:
Brant Knudson from IBM reported a vulnerability in keystonemiddleware (formerly shipped as python-keystoneclient). When the 'insecure' option is set in a S3Token paste configuration file its value is effectively ignored and instead assumed to be true. As a result certificate verification will be disabled, leaving TLS connections open to MITM attacks. Note that it's unusual to explicitly add this option and then set it to false, so the impact of this bug is thought to be limited. All versions of s3_token middleware with TLS settings configured are affected by this flaw.

[Brant Knudson (blk-u)]

the impact description in comment 25 looks good to me.

[Guang Yee (guang-yee)]

Comment #25 looks good!

[Jeremy Stanley (fungi)]

Thanks Tristan, I think that update will definitely help avoid confusion around the impact of this bug.

[Tristan Cacqueray (tristan-cacqueray)]

Oups, I got the python-keystoneclient version messed up, it should be:

Affects: versions through 1.3.0 (python-keystoneclient)

[Morgan Fainberg (mdrnstm)]

This will need to be backported to the 1.4.x series

[Tristan Cacqueray (tristan-cacqueray)]

Now that keystoneclient and keystonemiddleware have stable branch, we'll also need to cherry-pick there as well.
Tests succeed with proposed patch on those new branch.

Note that for python-keystoneclient there is a minor conflict on import: stable uses oslo_serialization.

[Tristan Cacqueray (tristan-cacqueray)]

Downstream is having difficulties to apply the proposed change on stable versions.

Here are the proposed backports for python-keystoneclient:

[Tristan Cacqueray (tristan-cacqueray)]

Here is the proposed backport for keystonemiddleware juno. As of now, the py27 keystonemiddleware.tests.test_auth_token_middleware test also fail without the patch...

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/173376

[Tristan Cacqueray (tristan-cacqueray)]

keystonemiddleware:
Kilo: https://review.openstack.org/173365
Juno: https://review.openstack.org/173376

python-keystoneclient:
Kilo: https://review.openstack.org/173370
Juno: https://review.openstack.org/173377
Icehouse: https://review.openstack.org/173378

Changeid: https://review.openstack.org/#/q/Id674f40532215788675c97a8fdfa91d4420347b3,n,z

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/173365
Committed: https://git.openstack.org/cgit/openstack/keystonemiddleware/commit/?id=90edbc8c1cfd2aa318bd70474f17e723cd67cb97
Submitter: Jenkins
Branch: master

commit 90edbc8c1cfd2aa318bd70474f17e723cd67cb97
Author: Brant Knudson <email address hidden>
Date: Mon Mar 23 18:19:18 2015 -0500

    Fix s3_token middleware parsing insecure option

    The "insecure" option was being treated as a bool when it was
    actually provided as a string. The fix is to parse the string to
    a bool.

    Change-Id: Id674f40532215788675c97a8fdfa91d4420347b3
    Closes-Bug: 1411063

[Fuel Devops McRobotson (fuel-devops-robot)]

Fix proposed to branch: master
Change author: Brant Knudson <email address hidden>
Review: https://review.fuel-infra.org/5625

[Boris Bobrov (bbobrov)]

Changed it back to "Fix commited", because status change was done by the bot by error.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/175595

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/176937

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/175595
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=8fa6b6f0b5e95493342ce71489d04f73db2418b8
Submitter: Jenkins
Branch: stable/kilo

commit 8fa6b6f0b5e95493342ce71489d04f73db2418b8
Author: Brant Knudson <email address hidden>
Date: Tue Apr 7 19:38:29 2015 +0000

    Fix s3_token middleware parsing insecure option

    The "insecure" option was being treated as a bool when it was
    actually provided as a string. The fix is to parse the string to
    a bool.

    Closes-Bug: 1411063
    Change-Id: Id674f40532215788675c97a8fdfa91d4420347b3

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/173378
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=0e3a23d28438f3a298a384b1e1f1390cfa92b151
Submitter: Jenkins
Branch: stable/icehouse

commit 0e3a23d28438f3a298a384b1e1f1390cfa92b151
Author: Brant Knudson <email address hidden>
Date: Tue Apr 7 19:38:29 2015 +0000

    Fix s3_token middleware parsing insecure option

    The "insecure" option was being treated as a bool when it was
    actually provided as a string. The fix is to parse the string to
    a bool.

    Closes-Bug: 1411063
    Change-Id: Id674f40532215788675c97a8fdfa91d4420347b3

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/176937
Committed: https://git.openstack.org/cgit/openstack/keystonemiddleware/commit/?id=0e63b0e13d0a7919fddd7576c99bc15a45d31a9f
Submitter: Jenkins
Branch: stable/kilo

commit 0e63b0e13d0a7919fddd7576c99bc15a45d31a9f
Author: Brant Knudson <email address hidden>
Date: Mon Mar 23 18:19:18 2015 -0500

    Fix s3_token middleware parsing insecure option

    The "insecure" option was being treated as a bool when it was
    actually provided as a string. The fix is to parse the string to
    a bool.

    Change-Id: Id674f40532215788675c97a8fdfa91d4420347b3
    Closes-Bug: 1411063

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/173376
Committed: https://git.openstack.org/cgit/openstack/keystonemiddleware/commit/?id=59f720ccc9a92da025baf7dc692e8e582ebfae0a
Submitter: Jenkins
Branch: stable/juno

commit 59f720ccc9a92da025baf7dc692e8e582ebfae0a
Author: Brant Knudson <email address hidden>
Date: Mon Mar 23 18:19:18 2015 -0500

    Fix s3_token middleware parsing insecure option

    The "insecure" option was being treated as a bool when it was
    actually provided as a string. The fix is to parse the string to
    a bool.

    Closes-Bug: 1411063
    Change-Id: Id674f40532215788675c97a8fdfa91d4420347b3