NLnetLabs / unbound

Unbound is a validating, recursive, and caching DNS resolver.
https://nlnetlabs.nl/unbound
BSD 3-Clause "New" or "Revised" License
3.17k stars 360 forks source link

calc_hash() function #61

Open iruzanov opened 5 years ago

iruzanov commented 5 years ago

Hello, Wouter once again! Firstly, big thank you for help to look for "new place" of your great codes ;)

Secondly, i have specific question to you: I test your latest Unbound-1.9.2 in conjunction of redis in-memory database. And my general purpose of the project (first step) is to write simple redis client to do some actions like connect to redis db and read/modify some DNS data for example TTL values of an RRs. And i wrote such the client using your sources redis.c (and other necessary C- and header files) and testcode/pktview.c to decode binary data stored in redis db. So, now i have some task to process the KEYs stored in redis. Now they look like: 1) "8C5B5634EA9E8C973A14ED5E4DEA9FD2F278E04504C73203BFFEFC96E5764C9A" 2) "34F89932F644AFA26BDD8C73565ECE26040908606E9879C1703649EA26B3AE9F" 3) "6BCB8FB9523A1C43ABEE55C9D1346840B7C1A85943CC4BFE3C0DDE92157AEFC5" 4) "E6EA9BA0D7586174E7D34E86CB08EF44B185E6BD60FE01E31C0A7DED22145BFF" 5) "www.google.com" 6) "5771013AE51CE03274B0CAB3BAF6A760B55BE6E022C71BD77E5D41594D6464AD" 7) "896B4381F0310B8C7B7DE656210C9079C18B09D6742B4C6FF438AF955B1E6104" 8) "053CBEEE46F43E5F33FF34A464D5E68A439A23BD0B4DD98CCDF9DFA31418B0DA" 9) "5991C8E781FEB0DB5D5113A3E66E6E5F202044BE2985675841B764067408BBC7" 10) "7FEF3AA1B0C4DF1D0558D02A8A14E3E202C2782F2A59B3D4D4E1391D26EB988C" 11) "3D77C6793AA9D3660A647041F4F93EE529D4287A879DFE9085CFE4C1D3D2F54F" 12) "2B07AFD3B97277D95126F897D4E34D1CA950F27A9F41442255672D19753489AC" 13) "8EACF8C917CF06D4471847DAEA3C2FAF058138AF22A947E15618C608F907634A" 14) "C5A8720EE5B1FE59AE245EBBCED277FA7C77ED9D6BD9AC0DDA3B80E931AD457C" 15) "www.ru" 16) "50E28602A91A9895B5798AB5E1C3423C04E7F75F01021498166AFAE8760A43A3" 17) "A7865343C6CC77353BCB66FFFA04AC8FD0C4253E95A44DB55D0DD06B7C7FCD75" 18) "C4696EEC08F5B1641B6CA7CD32580D281B85DD002A86CA3EC77C171A18339795"

Thank you in advance!

wcawijngaards commented 5 years ago

We do not currently have a need for unhashed storage, and we have other users that, I think, need hashed storage. So I would appreciate patches, and contributions, but for this case, it would need to be optional, so that it could operate in hashed or unhashed mode.

iruzanov commented 5 years ago

Hello Wouter!

Actually, now i would like to use unhashed mode just for my own needs. And currently it looks like: 1) "\tamazon-mx\x06amazon\x03com" 2) "\x03www\x02ru" 3) "\x03www\x06yandex\x02ru" 4) "\x03www\x02ee" 5) "\x03www\x03com" 6) "\x0bamazon-smtp\x06amazon\x03com" 7) "\x03www\x06amazon\x03com" 8) "\x05mail5\x06yandex\x02ru" 9) "\x0bamazon-mail\x06amazon\x03com"

