Closed Dioprz closed 2 weeks ago
For the first issue I responded in #152. Essentially lets restrict in both cases to a single execution of the estimators (removing the min) and increasing the tolerance to a higher value (I tested with 1.2, worked in 100 trials). We should include a comment that the higher tolerance compensates for the variance caused by the probabilistic nature of beullens runtime computation.
For the second issue:
For 1. maybe @FloydZ can give some feedback on this? I am not sure I yet understand the issue correctly.
For 2. I again suggest removing the correction factor and instead increasing the tolerance. I tested with tolerance equal to 1
and had no issues.
first commit: essential what @Memphisd wrote. Increasing the tolerance is fine, as long as we leave a comment explaining the problem.
second commit: I think the problem is the log2
function in cost.sage
, which uses log()
, which itself gets overwritten by attack_cost.sage
. And then you get the divide by 0
exception. So you can add a if/else catch block in the log2
function to return math.inf
in this case.
Thanks for your comments @Memphisd and @FloydZ .
log
in cost.sage
as sage_log
and then propagating the change in the file. It apparently works, but if you think this is bad for some reason, just let me know and I will apply your solution instead.Now new problems have arisen:
Once I remove the correction factor of the test and apply the second change mentioned above, I'm able to launch a succesful make docker-generate-kat
. However I'm getting the next results when running make docker-pytest-kat
: (click the items to see the logs)
Notice some things from this:
cost.sage
apparently works.bbps_1
and bbps_range
are really big (~3 for bbps_1
and ~15 for bbps_range
).Input: (100, 54, 31), estimator: bbps_range
You will notice that the second one has as actual output inf
.Please let me know if I'm misunderstood something about your comments, but this things looks like new problems for me.
ok had a look into the new problem. The current values in KATS.yml
are correct. I checked a few values against the online estimator. This means the output of the sage script is correct and the output of new python implementation is faulty. Will check it. And the problem only affect BBPS.
ok fixxed it. There were a few typos in tests/internal_estimators/le.py
calling the wrong function, wrong arguments.
Thank you so much for looking at the problem, and for the solution commit @FloydZ. I will make a cleanup and merge develop, and this will be ready to merge @Javierverbel :rocket:.
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Description
Help required
The migration of the LEEStimator has multiple issues for which I need help: @Javierverbel @Memphisd @FloydZ.
I will describe them in the same order I suggest you to look at them: by commit.
Note: Take in count that the LEEstimator have two testing files to migrate:
First commit: Migration of
test_le_beullens.sage
. Here we have two issues:internal_estimator
(calledt1
there) isinf
or not in the new framework. For this reason I only check the external estimator, as can be seen here: https://github.com/Crypto-TII/CryptographicEstimators/blob/611a0b9b7fbcedd898e48c4defbee30f659705f8/tests/external_estimators/ext_le.sage#L33-L41 While this doesn't broke anything, it is still a change so let me know if there is a better fix..yaml
files:good_kat.yaml
,failing_kat.yaml
andfailing_kat_2.yaml
. As expected, the prior have values matching the ones calculated by the internal estimator, but the second and third ones don't. (Possibly related: https://github.com/Crypto-TII/CryptographicEstimators/issues/152). If you want to test them, just change the name of any of them bykat.yaml
and runmake docker-run
followed bypytest tests/test_kat.py
.Second commit: Migration of
test_le_bbps.sage
. Here we another two issues:1.
As discussed in some point, the "load" function used in Sage pollutes the namespace of the current execution. That means that if there are any function defined in the current execution namespace, and that same function is defined in a "loaded" file, the loaded function will override the first one.
This behavior is causing that when these two files are loaded in the same file
The
bbps
kat generators doesn't work and complains about some "zero division". However, if you remove theattack_cost.sage
load, then they works as expected (but beulleans doesn't). I have tested swapping its loading order and the result is the same.This means that
attack_cost.sage
has some function used by some kat generator of the bbps functions, that isn't defined incost.sage
, and overwrites the default implementation thatbbps
functions expects.Probably the easier solution is to just find that function of
attack_cost.sage
, and change the name by something else (ex.log
->log_
) and propagate the change where is appropriate.2. At https://github.com/Crypto-TII/CryptographicEstimators/blob/2c97a0ed6c926af8e3481872c46ceb034cad87d4/tests/external_estimators/LEEstimator/test_le_bbps.sage#L43-L63 we are correcting the value produced by the external estimator using the internal one. This is not supported at the moment, so we need to solve that too.
Testing tips
ext_sdfq -> _ext_sdfq
andext_sd -> _ext_sd
. This way the commandmake docker-generate kat
only generate the kats ofext_le
. Otherwise you will have to wait more time.BBPS
and then runningmake docker-generate-kat
, and making the same by commenting theBEULLENS
section too.Review process
Not ready yet.
Pre-approval checklist