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

fix for python3 TypeError: ord() expected string of length 1, but int… #128

Merged
merged 2 commits into from
Apr 16, 2020

Conversation

slieberth
Copy link
Contributor

@slieberth slieberth commented Mar 23, 2020

python3 compatibility for SaltCrypt()

Copy link
Collaborator

@Istvan91 Istvan91 left a comment

Choose a reason for hiding this comment

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

It can be merged after the comments have been resolved.

salt = struct.pack('!H', random_generator.randrange(0, 65535))
salt = chr(ord(salt[0]) | 1 << 7)+salt[1]
if six.PY3:
random_value = 32768 + random_generator.randrange(0, 32767)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct, this is effectively a random number between 32768 and 65535 when it should be between 0 and 65535 (the full range of 2 bytes). The random value calculation is the same for the python2 and python3 case, so move it please out of the conditional blocks.

pyrad/packet.py Outdated
Comment on lines 578 to 586
#salt = struct.pack('!H', random_generator.randrange(0, 65535))
#salt = chr(ord(salt[0]) | 1 << 7)+salt[1]

length = struct.pack("B", len(value))
buf = length + value
if len(buf) % 16 != 0:
buf += six.b('\x00') * (16 - (len(buf) % 16))

result = six.b(salt)
#result = six.b(salt)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please delete commented out statements.

@slieberth
Copy link
Contributor Author

Hi Istvan, please double check RFC 2868:
Salt
The Salt field is two octets in length and is used to ensure the
uniqueness of the encryption key used to encrypt each instance of
the Tunnel-Password attribute occurring in a given Access-Accept
packet. The most significant bit (leftmost) of the Salt field
MUST be set (1).
The contents of each Salt field in a given
Access-Accept packet MUST be unique.

@Istvan91
Copy link
Collaborator

thanks for pointing that out, I've looked at the RFC earlier, but I completely missed that.
Nevertheless fixing it for the "old" python2 case would nice.

@Istvan91 Istvan91 merged commit ad0c828 into pyradius:master Apr 16, 2020
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.

2 participants