[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-dev] Stegotorus Challenge - Inheriting from FileStegMod



Hey Noah,

Thanks a lot for the patch. You are doing a great job. I commited your
changes here:

https://github.com/zackw/stegotorus/commit/a92f977259dd3aaabe9f5812f79bbe73456ecdf6#diff-07d0e8b69b9ddf7f371fd93a4204acea

Please pull the branch f--file-stegize-swf-js-pdf and continue work there.

Few comments:

1. Our main point is two get rid of redundant http_handle_client_SWF_receive, http_server_SWF_transmit (you undid deleting them). When you finish moving all modules the following "switch" in http_steg_t::http_client_receive (and in http_steg_t::transmit) will be discarded/disappear:

  switch(type) {
  case HTTP_CONTENT_SWF:
    rval = http_handle_client_SWF_receive(this, conn, dest, source);
    break;

  case HTTP_CONTENT_JAVASCRIPT:
  case HTTP_CONTENT_HTML:
    rval = http_handle_client_JS_receive(this, conn, dest, source);
    break;

  case HTTP_CONTENT_PDF:
    rval = http_handle_client_PDF_receive(this, conn, dest, source);
    break;

  default:
    log_debug(conn, "receiving a payload of type %i", type);
    rval = config->file_steg_mods[type]->http_client_receive(conn, dest, source);
    break;
     
  }

and we will only need this line (the default case):

    log_debug(conn, "receiving a payload of type %i", type);
    rval = config->file_steg_mods[type]->http_client_receive(conn, dest, source);

But for now, for any type that you move to FileStegMod structure, you just need to drop the case from this switch, because in new structure it will be taken care of in the default case, (so you don't get those 

src/steg/http.cc:697:67: error: 'http_handle_client_SWF_receive' was
not declared in this scope  

errors you mentioned).

2. In http.cc, add a line to http_steg_config_t::init_file_steg_mods() to create the SWFSteg:

void http_steg_config_t::init_file_steg_mods()
{
  // we can't call this in constructor cause 
  //it should be called after the payload server is initialized

  //initiating the steg modules
  //TODO: for now the first modules are set to void till their codes be
  //transformed into a FileStegMod child
  file_steg_mods[HTTP_CONTENT_JPEG] = new JPGSteg(payload_server, noise2signal);
  file_steg_mods[HTTP_CONTENT_PNG] = new PNGSteg(payload_server, noise2signal);
  file_steg_mods[HTTP_CONTENT_GIF] = new GIFSteg(payload_server, noise2signal);

}

3. compare how PayloadScraper::PayloadScraper defines old modules and new modules, and modify for SWFSteg accordingly:

  _available_file_stegs[HTTP_CONTENT_SWF] = NULL;
  _available_stegs[2].type = HTTP_CONTENT_SWF; _available_stegs[2].extension = ".swf";  _available_stegs[2].capacity_function = PayloadServer::capacitySWF;

vs.

  _available_file_stegs[HTTP_CONTENT_JPEG] = new JPGSteg(NULL); //We are only using the capacity function so we don't need a payload server
  _available_stegs[5].type = HTTP_CONTENT_JPEG; _available_stegs[5].extension = ".jpg"; _available_stegs[5].capacity_function = JPGSteg::static_capacity; //Temp measure, later we don't need to do such acrobat

Also please look at my comment on the proposal.

Bon courage,
vmon

Noah Rahman <selimthegrim@xxxxxxxxx> writes:

> result of running git format-patch origin/tor-improve --stdout > swfsteg.patch
>
> On 4/6/14, Noah Rahman <selimthegrim@xxxxxxxxx> wrote:
>> Success! Sending you the patch shortly...
>>
>> On 4/6/14, Noah Rahman <selimthegrim@xxxxxxxxx> wrote:
>>> attaching new steg files for purely informational purposes, these are
>>> the only ones I have modified - my guess is capacitySWF in payload
>>> server should remain a stub? It seems like we would want to take the
>>> capacity functions out of payload server...
>>>
>>>
>>>
>>> On 4/6/14, Noah Rahman <selimthegrim@xxxxxxxxx> wrote:
>>>> Here's my current holdup, I don't think I changed these functions but
>>>> will shelve what I have, restore, and do a diff.
>>>>
>>>> selimthegrim@Rimsky-Korsakoffee:~/stegotorus$ make
>>>> make  all-am
>>>> make[1]: Entering directory `/home/selimthegrim/stegotorus'
>>>> depbase=`echo src/steg/http.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
>>>> 	g++    -I. -I./src -I./src/steg -I./src/steg/http_steg_mods
>>>> -I./src/test/gtest -I./src/test/gtest/include   -Wall -Wextra
>>>> -Wno-missing-field-initializers -Wformat=2 -std=c++0x -g2 -O0
>>>> -std=gnu++11 -MT src/steg/http.o -MD -MP -MF $depbase.Tpo -c -o
>>>> src/steg/http.o src/steg/http.cc &&\
>>>> 	mv -f $depbase.Tpo $depbase.Po
>>>> src/steg/http.cc: In member function 'virtual int
>>>> http_steg_t::transmit(evbuffer*)':
>>>> src/steg/http.cc:562:75: error: 'http_server_SWF_transmit' was not
>>>> declared in this scope
>>>>        rval = http_server_SWF_transmit(config->payload_server, source,
>>>> conn);
>>>>
>>>> ^
>>>> src/steg/http.cc: In member function 'virtual int
>>>> http_steg_t::http_client_receive(evbuffer*, evbuffer*)':
>>>> src/steg/http.cc:697:67: error: 'http_handle_client_SWF_receive' was
>>>> not declared in this scope
>>>>      rval = http_handle_client_SWF_receive(this, conn, dest, source);
>>>>                                                                    ^
>>>> make[1]: *** [src/steg/http.o] Error 1
>>>> make[1]: Leaving directory `/home/selimthegrim/stegotorus'
>>>> make: *** [all] Error 2
>>>>
>>>>
>>>> On 4/6/14, Noah Rahman <selimthegrim@xxxxxxxxx> wrote:
>>>>> Ok will use c_HTTP and restore the free block. I did read that text
>>>>> file
>>>>> and have made appropriate changes
>>>>> On Apr 6, 2014 10:49 PM, <vmon@xxxxxxxxxx> wrote:
>>>>>
>>>>>> Hey Noah,
>>>>>>
>>>>>> > size_t tmp_len = cover_len * 32; //assert here to prevent overflow
>>>>>> This is probably prevented somewhere else, but it is always good to
>>>>>> harden the code.
>>>>>>
>>>>>> >   /*if (inf_len < 0 ||
>>>>>> >       out_sz < inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN)
>>>>>> > {
>>>>>> >     fprintf(stderr, "inf_len = %d\n", inf_len);
>>>>>> >     free(tmp_buf);
>>>>>> >     return -1;
>>>>>> >   }*/
>>>>>> Why did you get rid of this part?
>>>>>>
>>>>>> >//how to check max SWF size? c_HTTP_MSG_BUF_SIZE?
>>>>>> Sure, there is a confusion between HTTP_MSG_BUF_SIZE and
>>>>>> c_HTTP_MSG_BUF_SIZE but we should unite them and use one (the latter
>>>>>> is
>>>>>> better).
>>>>>>
>>>>>> >    tmp_len= inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN;
>>>>>> > //reassigned to existing variable
>>>>>> >    return (ssize_t)tmp_len; //added for new return type
>>>>>> sure, you can do
>>>>>>
>>>>>> return (ssize_t)(inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN);
>>>>>>
>>>>>> But it isn't a big deal.
>>>>>>
>>>>>> Also please take a look at:
>>>>>>
>>>>>>
>>>>>> doc/writing_new_file_steg_mod_for_http_steg.txt
>>>>>>
>>>>>> Best of luck,
>>>>>> vmon
>>>>>>
>>>>>> Noah Rahman <selimthegrim@xxxxxxxxx> writes:
>>>>>>
>>>>>> > Have finished everything I think I need, trying now to compile a
>>>>>> > patch
>>>>>> > for SWF. Will start on JS once that is socked away. A few questions
>>>>>> > though... (on tmp_len line, and commented out free block)
>>>>>> >
>>>>>> > ssize_t
>>>>>> > SWFSteg::decode(const uint8_t *cover_payload, size_t cover_len,
>>>>>> > uint8_t*
>>>>>> data)
>>>>>> > {
>>>>>> >   int inf_len;
>>>>>> >   size_t tmp_len = cover_len * 32; //assert here to prevent overflow
>>>>>> > for large covers?
>>>>>> >   char* tmp_buf = (char *)xmalloc(tmp_len);
>>>>>> >
>>>>>> >   for (;;) {
>>>>>> >     inf_len = decompress(cover_payload + 8, cover_len - 8,
>>>>>> >                          (uint8_t *)tmp_buf, tmp_len);
>>>>>> >     if (inf_len != -2)
>>>>>> >       break;
>>>>>> >     tmp_len *= 2;
>>>>>> >     tmp_buf = (char *)xrealloc(tmp_buf, tmp_len);
>>>>>> >   }
>>>>>> >
>>>>>> >   /*if (inf_len < 0 ||
>>>>>> >       out_sz < inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN)
>>>>>> > {
>>>>>> >     fprintf(stderr, "inf_len = %d\n", inf_len);
>>>>>> >     free(tmp_buf);
>>>>>> >     return -1;
>>>>>> >   }*/ //how to check max SWF size? c_HTTP_MSG_BUF_SIZE?
>>>>>> >
>>>>>> >   memcpy(data, tmp_buf + SWF_SAVE_HEADER_LEN,
>>>>>> >          inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN);
>>>>>> >    tmp_len= inf_len - SWF_SAVE_HEADER_LEN - SWF_SAVE_FOOTER_LEN;
>>>>>> > //reassigned to existing variable
>>>>>> >    return (ssize_t)tmp_len; //added for new return type
>>>>>> > }
>>>>>> >
>>>>>> > On 4/5/14, vmon <vmon@xxxxxxxxxx> wrote:
>>>>>> >>
>>>>>> >>
>>>>>> >> Just make a patch of you changes and email me the patch:
>>>>>> >>
>>>>>> >> http://ariejan.net/2009/10/26/how-to-create-and-apply-a-patch-with-git/
>>>>>> >> Envoyà depuis un mobile Samsung
>>>>>> >>
>>>>>> >> -------- Message d'origine --------
>>>>>> >> De : Noah Rahman <selimthegrim@xxxxxxxxx>
>>>>>> >> Date :2014/04/05  19:11  (GMT-05:00)
>>>>>> >> Ã : vmon@xxxxxxxxxx
>>>>>> >> Objet : Re: Stegotorus Challenge - Inheriting from FileStegMod
>>>>>> >>
>>>>>> >> Ok. I'll shoot for SWF and Js by turn and add that to a revised
>>>>>> >> proposal for
>>>>>> >> the challenge period. I have not heard back from Zack yet. I am in
>>>>>> >> UTC+5 now
>>>>>> >> so that leaves me all of my Sunday at least?
>>>>>> >>
>>>>>> >> On Apr 5, 2014 11:02 PM, <vmon@xxxxxxxxxx> wrote:
>>>>>> >> Hey Noah,
>>>>>> >>
>>>>>> >> Just keep in mind that I need to make a recommendation on your
>>>>>> >> application by the end of the weekend.
>>>>>> >>
>>>>>> >> I also like very much if you can add "merging SRI branch of
>>>>>> >> Stegotorus"
>>>>>> >> to our Stegotorus as well, to the list of the deliverables of the
>>>>>> >> proposal.
>>>>>> >>
>>>>>> >> Thanks,
>>>>>> >> vmon
>>>>>> >>
>>>>>> >> Noah Rahman <selimthegrim@xxxxxxxxx> writes:
>>>>>> >>> Yes, the capacities were a bit of a head scratcher. I'll try swf
>>>>>> >>> first
>>>>>> >>> and then js. My github ID is baronwolfenstein.
>>>>>> >>>
>>>>>> >>> Hey Noah,
>>>>>> >>>
>>>>>> >>> For committing:
>>>>>> >>> I think Zack needs to give you write access. Please send your
>>>>>> >>> github
>>>>>> >>> id
>>>>>> >>> to Zack (zackw@xxxxxxx) and cc me. Once that is done, please
>>>>>> >>> branch
>>>>>> >>> out
>>>>>> >>> of tor-improve and commit. If it takes Zack long to give you
>>>>>> >>> permission,
>>>>>> >>> you can just fork Zackw/stegotorus of course. Just please only
>>>>>> >>> commit
>>>>>> >>> compilable code (no syntax error etc), this way it is easier to
>>>>>> >>> review
>>>>>> >>> the commits.
>>>>>> >>>
>>>>>> >>> SWF would be easier than JS but if you like the JS it is cool with
>>>>>> >>> me. For the swf, basically swf_wrap should go to encode,
>>>>>> >>> swf_unwrap
>>>>>> >>> to
>>>>>> >>> decode and you need basically discard the http_server_SWF_transmit
>>>>>> >>> and
>>>>>> >>> http_handle_client_SWF_receive functions (these are
>>>>>> >>> redundant). Just
>>>>>> >>> be
>>>>>> >>> careful that what is supposed to be send to each function, is it
>>>>>> >>> the
>>>>>> >>> payload without header? with header? etc. So just tweak the
>>>>>> >>> swf_unwrap/swf_wrap to be compatible with what encode/decode.
>>>>>> >>>
>>>>>> >>> Capacity functions:
>>>>>> >>> Then PayloadServer::capacitySWF should be implemented as
>>>>>> >>> swfSteg::capacity.
>>>>>> >>> However, You'll find 4 capacity functions and it is probably quite
>>>>>> >>> confusing.
>>>>>> >>>
>>>>>> >>> capacity
>>>>>> >>> headless_capacity
>>>>>> >>> static_capacity
>>>>>> >>> static_headless_capacity
>>>>>> >>>
>>>>>> >>> static vs virtual:
>>>>>> >>> When all the modules are moved to the new model, we'll get rid of
>>>>>> >>> static_capacity, static_headless_capacity but till then, we need
>>>>>> >>> them
>>>>>> >>> to
>>>>>> >>> keep this old/new hybrid stegs model. Hence:
>>>>>> >>>
>>>>>> >>> capacity just calls static_capacity
>>>>>> >>> headless_capacity just calls static_headless_capacity
>>>>>> >>>
>>>>>> >>> headless vs headfull:
>>>>>> >>> headless_capcity: computes the capacity when the payload is given
>>>>>> >>> without
>>>>>> >>> http header (pure file).
>>>>>> >>>
>>>>>> >>> capacity: compute the capacity when the payload is given as an
>>>>>> >>> HTTP
>>>>>> >>> response (So it starts with
>>>>>> >>>
>>>>>> >>> HTTP/1.1 200 OK Server: Apache
>>>>>> >>> Accept-Ranges: bytes
>>>>>> >>> Content-Type: text/html; charset=utf-8
>>>>>> >>> Content-Length: 920
>>>>>> >>> Connection: keep-alive
>>>>>> >>>
>>>>>> >>> and then the file starts, so you just need basically to skip the
>>>>>> >>> header
>>>>>> >>> and call headless_capacity (we probably should implement the
>>>>>> >>> capacity
>>>>>> >>> in
>>>>>> >>> the parent and only make headless_capacity pure virtual).
>>>>>> >>>
>>>>>> >>> If you take look at:
>>>>>> >>>
>>>>>> >>> jpgSteg::capacity
>>>>>> >>> jpgSteg::headless_capacity
>>>>>> >>> jpgSteg::static_capacity
>>>>>> >>> jpgSteg::static_headless_capacity
>>>>>> >>>
>>>>>> >>> all I what say will sound less gibberish to you.
>>>>>> >>>
>>>>>> >>> Good luck et bon courage,
>>>>>> >>> vmon
>>>>>> >>>
>>>>>> >>> Noah Rahman <selimthegrim@xxxxxxxxx> writes:
>>>>>> >>>
>>>>>> >>>> Laying over in Tokyo enroute to bkk, had a look at all the
>>>>>> >>>> modules
>>>>>> >>> and
>>>>>> >>>> will start on the JS one on the plane. I have 24 hours in bkk so
>>>>>> >>>> should be able to convert the Js and at least one of the other
>>>>>> >>>> ones
>>>>>> >>> if
>>>>>> >>>> not both to inherit from the FileStegMod by the time I leave
>>>>>> >>>> there.
>>>>>> >>>> Should I submit the pr to your Tor-improve branch or zackw's?
>>>>>> >>>>
>>>>>> >>
>>>>>>
>>>>>
>>>>
>>>
>>
_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev