tgtadm iscsi chap does not work

[ling-yun]

When using LVMISCSIDriver and iscsi_helper tgtadm, it should support chap unidirectional authentication because target configuration file and db.volume has record chap user and chap passwd.
By testing, I found that tgtadm iscsi chap does not work.
Is it a security bug for iscsi_helper tgtadm?

My detail test work is as follows.
1. Test details as follows without modify the source code:
1) Devstack all in one server A(10.250.10.190); another testing server B(10.250.10.191)
2) create a vm VM-A and a cinder volume VOLUME-A, attach VOLUME-A to VM-A
3) server B directly login the iscsi target that server-A export and get VOLUME-A sucessfully .
    iscsiadm -m discovery -t sendtargets -p 10.250.10.190
    iscsiadm -m node -T iqn.2010-10.org.openstack:volume-ee32035f-73d2-4312-a468-c7773f90a75e -p 10.250.10.190 -l --login

2. Test details as follows with modify the source code:
1) add creating user/passwd and binding user to tid code before leaving the function TgtAdm:create_iscsi_target.
        type, name, passwd = chap_auth.split()
        self._execute('tgtadm',
                      '--lld',
                      'iscsi',
                      '--mode',
                      'account',
                      '--op',
                      'new',
                      '--user',
                      name,
                      '--password',
                      passwd)
        self._execute('tgtadm',
                      '--lld',
                      'iscsi',
                      '--mode',
                      'account',
                      '--op',
                      'bind',
                      '--tid',
                      tid,
                      '--user',
                      name
                      )

2) try to login VOLUME-A as the steps in item 1, it reported an authorization error as follows.
root@devaio1:/etc/iscsi# iscsiadm -m node -T iqn.2010-10.org.openstack:volume-ee32035f-73d2-4312-a468-c7773f90a75e -p 10.250.10.190 -l --login
Logging in to [iface: default, target: iqn.2010-10.org.openstack:volume-ee32035f-73d2-4312-a468-c7773f90a75e, portal: 10.250.10.190,3260] (multiple)
iscsiadm: Could not login to [iface: default, target: iqn.2010-10.org.openstack:volume-ee32035f-73d2-4312-a468-c7773f90a75e, portal: 10.250.10.190,3260].
iscsiadm: initiator reported error (24 - iSCSI login failed due to authorization failure)
iscsiadm: Could not log into all portals

[Thierry Carrez (ttx)]

This may be a bug, but I fail to see the security vulnerability in it. Could you explain how this may constitute a vulnerability ?

[ling-yun (zengyunling)]

If cinder support tgtadm iscsi chap auth, we could only connect and mount volume with chap username and chap password in order to prevent Unauthorized access.
Now we could connect and mount volume without chap username and chap password, which would leave anybody to access volume arbitrarily.

[ling-yun (zengyunling)]

Did I explain clearly about the problem?
Looking forward to your feedback.

[Thierry Carrez (ttx)]

Adding Cinder security contacts so that they can confirm the issue.

[ling-yun (zengyunling)]

hi stackers, looking forward to your feedback.

[John Griffith (john-griffith)]

Hi ling-yun,

Indeed, somewhere along the way we've broken setup of CHAP. I don't know that I would classify this as requiring a private security bug, but yes it's a bug that needs to be addressed.

Being that we keep the iscsi connection info away from the end-user I think we can just open this as a "regular" bug, but mark it higher priority.

Thanks!

[Thierry Carrez (ttx)]

Yes, I don't think that qualifies as a vulnerability. It's a missing security feature that would result in generally strengthening the Cinder setup.

If you agree, I will open this bug publicly so that the bug can be fixed in future versions of Cinder as fast as possible.

[ling-yun (zengyunling)]

To John Griffith (john-griffith) & Thierry Carrez (ttx) :
OK, agree with both you two.
We should fix it up as soon as possible.

[Jeremy Stanley (fungi)]

Updated to a public bug, no advisory, tagged as possible security hardening feature/improvement.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Duncan Thomas (<email address hidden>) on branch: master
Review: https://review.openstack.org/102420
Reason: Abandoning change: No update for more than 14 days

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Nathan Kinder (nkinder)]

Did CHAP authentication ever work for this case? I would like to know if this is a regression. This seems worthy of a security note since people might assume that their volumes are protected when they are in-fact accessible.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/128478
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=e3563891545c801726d227f752cf99488ed5c7dd
Submitter: Jenkins
Branch: master

commit e3563891545c801726d227f752cf99488ed5c7dd
Author: Tomoki Sekiyama <email address hidden>
Date: Tue Oct 14 19:09:44 2014 -0400

    Fix LVM iSCSI driver tgtadm CHAP authentication

    Currently CHAP Authentication in LVM iSCSI driver with tgtadm does not work.
    This is because the tgtadm helper creates the target configuration file
    with an 'IncomingUser' entry, which is ignored by tgtd.
    This patch fixes it to 'incominguser'.

    Change-Id: I14871985a2a916834122f849238f05b75726bc1a
    Closes-Bug: #1329214

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/128507
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=be3d4604dc0566e0838959d998ff1d37755de6d3
Submitter: Jenkins
Branch: proposed/juno

