Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: ssl: handle handshake errors
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina 
Gerrit-Reviewer: Oved Ourfali 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has submitted this change and it was merged. ( 
https://gerrit.ovirt.org/79668 )

Change subject: ssl: handle handshake errors
..


ssl: handle handshake errors

There are two possible errors (sslerror and socket.error) which are
raised during ssl handshake that need to be handled. We do not want
to see unhandled error backtraces in the logs which could easily
generate enourmos amount of logs when malicious client exploit it.
We log now only the occurence of the error and the offending IP address
that caused the error.

We fix the issue by handling both errors and cleanly closing file
descriptor.

Bug-url: https://bugzilla.redhat.com/1473295
Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Signed-off-by: Piotr Kliczewski 
Signed-off-by: Irit Goihman 
---
M lib/vdsm/sslutils.py
1 file changed, 6 insertions(+), 1 deletion(-)

Approvals:
  Martin Peřina: Looks good to me, but someone else must approve
  Yaniv Bronhaim: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Irit Goihman: Verified
  Dan Kenigsberg: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina 
Gerrit-Reviewer: Oved Ourfali 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Yaniv Bronhaim :

Yaniv Bronhaim has posted comments on this change.

Change subject: ssl: handle handshake errors
..


Patch Set 9: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Oved Ourfali 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: ssl: handle handshake errors
..


Patch Set 7:

(1 comment)

https://gerrit.ovirt.org/#/c/79668/7/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 254: elif err.args[0] == ssl.SSL_ERROR_WANT_WRITE:
Line 255: self.want_write = True
Line 256: else:
Line 257: dispatcher.close()
Line 258: except socket.error:
silently ignoring these two errors is counter intuitive. Maybe you can add a 
comment for future developers?
Line 259: dispatcher.close()
Line 260: else:
Line 261: self.want_read = self.want_write = True
Line 262: self._is_handshaking = False


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Yaniv Bronhaim :

Yaniv Bronhaim has posted comments on this change.

Change subject: ssl: handle handshake errors
..


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/79668/4//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2017-07-24 12:08:16 +0200
Line 6: 
Line 7: ssl: handle handshake errors
Line 8: 
Line 9: We need to make sure to close socket when handshake fails.
what happened otherwise?
Line 10: 
Line 11: 
Line 12: Bug-url: https://bugzilla.redhat.com/1473295
Line 13: Change-Id: I99cfa35e608f429640455c35495be1783854e3da


https://gerrit.ovirt.org/#/c/79668/4/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 250: self.want_read = True
Line 251: elif err.args[0] == ssl.SSL_ERROR_WANT_WRITE:
Line 252: self.want_write = True
Line 253: else:
Line 254: dispatcher.close()
don't we need the above treatment ?
Line 255: except socket.error:
Line 256: dispatcher.close()
Line 257: else:
Line 258: self.want_read = self.want_write = True


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: ssl: handle handshake errors

[Code Review]

From Yaniv Bronhaim :

Yaniv Bronhaim has posted comments on this change.

Change subject: ssl: handle handshake errors
..


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/79668/1/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 187: self._handshake(dispatcher)
Line 188: except socket.error:
Line 189: self.log.error("Handshake failed")
Line 190: dispatcher.socket.close()
Line 191: return
seems like _handshake itself takes care of exceptions.. if you need to handle 
socker.error somewhere its there and not here
Line 192: else:
Line 193: if config.getboolean('vars', 'verify_client_cert'):
Line 194: if not 
self._verify_host(dispatcher.socket.getpeercert(),
Line 195:  
dispatcher.socket.getpeername()[0]):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org