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

Ensure unique x-coordinates with sampling (Fixes #20) #21

Merged
merged 9 commits into from
Mar 13, 2023

Conversation

sidsbrmnn
Copy link
Collaborator

This pull request fixes an issue where duplicate x-coordinates could be generated. The fix involves using random sampling to ensure that all x-coordinates in the list are unique.

Changes Made

  • pyshamir.py - use random.sample() to generate a list of unique numbers than manually loop to generate a random secret.
  • test_pyshamir.py - manually test that the parts are different

@sidsbrmnn sidsbrmnn added the bug Something isn't working label Mar 11, 2023
@sidsbrmnn sidsbrmnn self-assigned this Mar 11, 2023
@sidsbrmnn sidsbrmnn linked an issue Mar 11, 2023 that may be closed by this pull request
@sidsbrmnn
Copy link
Collaborator Author

Oops! Looks like we can't use random.sample(). Hmm, maybe just brute force the uniqueness.

@konidev20
Copy link
Owner

Can we make x_coordinates a set?

Also, thinking if generating random numbers per array index is a good idea in terms of security?

Need to check the best way to generate a random array correctly.

@Loadingname91
Copy link
Collaborator

Loadingname91 commented Mar 12, 2023

Can we make x_coordinates a set?

Also, thinking if generating random numbers per array index is a good idea in terms of security?

Need to check the best way to generate a random array correctly.

How about using this

import secrets x_coordinates = list(range(256)) secrets.SystemRandom().shuffle(x_coordinates) x_coordinates = x_coordinates[:-1]

pyshamir/shamir.py Outdated Show resolved Hide resolved
Copy link
Owner

@konidev20 konidev20 left a comment

Choose a reason for hiding this comment

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

LGTM. @Loadingname91 , @AbhishekSrikanth please check and let me know.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@konidev20 konidev20 merged commit e172958 into main Mar 13, 2023
@sidsbrmnn sidsbrmnn deleted the 20-split-method-generating-duplicate-shares-sometimes branch March 21, 2023 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split method generating duplicate shares sometimes
4 participants