gamelinux / passivedns

A network sniffer that logs all DNS server replies for use in a passive DNS setup
http://gamelinux.org/
1.67k stars 372 forks source link

Parts of the code was copied without permission. #19

Closed errzey closed 12 years ago

errzey commented 12 years ago

It seems as if you have used code from a project of mine without permission or giving credits.

The project in which a portion of your code is derived is here: https://github.com/ellzey/dns-archiver. It's blatantly obvious due to the fact this project retained even my source comments word-for-word.

This is a good example:

https://github.com/gamelinux/passivedns/blob/master/src/dns.c#L319 https://github.com/ellzey/dns-archiver/blob/master/archiver.c#L244

Though I did not apply a license to my project, it does not mean it defaults to no restrictions. According to the Bern Convention which almost all countries abide by, you are under violation.

If you credit the orignal project, you have permission :)

Cheers!

errzey commented 12 years ago

Hmm, I just looked and realized you released this code under GPL. If it is kept this way, I do not give you permission, either you change to a less-restrictive license (like BSD) and add credits, or I'll issue a takedown.

Thanks!

gamelinux commented 12 years ago

Well, Ill remove the comment that is from your project, but if you look at the source code of ldns, you have clearly stolen the way they do stuff in their source, which is just what I have if I remove your comments. You even use the same parameter names from ldns, so I really dont see the issue, other than I kept the non intuitive names for your functions.

If you look at some of the functions, they have the same name as yours, but they are called with different values.

If you look at the specific switch that your refer to in your comparison, you will find the exact switch in the ldns_rr2buffer_wire_canonical function from ldns:

.... switch (ldns_rr_get_type(rr)) { ....

you also call ldns_rr_list_rr with the same parameters as libldns: ldns_rr_list_rr(list, i);

gamelinux commented 12 years ago

hm... that got posted.... closed! before I hit any buttons here :/

gamelinux commented 12 years ago

A lot the names/params you have are from lib ldns. A lot of the solutions you have, are from lib ldns. If I rewrite the function names, and the parameter names, Ill end up with the same code as I have. And beside, the code is so different if you look at the majority of it. Ill credit you and your app for beeing the app looking much alike what I was coding, and Ill credit you for your comments, and the names of the parameters used, but again, Ill rewrite them and strip them out.

If you compare your "associated_lookup_or_make_insert" with mine "associated_lookup_or_make_insert" you will see that it reassembles very little etc.

BTW - I have no problems with BSD :)

gamelinux commented 12 years ago

AUTHORS pushed in c04111e126e481dbac97aa6a2fa1a091ae2ef32b

gamelinux commented 12 years ago

BTW - your source code has no credits to non eater....

errzey commented 12 years ago

If you look at the specific switch that your refer to in your comparison, you will find the exact switch in the ldns_rr2buffer_wire_canonical function from ldns:

I'm trying to figure out how I copied ldns based on your comment. How does this:

ldns_status
ldns_rr2buffer_wire_canonical(ldns_buffer *buffer,
                    const ldns_rr *rr,
                    int section)
{
uint16_t i;
uint16_t rdl_pos = 0;
bool pre_rfc3597 = false;
switch (ldns_rr_get_type(rr)) {
case LDNS_RR_TYPE_NS:
case LDNS_RR_TYPE_MD:
case LDNS_RR_TYPE_MF:
case LDNS_RR_TYPE_CNAME:
case LDNS_RR_TYPE_SOA:
case LDNS_RR_TYPE_MB:
case LDNS_RR_TYPE_MG:
case LDNS_RR_TYPE_MR:
case LDNS_RR_TYPE_PTR:
case LDNS_RR_TYPE_HINFO:
case LDNS_RR_TYPE_MINFO:
case LDNS_RR_TYPE_MX:
case LDNS_RR_TYPE_RP:
case LDNS_RR_TYPE_AFSDB:
case LDNS_RR_TYPE_RT:
case LDNS_RR_TYPE_SIG:
case LDNS_RR_TYPE_PX:
case LDNS_RR_TYPE_NXT:
case LDNS_RR_TYPE_NAPTR:
case LDNS_RR_TYPE_KX:
case LDNS_RR_TYPE_SRV:
case LDNS_RR_TYPE_DNAME:
case LDNS_RR_TYPE_A6:
case LDNS_RR_TYPE_RRSIG:
    pre_rfc3597 = true;
    break;
default:
    break;
}

A copy of:

    rr = ldns_rr_list_rr(list, i);

    switch (ldns_rr_get_type(rr)) {
        /* at the moment, we only really care about
         * rr's that have an addr or cname for the rname. */
        case LDNS_RR_TYPE_AAAA:
        case LDNS_RR_TYPE_A:
        case LDNS_RR_TYPE_CNAME:
            data_offset = 0;
            break;
        default:
            data_offset = -1;
            break;
    }

you also call ldns_rr_list_rr with the same parameters as libldns: ldns_rr_list_rr(list, i);

Really? list is type ldns_rr_list, that's like saying int i; for (i = 0....) is copied code. Even if so, calling an exported function from a library isn't copying, it's using.

The part I pointed out wasn't the only example.

From dns-archiver:

log_debug("%d qa_rrcount", qa_rrcount);

for (i = 0; i < qa_rrcount; i++) {
    ldns_rr  *question_rr;
    ldns_rdf *rdf_data;
    int       ret;

    question_rr = ldns_rr_list_rr(questions, i);
    rdf_data    = ldns_rr_owner(question_rr);

    log_debug("archive(): rdf_data = %p", rdf_data);

    /* plop all the answers into the correct archive_node_t's
     * associated_nodes hash. */
    ret = archive_lname_list(archive_hash, rdf_data, answers, dns_buffer);

    if (ret < 0) {
        log_debug("archive_lname_list() returned error");
    }
    /* archive_lname_list(archive_hash, rdf_data, authorities, dns_buffer); */
}

From passivedns

dlog("[*] %d qa_rrcount\n", qa_rrcount);

// Do we ever have more than one Question?
// If we do - are we handling it correct ?
for (i = 0; i < qa_rrcount; i++) {
    ldns_rr  *question_rr;
    ldns_rdf *rdf_data;
    int       ret;

    question_rr = ldns_rr_list_rr(questions, i);
    rdf_data    = ldns_rr_owner(question_rr);

    dlog("[D] rdf_data = %p\n", rdf_data);

    /* plop all the answers into the correct archive_node_t's
     * associated_nodes hash. */
    ret = archive_lname_list(pi, rdf_data, answers, dns_buffer, decoded_dns);

    if (ret < 0) {
        dlog("[D] archive_lname_list() returned error\n");
    }
}

Heck, you even retained code you don't even use.

dns-archiver:

typedef enum {
    QUESTION,
    ANSWER
} query_type_t;

typedef struct {
    char   *key;
    uint8_t type;
    time_t  ts; 
} associated_node_t;

typedef struct {
    query_type_t query_type;
    char        *key;
    GHashTable  *associated;
} archive_node_t;    

passivedns:

typedef enum {
     QUESTION,
     ANSWER
} query_type_t;

typedef struct {
     query_type_t   query_type;
    char          *key;
} archive_node_t;       
errzey commented 12 years ago

BTW - your source code has no credits to non eater....

No credits to whom?

errzey commented 12 years ago

Thanks for looking into this issue.

Cheers.

gamelinux commented 12 years ago

What Im talking about is derivative work, but still, Im not saying I did not reuse the way of thinking and the names of params/functions from your code, but your code does not fit into my application "as is", and your functions != my functions... but yes, it is derived from your code and ldns in some ways, Im not saying I didnt!

It is obvious a convenient way of doing: switch (ldns_rr_get_type(rr) instead of if, if, if, if, and when ldns does it, why not.

"..calling an exported function from a library isn't copying, it's using. " its derived work obviously if you manage to reuse the same name for the variables etc? There is a small chance that you made them up, and got the same...

But any ways, Ill rewrite from scratch all the parts that you like. No derivative work, no nothing.

E

errzey commented 12 years ago

It is obvious a convenient way of doing: switch (ldns_rr_get_type(rr) instead of if, if, if, if, and when ldns does it, why not.

if / else if / else conditions don't compile down the same as a switch/case statement. With switch/case a compiler generates a lookup table for easy access, while if/else uses a jump table.

It's standard practice to switch/case over a static enumeration on non static input, not only for performance, but because compilers will give off useful warnings if you don't cover every possible case on an enum. The compiler won't do that with if/else.

its derived work obviously if you manage to reuse the same name for the variables etc?

things like rr, qr, lname, rname are standard names used by anyone working on or about DNS systems.

But any ways, Ill rewrite from scratch all the parts that you like. No derivative work, no nothing.

Black-boxing is perfectly acceptable in software design. Looking at code to get an idea of how to do things, running it to see how it runs, then using all that information to create a completely new work is just fine. It was the cut-and-paste that had me a little riled up :)

errzey commented 12 years ago

If it's any interest, I looked at some of your TODO's and thought some of my really old code may be of use to you in some way:

https://strcpy.net/mark/dns_stuff/dns_index/ - It's a DNS query specific indexer for large pcap files. Check out the README for details.

https://strcpy.net/mark/dns_stuff/dns_spoof_watch/ - uses fuzzy logic in order to determine if someone is running a dnsid-based spoof attack.

gamelinux commented 12 years ago

I know why we use switch, thats why Im saying thats its not the same as and if if if if, and that there are reasons why they should be a switch, and a reason why they are like that in any ldns based code Ive seen.

archive_query() I see is in my code, but also never used btw.

It would be interested to see what you have done, but after this thread, I guess not :)