The main reason of unhashed mode for me - is possibility to handle the keys from some frontend (Apache + SQL) for example to change TTL of domains (the keys in redis context), stored in redis. And currently i get the following workaround: static void calc_hash(struct module_qstate qstate, char buf, size_t len) { uint8_t clear[1024]; size_t clen = 0; uint8_t hash[CACHEDB_HASHSIZE/8]; const char hex = "0123456789ABCDEF"; const char secret = qstate->env->cfg->cachedb_secret ? qstate->env->cfg->cachedb_secret : "default"; size_t i;

    /* copy the hash info into the clear buffer */
    if(clen + qstate->qinfo.qname_len < sizeof(clear)) {
            memmove(clear+clen, qstate->qinfo.qname,
                    qstate->qinfo.qname_len);
            clen += qstate->qinfo.qname_len;
    }
    if(clen + 4 < sizeof(clear)) {
            uint16_t t = htons(qstate->qinfo.qtype);
            uint16_t c = htons(qstate->qinfo.qclass);
            memmove(clear+clen, &t, 2);
            memmove(clear+clen+2, &c, 2);
            clen += 4;
    }
    //if(secret && secret[0] && clen + strlen(secret) < sizeof(clear)) {
    //      memmove(clear+clen, secret, strlen(secret));
    //      clen += strlen(secret);
    //}

    /* hash the buffer */
    //secalgo_hash_sha256(clear, clen, hash);
    //memset(clear, 0, clen);

    /* hex encode output for portability (some online dbs need
     * no nulls, no control characters, and so on) */
    //log_assert(len >= sizeof(hash)*2 + 1);
    //(void)len;
    //for(i=0; i<sizeof(hash); i++) {
    //      buf[i*2] = hex[(hash[i]&0xf0)>>4];
    //      buf[i*2+1] = hex[hash[i]&0x0f];
    //}
    //buf[sizeof(hash)*2] = 0;

    for(i=0; i<clen; i++) {
            buf[i] = clear[i];
    }
    buf[clen] = 0;
    memset(clear, 0, clen);

}

wcawijngaards commented 5 years ago

Hi,

The patch has an issue. It looks good for having an option to enable the piece of code you commented out. But the qinfo.qtype and qinfo.qclass are added after the query name. And in your list of examples, I see the string has binary stuff in it and also ends at the zero after the query name. But after that are 4 bytes with type and class and those are necessary for the lookup to work. Otherwise AAAA lookups could fetch A answers or the other way around.

It would be easy for me to make a suggested patch that encodes the strings as "domain.name.example.com. IN AAAA" or something like that. Or even "domain.name.example.com. IN AAAA <secret from config if not "">".

iruzanov commented 5 years ago

Ok, Wouter! I have fully understood your idea. Big thanks for your thoughts. I will try to rewrite patch taking into account your important tips and share the patch with you later.

wcawijngaards commented 5 years ago

Here is a quick setup for the diff you could use, instead?

index 08389a43..2c16ae8a 100644
--- a/cachedb/cachedb.c
+++ b/cachedb/cachedb.c
@@ -329,6 +329,21 @@ calc_hash(struct module_qstate* qstate, char* buf, size_t len)
                qstate->env->cfg->cachedb_secret : "default";
        size_t i;

+       if(1) {
+               int w = 0;
+               char* s = buf;
+               size_t slen = len;
+               uint8_t* d = qstate->qinfo.qname;
+               size_t dlen = qstate->qinfo.qname_len;
+               w += sldns_wire2str_dname_scan(&d, &dlen, &s, &slen, NULL, 0);
+               w += sldns_str_print(&s, &slen, " ");
+               w += sldns_wire2str_class_print(&s, &slen, qstate->qinfo.qclass);
+               w += sldns_str_print(&s, &slen, " ");
+               w += sldns_wire2str_type_print(&s, &slen, qstate->qinfo.qtype);
+               verbose(VERB_ALGO, "cachedb uses hash '%s'", buf);
+               return;
+       }
+
        /* copy the hash info into the clear buffer */
        if(clen + qstate->qinfo.qname_len < sizeof(clear)) {
                memmove(clear+clen, qstate->qinfo.qname,
iruzanov commented 5 years ago

Cool code, thanks! If you don't mind i would like to enclose this code in separate small procedure. And call this procedure if the special configuration option is set.

wcawijngaards commented 5 years ago

Yes! That would be good.