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.

Sign up for GitHub

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

Resolve concurrency issue #133

Closed
wants to merge 3 commits into from
Closed

Resolve concurrency issue #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa22e19
Revert "Revert "Resolve potential concurrency issue""
mkomitee Jul 1, 2019
7f4327b
Avoid memory leak
mkomitee Jul 1, 2019
3e3da1c
Drop support for winrm.
mkomitee Jul 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
History
=======

0.14.0.dev0: 2019-07-01
-----------------------

- Dropped winrm support. The kerberos context is now attached to response
objects so applications like winrm can be implemented external to
requests-kerberos.
- Corrected a concurrency issue exposed by threaded applications.

0.12.0: 2017-12-20
------------------------

Expand Down
2 changes: 1 addition & 1 deletion requests_kerberos/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@

__all__ = ('HTTPKerberosAuth', 'MutualAuthenticationError', 'REQUIRED',
'OPTIONAL', 'DISABLED')
__version__ = '0.13.0.dev0'
__version__ = '0.14.0.dev0'
51 changes: 34 additions & 17 deletions requests_kerberos/kerberos_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,13 @@ def __init__(
self.hostname_override = hostname_override
self.sanitize_mutual_error_response = sanitize_mutual_error_response
self.auth_done = False
self.winrm_encryption_available = hasattr(kerberos, 'authGSSWinRMEncryptMessage')

# Set the CBT values populated after the first response
self.send_cbt = send_cbt
self.cbt_binding_tried = False
self.cbt_struct = None

def generate_request_header(self, response, host, is_preemptive=False):
def generate_request_header(self, response, host, request=None, is_preemptive=False):
"""
Generates the GSSAPI authentication token with kerberos.

Expand All @@ -195,6 +194,9 @@ def generate_request_header(self, response, host, is_preemptive=False):

"""

if request is None:
request = response.request

# Flags used by kerberos module.
gssflags = kerberos.GSS_C_MUTUAL_FLAG | kerberos.GSS_C_SEQUENCE_FLAG
if self.delegate:
Expand All @@ -209,9 +211,11 @@ def generate_request_header(self, response, host, is_preemptive=False):
kerb_host = self.hostname_override if self.hostname_override is not None else host
kerb_spn = "{0}@{1}".format(self.service, kerb_host)

result, self.context[host] = kerberos.authGSSClientInit(kerb_spn,
result, ctx = kerberos.authGSSClientInit(kerb_spn,
gssflags=gssflags, principal=self.principal)

self.context[request] = ctx

if result < 1:
raise EnvironmentError(result, kerb_stage)

Expand All @@ -222,18 +226,18 @@ def generate_request_header(self, response, host, is_preemptive=False):
kerb_stage = "authGSSClientStep()"
# If this is set pass along the struct to Kerberos
if self.cbt_struct:
result = kerberos.authGSSClientStep(self.context[host],
result = kerberos.authGSSClientStep(ctx,
negotiate_resp_value,
channel_bindings=self.cbt_struct)
else:
result = kerberos.authGSSClientStep(self.context[host],
result = kerberos.authGSSClientStep(ctx,
negotiate_resp_value)

if result < 0:
raise EnvironmentError(result, kerb_stage)

kerb_stage = "authGSSClientResponse()"
gss_response = kerberos.authGSSClientResponse(self.context[host])
gss_response = kerberos.authGSSClientResponse(ctx)

return "Negotiate {0}".format(gss_response)

Expand Down Expand Up @@ -298,6 +302,9 @@ def handle_other(self, response):

log.debug("handle_other(): Handling: %d" % response.status_code)

setattr(response, 'requests_kerberos_context',
self.context.pop(response.request, None))

if self.mutual_authentication in (REQUIRED, OPTIONAL) and not self.auth_done:

is_http_error = response.status_code >= 400
Expand Down Expand Up @@ -348,16 +355,19 @@ def authenticate_server(self, response):
log.debug("authenticate_server(): Authenticate header: {0}".format(
_negotiate_value(response)))

host = urlparse(response.url).hostname
ctx = response.requests_kerberos_context
if ctx is None:
log.exception("authenticate_server(): no established context")
return False

try:
# If this is set pass along the struct to Kerberos
if self.cbt_struct:
result = kerberos.authGSSClientStep(self.context[host],
result = kerberos.authGSSClientStep(ctx,
_negotiate_value(response),
channel_bindings=self.cbt_struct)
else:
result = kerberos.authGSSClientStep(self.context[host],
result = kerberos.authGSSClientStep(ctx,
_negotiate_value(response))
except kerberos.GSSError:
log.exception("authenticate_server(): authGSSClientStep() failed:")
Expand Down Expand Up @@ -417,24 +427,31 @@ def deregister(self, response):
response.request.deregister_hook('response', self.handle_response)

def wrap_winrm(self, host, message):
if not self.winrm_encryption_available:
raise NotImplementedError("WinRM encryption is not available on the installed version of pykerberos")

return kerberos.authGSSWinRMEncryptMessage(self.context[host], message)
raise NotImplementedError(
"WinRM encryption is no longer supported. The established "
"kerberos is now made available on the returned response objects "
"with the attribute named 'requests_kerberos_context' so WinRM "
"and other similar applications can be implemented external to "
"requests_kerberos."
)

def unwrap_winrm(self, host, message, header):
if not self.winrm_encryption_available:
raise NotImplementedError("WinRM encryption is not available on the installed version of pykerberos")
raise NotImplementedError(
"WinRM encryption is no longer supported. The established "
"kerberos is now made available on the returned response objects "
"with the attribute named 'requests_kerberos_context' so WinRM "
"and other similar applications can be implemented external to "
"requests_kerberos."
)

return kerberos.authGSSWinRMDecryptMessage(self.context[host], message, header)

def __call__(self, request):
if self.force_preemptive and not self.auth_done:
# add Authorization header before we receive a 401
# by the 401 handler
host = urlparse(request.url).hostname

auth_header = self.generate_request_header(None, host, is_preemptive=True)
auth_header = self.generate_request_header(None, host, request=request, is_preemptive=True)

log.debug("HTTPKerberosAuth: Preemptive Authorization header: {0}".format(auth_header))

Expand Down
46 changes: 26 additions & 20 deletions tests/test_requests_kerberos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,14 +265,13 @@ def test_authenticate_server(self):
with patch.multiple(kerberos_module_name, authGSSClientStep=clientStep_complete):

response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.requests_kerberos_context = "CTX"
response_ok.status_code = 200
response_ok.headers = {
'www-authenticate': 'negotiate servertoken',
'authorization': 'Negotiate GSSRESPONSE'}

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
result = auth.authenticate_server(response_ok)

self.assertTrue(result)
Expand All @@ -281,15 +280,16 @@ def test_authenticate_server(self):
def test_handle_other(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_complete):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200
response_ok.headers = {
'www-authenticate': 'negotiate servertoken',
'authorization': 'Negotiate GSSRESPONSE'}

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

r = auth.handle_other(response_ok)

Expand All @@ -299,15 +299,16 @@ def test_handle_other(self):
def test_handle_response_200(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_complete):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200
response_ok.headers = {
'www-authenticate': 'negotiate servertoken',
'authorization': 'Negotiate GSSRESPONSE'}

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

r = auth.handle_response(response_ok)

Expand All @@ -317,13 +318,14 @@ def test_handle_response_200(self):
def test_handle_response_200_mutual_auth_required_failure(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_error):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200
response_ok.headers = {}

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

self.assertRaises(requests_kerberos.MutualAuthenticationError,
auth.handle_response,
Expand All @@ -334,15 +336,16 @@ def test_handle_response_200_mutual_auth_required_failure(self):
def test_handle_response_200_mutual_auth_required_failure_2(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_exception):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200
response_ok.headers = {
'www-authenticate': 'negotiate servertoken',
'authorization': 'Negotiate GSSRESPONSE'}

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

self.assertRaises(requests_kerberos.MutualAuthenticationError,
auth.handle_response,
Expand All @@ -353,16 +356,17 @@ def test_handle_response_200_mutual_auth_required_failure_2(self):
def test_handle_response_200_mutual_auth_optional_hard_failure(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_error):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200
response_ok.headers = {
'www-authenticate': 'negotiate servertoken',
'authorization': 'Negotiate GSSRESPONSE'}

auth = requests_kerberos.HTTPKerberosAuth(
requests_kerberos.OPTIONAL)
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

self.assertRaises(requests_kerberos.MutualAuthenticationError,
auth.handle_response,
Expand All @@ -373,13 +377,14 @@ def test_handle_response_200_mutual_auth_optional_hard_failure(self):
def test_handle_response_200_mutual_auth_optional_soft_failure(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_error):

request = requests.PreparedRequest()
response_ok = requests.Response()
response_ok.url = "http://www.example.org/"
response_ok.request = request
response_ok.status_code = 200

auth = requests_kerberos.HTTPKerberosAuth(
requests_kerberos.OPTIONAL)
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

r = auth.handle_response(response_ok)

Expand All @@ -390,8 +395,9 @@ def test_handle_response_200_mutual_auth_optional_soft_failure(self):
def test_handle_response_500_mutual_auth_required_failure(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_error):

request = requests.PreparedRequest()
response_500 = requests.Response()
response_500.url = "http://www.example.org/"
response_500.request = request
response_500.status_code = 500
response_500.headers = {}
response_500.request = "REQUEST"
Expand All @@ -402,7 +408,7 @@ def test_handle_response_500_mutual_auth_required_failure(self):
response_500.cookies = "COOKIES"

auth = requests_kerberos.HTTPKerberosAuth()
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

r = auth.handle_response(response_500)

Expand All @@ -422,7 +428,6 @@ def test_handle_response_500_mutual_auth_required_failure(self):

# re-test with error response sanitizing disabled
auth = requests_kerberos.HTTPKerberosAuth(sanitize_mutual_error_response=False)
auth.context = {"www.example.org": "CTX"}

r = auth.handle_response(response_500)

Expand All @@ -431,8 +436,9 @@ def test_handle_response_500_mutual_auth_required_failure(self):
def test_handle_response_500_mutual_auth_optional_failure(self):
with patch(kerberos_module_name+'.authGSSClientStep', clientStep_error):

request = requests.PreparedRequest()
response_500 = requests.Response()
response_500.url = "http://www.example.org/"
response_500.request = request
response_500.status_code = 500
response_500.headers = {}
response_500.request = "REQUEST"
Expand All @@ -444,7 +450,7 @@ def test_handle_response_500_mutual_auth_optional_failure(self):

auth = requests_kerberos.HTTPKerberosAuth(
requests_kerberos.OPTIONAL)
auth.context = {"www.example.org": "CTX"}
auth.context = {request: "CTX"}

r = auth.handle_response(response_500)

Expand Down Expand Up @@ -562,7 +568,7 @@ def test_generate_request_header_custom_service(self):
response.headers = {'www-authenticate': 'negotiate token'}
host = urlparse(response.url).hostname
auth = requests_kerberos.HTTPKerberosAuth(service="barfoo")
auth.generate_request_header(response, host),
auth.generate_request_header(response, host)
clientInit_complete.assert_called_with(
"[email protected]",
gssflags=(
Expand Down