Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed diffie-hellman shared key computation #44

Merged
merged 2 commits into from
May 15, 2018

Conversation

pmconrad
Copy link

The shared secret computed by DH_compute_key can be smaller than the estimate provided with DH_size. This must be accounted for, otherwise there will be uninitialized trailing bytes in the resulting shared_key vector.
https://www.openssl.org/docs/man1.0.2/crypto/DH_compute_key.html

Copy link

@jmjatlanta jmjatlanta left a comment

Choose a reason for hiding this comment

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

Suggestion: The test must be run several times for keys to be generated that produce a shared key that is smaller than the estimated size. Perhaps adjust the test to run several iterations (15-25 in my testing), or hard-code the user keys so that they always produce a shared key that is smaller.

Copy link

@jmjatlanta jmjatlanta left a comment

Choose a reason for hiding this comment

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

Works well.

@pmconrad
Copy link
Author

Hm, travis failed with this (unrelated) error:

1306549ms th_a task_cancel.cpp:245 test_method ] 9 canceled_exception: Canceled
task simple_task canceled before starting, reason canceling scheduled task to test if cancel works
{"description":"simple_task","reason":"canceling scheduled task to test if cancel works"}
th_a task.cpp:50 run_impl
/home/travis/build/bitshares/bitshares-fc/tests/thread/thread_tests.cpp(65): error in "yields_execution": check "hello world" == result failed [hello world != worldhello ]

Looks like a race condition somewhere. Hopefully in the test only. :-/

@pmconrad
Copy link
Author

There is a race condition in thread_tests/yields_execution: if the first task executes its yield() before the second task has been created, the yield() is a no-op and "world" is written to the result before "hello ". The race condition can be reproduced by inserting a usleep() between the creation of the two async tasks.

I have tried fixing this with a spin lock, but didn't succeed. In any case this is unrelated to this PR, so I'm merging now.

@pmconrad pmconrad merged commit a6671d6 into bitshares:master May 15, 2018
@pmconrad pmconrad deleted the dh_fix branch May 15, 2018 16:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants