This repository was archived by the owner on Mar 4, 2021. It is now read-only.
HTTP Host and TLS SNI fragmentation/stuttering/fuzzing test#1522
Closed
joelanders wants to merge 8 commits intomasterfrom
Closed
HTTP Host and TLS SNI fragmentation/stuttering/fuzzing test#1522joelanders wants to merge 8 commits intomasterfrom
joelanders wants to merge 8 commits intomasterfrom
Conversation
bassosimone
reviewed
Dec 20, 2017
| auto aread = std::make_shared<std::string>(); | ||
| auto awrite = std::make_shared<std::string>(); | ||
|
|
||
| SSL_CTX* ctx = SSL_CTX_new(SSLv23_method()); //XXX don't ignore errors |
Member
There was a problem hiding this comment.
SSLv23_method() cannot fail, IIRC. It should return a pointer to a statically allocated struct.
bassosimone
reviewed
Dec 20, 2017
| return *aread; //XXX probably raise exception | ||
| } | ||
|
|
||
| if (connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) { |
Member
There was a problem hiding this comment.
If think you can ask OpenSSL to connect on your behalf
bassosimone
reviewed
Dec 20, 2017
|
|
||
| void dpi_fragment(Settings options, Callback<SharedPtr<report::Entry>> callback, | ||
| SharedPtr<Reactor> reactor, SharedPtr<Logger> logger) { | ||
| reactor->call_in_thread(logger, [=]() { |
Member
There was a problem hiding this comment.
I suggest to do [callback = std::move(callback)]. This is C++14 and should guarantee that the thread uniquely owns the callback. I remember not doing that was leading to weird behavior sometimes.
bassosimone
reviewed
Dec 20, 2017
| logger->info("unfragmented https response length: %d", unfragmented_https.length()); | ||
| logger->info("fragmented http response length: %d", fragmented_http.length()); | ||
| logger->info("unfragmented http response length: %d", unfragmented_http.length()); | ||
| callback(entry); |
Member
There was a problem hiding this comment.
I suggest to do
reactor->call_soon([entry = std::move(entry), callback = std::move(callback)]() {
callback(entry);
});so that the callback is called from the I/O thread loop, which is what usually happens.
bassosimone
reviewed
Dec 20, 2017
| int main(std::list<Callback<BaseTest &>> &initializers, int argc, char **argv) { | ||
| mk::nettests::DpiFragmentTest test; | ||
| int ch; | ||
| while ((ch = getopt(argc, argv, "B:f:")) != -1) { |
Member
There was a problem hiding this comment.
You should clean the options string here
joelanders
force-pushed
the
sni_stutter
branch
from
d80070b to
e15375d
Compare
joelanders
force-pushed
the
sni_stutter
branch
from
e15375d to
d74f415
Compare
Contributor
Author
|
joelanders
changed the title
Member
|
Closing as stated in ooni/spec#103 (comment). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to spec PR
So instead of trying to hook things at the libevent level, which even Simone says is Hard To Do (:)), I'm doing a good ol' blocking
socket(); connect(); select(); read(); write();loop in my own thread. I'm telling OpenSSL to talk to some buffers instead of the socket directly, and I'm taking care of read/writing them to the socket (with thesleep()trick around the plaintext hostname).