Hello Adam Litke, Freddy Rolland,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/50940

to review the following change.

Change subject: iscsi: Fix credentials initialization when not using CHAP
......................................................................

iscsi: Fix credentials initialization when not using CHAP

There was a mismatch between the way we initialize ChapCredentials when
receiving empty credentials from engine, and when reading empty
credentials from active session.

This was broken when we started to protect password with
ProtectedPassword.  When getting a request from engine we used
ProtectedPassword(""), while the iscsi code used None. This causes
session disconnect to fail with "cred mismatch".

Now we have a note explaining the dependency and we use the same way to
initialize the credentials:

- When receiving request from engine, empty username or password are
  stored as None.
- When reading requests from active session, empty username or password
  are stored as None, including the special empty values "<NULL>" and
  "(null)".

Code was moved so username and password normalization is near
ChapCredentials initialization in both flows.

Change-Id: I65e1177bc33998b8eba34b0fd04ec62cf8bf1a2c
Bug-Url: https://bugzilla.redhat.com/1279485
Reported-By: Elad Ben Aharon <ebena...@redhat.com>
Signed-off-by: Nir Soffer <nsof...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/48483
Reviewed-by: Freddy Rolland <froll...@redhat.com>
Continuous-Integration: Jenkins CI
Reviewed-by: Adam Litke <ali...@redhat.com>
---
M vdsm/storage/hsm.py
M vdsm/storage/iscsi.py
2 files changed, 21 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/50940/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index b80adbc..f8eb3e5 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -230,9 +230,17 @@
                                    conDict.get('initiatorName', None),
                                    conDict.get('netIfaceName', None))
 
+        # NOTE: ChapCredentials must match the way we initialize username and
+        # password when reading session info in iscsi.readSessionInfo(). Empty
+        # or missing username or password are stored as None.
+
+        username = conDict.get('user')
+        if not username:
+            username = None
+        password = conDict.get('password')
+        if not getattr(password, "value", None):
+            password = None
         cred = None
-        username = conDict.get('user', None)
-        password = conDict.get('password', None)
         if username or password:
             cred = iscsi.ChapCredentials(username, password)
 
diff --git a/vdsm/storage/iscsi.py b/vdsm/storage/iscsi.py
index 74fba43..f33cfc4 100644
--- a/vdsm/storage/iscsi.py
+++ b/vdsm/storage/iscsi.py
@@ -156,21 +156,24 @@
     port = int(port)
     tpgt = int(tpgt)
 
-    # Fix username and password if needed (iscsi reports empty user/password
-    # as "<NULL>" (RHEL5) or "(null)" (RHEL6)
-    if username in ["<NULL>", "(null)"]:
-        username = None
-    if password.value in ["<NULL>", "(null)"]:
-        password = None
-
     if netdev in ["<NULL>", "(null)"]:
         netdev = None
 
     iface = IscsiInterface(iface, netIfaceName=netdev)
     portal = IscsiPortal(ip, port)
     target = IscsiTarget(portal, tpgt, iqn)
+
+    # NOTE: ChapCredentials must match the way we initialize username and
+    # password when receiving request from engine in
+    # hsm._connectionDict2ConnectionInfo().
+    # iscsi reports empty user/password as "<NULL>" (RHEL5) or "(null)"
+    # (RHEL6);  empty values are stored as None.
+
+    if username in ["<NULL>", "(null)", ""]:
+        username = None
+    if password.value in ["<NULL>", "(null)", ""]:
+        password = None
     cred = None
-    # FIXME: Don't just assume CHAP
     if username or password:
         cred = ChapCredentials(username, password)
 


-- 
To view, visit https://gerrit.ovirt.org/50940
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I65e1177bc33998b8eba34b0fd04ec62cf8bf1a2c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to