Skip to content

bpo-31933: fix blake2 multi-byte params on big endian platforms#4250

Merged
tiran merged 1 commit into
python:masterfrom
oconnor663:blake2-big-endian
Nov 3, 2017
Merged

bpo-31933: fix blake2 multi-byte params on big endian platforms#4250
tiran merged 1 commit into
python:masterfrom
oconnor663:blake2-big-endian

Conversation

@oconnor663

@oconnor663 oconnor663 commented Nov 3, 2017

Copy link
Copy Markdown
Contributor

All Blake2 params have to be encoded in little-endian byte order. For
the two multi-byte integer params, leaf_length and node_offset, that
means that assigning a native-endian integer to them appears to work on
little-endian platforms, but gives the wrong result on big-endian. The
current libb2 API doesn't make that very clear, and @sneves is working
on new API functions in the GH issue above. In the meantime, we can work
around the problem by explicitly assigning little-endian values to the
parameter block.

See BLAKE2/libb2#12.

https://bugs.python.org/issue31933

@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@oconnor663

Copy link
Copy Markdown
Contributor Author

Just added my GitHub username to my BPO account.

@oconnor663

Copy link
Copy Markdown
Contributor Author

Added a news entry.

@oconnor663

Copy link
Copy Markdown
Contributor Author

@sneves could you give me feedback on this change from the libb2 side of things?

@tiran tiran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests are failing. Please run make all clinic to regenerate the argument clinic files.

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

All Blake2 params have to be encoded in little-endian byte order. For
the two multi-byte integer params, leaf_length and node_offset, that
means that assigning a native-endian integer to them appears to work on
little-endian platforms, but gives the wrong result on big-endian. The
current libb2 API doesn't make that very clear, and @sneves is working
on new API functions in the GH issue above. In the meantime, we can work
around the problem by explicitly assigning little-endian values to the
parameter block.

See BLAKE2/libb2#12.
@oconnor663

Copy link
Copy Markdown
Contributor Author

Thanks, definitely didn't know what I was doing there. I have made the requested changes; please review again :)

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

@tiran tiran self-assigned this Nov 3, 2017

@tiran tiran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, it's a bit tricky.

blake2s_impl.c is generated from blake2b_impl.c, then argument clinic does its magic to create the entry points.

@tiran tiran merged commit dcfb0e3 into python:master Nov 3, 2017
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @oconnor663 for the PR, and @tiran for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 3, 2017
…onGH-4250)

All Blake2 params have to be encoded in little-endian byte order. For
the two multi-byte integer params, leaf_length and node_offset, that
means that assigning a native-endian integer to them appears to work on
little-endian platforms, but gives the wrong result on big-endian. The
current libb2 API doesn't make that very clear, and @sneves is working
on new API functions in the GH issue above. In the meantime, we can work
around the problem by explicitly assigning little-endian values to the
parameter block.

See BLAKE2/libb2#12.
(cherry picked from commit dcfb0e3)
@bedevere-bot

Copy link
Copy Markdown

GH-4262 is a backport of this pull request to the 3.6 branch.

@oconnor663 oconnor663 deleted the blake2-big-endian branch November 3, 2017 19:05
tiran pushed a commit that referenced this pull request Nov 3, 2017
) (#4262)

All Blake2 params have to be encoded in little-endian byte order. For
the two multi-byte integer params, leaf_length and node_offset, that
means that assigning a native-endian integer to them appears to work on
little-endian platforms, but gives the wrong result on big-endian. The
current libb2 API doesn't make that very clear, and @sneves is working
on new API functions in the GH issue above. In the meantime, we can work
around the problem by explicitly assigning little-endian values to the
parameter block.

See BLAKE2/libb2#12.
(cherry picked from commit dcfb0e3)
embray pushed a commit to embray/cpython that referenced this pull request Nov 9, 2017
…on#4250)

All Blake2 params have to be encoded in little-endian byte order. For
the two multi-byte integer params, leaf_length and node_offset, that
means that assigning a native-endian integer to them appears to work on
little-endian platforms, but gives the wrong result on big-endian. The
current libb2 API doesn't make that very clear, and @sneves is working
on new API functions in the GH issue above. In the meantime, we can work
around the problem by explicitly assigning little-endian values to the
parameter block.

See BLAKE2/libb2#12.
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.

5 participants