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