commit be3d4604dc0566e0838959d998ff1d37755de6d3
Author: Tomoki Sekiyama <email address hidden>
Date: Tue Oct 14 19:09:44 2014 -0400

    Fix LVM iSCSI driver tgtadm CHAP authentication

    Currently CHAP Authentication in LVM iSCSI driver with tgtadm does not work.
    This is because the tgtadm helper creates the target configuration file
    with an 'IncomingUser' entry, which is ignored by tgtd.
    This patch fixes it to 'incominguser'.

    Change-Id: I14871985a2a916834122f849238f05b75726bc1a
    Closes-Bug: #1329214
    (cherry picked from commit e3563891545c801726d227f752cf99488ed5c7dd)

[Tomoki Sekiyama (tsekiyama)]

Hi Nathan,

I'm not sure if it had worked, but it seems already broken in Jan '14 according to this page:
https://ask.openstack.org/en/users/2406/ale/?sort=recent

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Mike Perez (<email address hidden>) on branch: master
Review: https://review.openstack.org/128920

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/128920
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=66494f54112fdfa135b3974c75aa388c8d1fb49e
Submitter: Jenkins
Branch: master

commit be3d4604dc0566e0838959d998ff1d37755de6d3
Author: Tomoki Sekiyama <email address hidden>
Date: Tue Oct 14 19:09:44 2014 -0400

    Fix LVM iSCSI driver tgtadm CHAP authentication

    Currently CHAP Authentication in LVM iSCSI driver with tgtadm does not work.
    This is because the tgtadm helper creates the target configuration file
    with an 'IncomingUser' entry, which is ignored by tgtd.
    This patch fixes it to 'incominguser'.

    Change-Id: I14871985a2a916834122f849238f05b75726bc1a
    Closes-Bug: #1329214
    (cherry picked from commit e3563891545c801726d227f752cf99488ed5c7dd)

commit f7ee62cc58d8b642af67510a310f6259492a4508
Author: Mitsuhiro Tanino <email address hidden>
Date: Tue Oct 14 12:41:41 2014 -0400

    Export cinder volumes only if the status is 'in-use'

    Currently, cinder volumes are exported both 'in-use' and 'available'
    after restarting cinder-volume service.
    This behavior was introduced following commit.

      commit ffefe18334a9456250e1b6ff88b7b47fb366f374
      Author: Zhiteng Huang <email address hidden>
      Date: Sat Aug 23 18:32:57 2014 +0000

    If the volumes are attached to nova instances, they should be exported
    via tgtd after restarting cinder-volume.
    But the volumes which are not attached to instances must not be exported
    because everyone can connect these volumes.

    This patch changes volume export behavior that exports a volume only if
    the volume status is 'in-use'.

    Change-Id: I4c598c240b9290c81bd8001e5a0720c8c329aeb9
    Signed-off-by: Mitsuhiro Tanino <email address hidden>
    Closes-bug: #1381106
    (cherry picked from commit e2f28b967910625432be0eab6a851adf53ac58ea)

commit 01e7c516852e53df661b2eedc970c327c1ff10ce
Author: Vipin Balachandran <email address hidden>
Date: Fri Oct 10 23:06:27 2014 +0530

    Revert "Relocate volume to compliant datastore"

    Commit 4be8913520f5e9fe4109ade101da9509e4a83360 introduced a regression
    which causes failures during cinder volume re-attach. This patch reverts
    commit 4be8913520f5e9fe4109ade101da9509e4a83360 as an immediate fix.

    Closes-Bug: #1379830
    Change-Id: I5dfbd45533489c3c81db8d256bbfd2f85614a357
    (cherry picked from commit 48cb82971e0418f9a629e2b39d0433dc2c0e6919)

commit 900d49723f65e87658381ff955559f54ac98c487
Author: Andreas Jaeger <email address hidden>
Date: Thu Oct 9 12:25:28 2014 +0200

    Updated translations

    Commands run:-
    $ python setup.py extract_messages
    $ python setup.py update_catalog --no-fuzzy-matching \
      --ignore-obsolete=true
    $ source \
      ../openstack-infra/project-config/jenkins/scripts/common_translation_update.sh
    $ setup_loglevel_vars
    $ cleanup_po_files cinder

    Change-Id: I73f3bdccb4be98df95fa853864e465f4d83a8884

commit 8e94aaa2b28b491314fe8642061ac73e3fe8e966
Author: Navneet Singh <email address hidden>
Date: Thu Aug 28 16:03:41 2014 +0530

    NetApp fix eseries unit test mock clean

 ...

[Dave Walker (davewalker)]

@steve.westom: The OSSN task is assigned to you, are you still working on this?

Thanks

[Michael McCune (mimccune)]

OSSN review

https://review.openstack.org/#/c/219939

[Nathan Kinder (nkinder)]

This has been published as OSSN-0058:

  https://wiki.openstack.org/wiki/OSSN/OSSN-0058