Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always buffer TCP data in __handle_recv() #44

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

frozencemetery
Copy link
Member

Refactor __handle_recv() to always create a BytesIO() object for TCP
data. Linearize control flow for ease of debugging. Always apply
length checks so that we don't have to wait for EOF in the multiple-recv
case.

Fixes a bug where we wouldn't return any data because we never received
the EOF, or didn't receive it fast enough.

Signed-off-by: Robbie Harwood [email protected]

Refactor __handle_recv() to always create a BytesIO() object for TCP
data.  Linearize control flow for ease of debugging.  Always apply
length checks so that we don't have to wait for EOF in the multiple-recv
case.

Fixes a bug where we wouldn't return any data because we never received
the EOF, or didn't receive it fast enough.

Signed-off-by: Robbie Harwood <[email protected]>
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch looks good to me.

@simo5 please take a look, too.

Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@frozencemetery frozencemetery merged commit 7e2b1ab into latchset:master Aug 29, 2019
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Apr 29, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request May 18, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request May 18, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request May 19, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request May 19, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request Jun 3, 2020
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Jun 3, 2020
The test fails with older versions of kdcproxy as the related bug
was fixed in 0.4.2: latchset/kdcproxy#44
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 5, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 5, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 17, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
abbra pushed a commit to freeipa/freeipa that referenced this pull request Mar 18, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.

Reviewed-By: Anuja More <[email protected]>
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 18, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 18, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 18, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
abbra pushed a commit to freeipa/freeipa that referenced this pull request Mar 18, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.

Reviewed-By: Anuja More <[email protected]>
wladich pushed a commit to wladich/freeipa that referenced this pull request Mar 29, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.
abbra pushed a commit to freeipa/freeipa that referenced this pull request Mar 30, 2021
…ackets

This is a regression test for the bug in python-kdcproxy mentioned in
latchset/kdcproxy#44
  When the reply from AD is split into several TCP packets the kdc
  proxy software cannot handle it and returns a false error message
  indicating it cannot contact the KDC server.

This could be observed as login failures of AD user on IPA clients
when:
* IPA client was configured to use kdcproxy to communicate with AD
* kdcproxy used TCP to communicate with AD
* response from AD to kdcproxy was split into several packets

This patch also refactors and improves existing tests:
* switch to using pytest fixtures for test setup and cleanup steps to make
  them isolated and reusable
* simulate a much more restricted network environment: instead of blocking
  single 88 port we now block all outgoing traffic except few essential
  ports
* add basic tests for using kdcproxy to communicate between IPA client
  and AD DC.

Reviewed-By: Anuja More <[email protected]>
Reviewed-By: Alexander Bokovoy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants