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

probably_prime is skipping lots of primes, including primes below 2⁶⁴ #51

Closed
darconeous opened this issue Jun 27, 2023 · 6 comments · Fixed by #54
Closed

probably_prime is skipping lots of primes, including primes below 2⁶⁴ #51

darconeous opened this issue Jun 27, 2023 · 6 comments · Fixed by #54
Assignees
Labels
bug Something isn't working

Comments

@darconeous
Copy link

darconeous commented Jun 27, 2023

From the documentation for probably_prime:

ProbablyPrime is 100% accurate for inputs less than 2⁶⁴. See Menezes et al., Handbook of Applied Cryptography, 1997, pp. 145-149, and FIPS 186-4 Appendix F for further discussion of the error probabilities.

I have found counterexamples which show this assertion to be false:

assert!(probably_prime(&BigUint::from_u64(1579751).unwrap(), 20));
assert!(probably_prime(&BigUint::from_u64(1884791).unwrap(), 20));
assert!(probably_prime(&BigUint::from_u64(3818929).unwrap(), 20));
assert!(probably_prime(&BigUint::from_u64(4080359).unwrap(), 20));
assert!(probably_prime(&BigUint::from_u64(4145951).unwrap(), 20));

All 5 of these prime numbers are lower than 2⁶⁴, yet none cause probably_prime to return true.

@dignifiedquire
Copy link
Owner

dignifiedquire commented Jun 27, 2023

The algorithm is based on the implementation in math/big from golang, including the reference. So it would be good to check if there is an issue in either the original algorithm, the go implementation, or just this implementation.

@darconeous
Copy link
Author

Looks like it works fine in Go: https://go.dev/play/p/IfJc2JqaCjL

@dignifiedquire
Copy link
Owner

Thanks for checking, that is not good.

dignifiedquire added a commit that referenced this issue Jun 27, 2023
Changes based on reviewing the lastest version of golang math/big

Closes #51
@dignifiedquire
Copy link
Owner

@darconeous I have a fix, at least for the primes you posted in #54

@dignifiedquire dignifiedquire added the bug Something isn't working label Jun 27, 2023
@dignifiedquire dignifiedquire self-assigned this Jun 27, 2023
@darconeous
Copy link
Author

In my initial testing, this seems to be performing much, much better. The output now looks equivalent to OpenSSL's BN_is_prime function.

dignifiedquire added a commit that referenced this issue Jun 28, 2023
Changes based on reviewing the lastest version of golang math/big

Closes #51
@dignifiedquire
Copy link
Owner

Thanks a lot for reporting and testing!

dignifiedquire added a commit that referenced this issue Jul 7, 2023
Changes based on reviewing the lastest version of golang math/big

Closes #51
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 a pull request may close this issue.

2 participants