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

files_load_unique_data: fix unique data copy to ecc point #3331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gomesj
Copy link
Contributor

@gomesj gomesj commented Jan 8, 2024

Split of unique data into x and y components of ecc point currently fumbles the indexes leading to incomplete copy.
Using "IAK" (size=3) as input, this leads to:

x.size = (unique_size / 2) = 1
x.buffer = unique_size[0,1[ = ['I']
y.size = (unique_size / 2) + (unique_size % 2) = 2
y.buffer = unique_size[1,2[ + 0 = ['A', 0]

Change copy bounds to copy "bigger" first half in x and remainder in y to reach a more reasonable result:

x.size = (unique_size / 2) + (unique_size % 2) = 2
x.buffer = unique_size[0,2[ = ['I', 'A']
y.size = (unique_size / 2) + (unique_size % 2) = 2
y.buffer = unique_size[2,3[ + 0 = ['K', 0]

Split of unique data into x and y components of ecc point currently
fumbles the indexes leading to incomplete copy.
Using "IAK" (size=3) as input, this leads to:
> x.size = (unique_size / 2) = 1
> x.buffer = unique_size[0,1[ = ['I']
> y.size = (unique_size / 2) + (unique_size % 2) = 2
> y.buffer = unique_size[1,2[ + 0 = ['A', 0]

Change copy bounds to copy "bigger" first half in x and remainder in y
to reach a more reasonable result:
> x.size = (unique_size / 2) + (unique_size % 2) = 2
> x.buffer = unique_size[0,2[ = ['I', 'A']
> y.size = (unique_size / 2) + (unique_size % 2) = 2
> y.buffer = unique_size[2,3[ + 0 = ['K', 0]

Signed-off-by: Julien Gomes <julien@arista.com>
Copy link

codecov bot commented Jan 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b83de07) 74.38% compared to head (93a4262) 74.38%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3331      +/-   ##
==========================================
- Coverage   74.38%   74.38%   -0.01%     
==========================================
  Files         173      173              
  Lines       23673    23672       -1     
==========================================
- Hits        17610    17609       -1     
  Misses       6063     6063              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndreasFuchsTPM
Copy link
Member

I think this makes a lot of sense.

BUT, this looks like a change in behavior, i.e. different key derivation and such.
So shall we postpone this until 6.0 ?

@williamcroberts Your thoughts ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants