Re: Bug#653838: Inadequate source of entropy in recursive queries: maradns
Julien,
Comments below. What is the next step?
On 12/01/12 21:40, Julien Cristau wrote:
> On Sun, Jan 1, 2012 at 17:52:21 +0000, Nicholas Bamber wrote:
>
>> Julien,
>> The attached file is a debdiff for 1.4.03-1.1 -> 1.4.03-1.2. I have not
>> run an FTBS test on it but I wanted to know if I was on the right lines.
>>
> Looks basically ok, there's a couple oddities but I guess they're that
> way upstream?
>
>> diff -u maradns-1.4.03/debian/copyright maradns-1.4.03/debian/copyright
>> --- maradns-1.4.03/debian/copyright
>> +++ maradns-1.4.03/debian/copyright
>> @@ -4,7 +4,7 @@
>>
>> Files: *
>> Copyright:
>> - (C) 2002-2010 Sam Trenholme <maradns@gmail.com>
>> + (C) 2002-2011 Sam Trenholme <maradns@gmail.com>
>> License: BSD license
>>
>> Files: debian/*
>> diff -u maradns-1.4.03/debian/changelog maradns-1.4.03/debian/changelog
>> --- maradns-1.4.03/debian/changelog
>> +++ maradns-1.4.03/debian/changelog
>> @@ -1,3 +1,9 @@
>> +maradns (1.4.03-1.2) stable; urgency=low
>> +
>> + * Applied patch to ensure adequate entropy (Closes: #653838)
>> +
>> + -- Nicholas Bamber <nicholas@periapt.co.uk> Sun, 01 Jan 2012 16:29:53 +0000
>> +
>> maradns (1.4.03-1.1) unstable; urgency=high
>>
>> * Non-maintainer upload by the Security Team
>> only in patch2:
>> unchanged:
>> --- maradns-1.4.03.orig/server/MaraDNS.c
>> +++ maradns-1.4.03/server/MaraDNS.c
>> @@ -3933,6 +3933,24 @@
>> int recurse_number_ports = 4096;
>> #endif
>>
>> + /* First order of business: Initialize the hash */
>> + if(mhash_set_add_constant(
>> +#ifdef MINGW32
>> + "secret.txt"
>> +#else
>> + "/dev/urandom"
>> +#endif
>> + ) != 1) {
>> + printf(
>> +#ifdef MINGW32
>> + "Fatal error opening secret.txt"
>> +#else
>> + "Fatal error opening /dev/urandom"
>> +#endif
>
> Shouldn't that go to stderr?
Actually the stdout gets piped into a related logger process.
>
>> + );
>> + return 32;
>> + }
>> +
>> memset(&client,0,sizeof(client)); /* Initialize ya variables */
>> clin = (struct sockaddr_in *)&client;
>> #ifdef AUTHONLY
>> only in patch2:
>> unchanged:
>> --- maradns-1.4.03.orig/libs/MaraHash.c
>> +++ maradns-1.4.03/libs/MaraHash.c
>> @@ -1,4 +1,4 @@
>> -/* Copyright (c) 2006 Sam Trenholme
>> +/* Copyright (c) 2006,2011 Sam Trenholme
>> *
>> * TERMS
>> *
>> @@ -32,6 +32,7 @@
>> #include "JsStr.h"
>> #endif
>> #include "MaraHash.h"
>> +#include <stdio.h>
>>
>> /* Masks to limit the size of the hash */
>> /* These are powers of two, minus one */
>> @@ -41,6 +42,8 @@
>> 16777215, 33554431, 67108863, 134217727,
>> 268435455, 536870911, 1073741823 };
>>
>> +mhash_offset mhash_secret_add_constant = 7;
>> +
>> /* Create a new, blank mhash object
>> input: none
>> output: pointer to the object in quesiton on success, NULL (0)
>> @@ -100,6 +103,7 @@
>> /* Simple enough hash */
>> while(point < max) {
>> ret += (mhash_offset)(*point << shift);
>> + ret += mhash_secret_add_constant;
>
> odd indent.
I hate the indenting in the souce code generally.
>
>> shift += 7;
>> shift %= hash_bits;
>> point++;
>> @@ -684,3 +688,23 @@
>> return tuple->tuple_list[element];
>> }
>>
>> +/* Read four bytes from a filename and use that as a secret add constant */
>> +int mhash_set_add_constant(char *filename) {
>> + FILE *read = 0;
>
> and odd choice of variable name.
You're worried about a clash between the variable name and unction
sysmbols? I guess this could be changed and sent upstream.
>
>> +
>> + read = fopen(filename,"rb");
>> + if(read == NULL) {
>> + return -1;
>> + }
>> +
>> + mhash_secret_add_constant ^= getc(read);
>> + mhash_secret_add_constant <<= 8;
>> + mhash_secret_add_constant ^= getc(read);
>> + mhash_secret_add_constant <<= 8;
>> + mhash_secret_add_constant ^= getc(read);
>> + mhash_secret_add_constant <<= 7;
>> + mhash_secret_add_constant ^= getc(read);
>> + fclose(read);
>> + return 1;
>> +}
>> +
>> only in patch2:
>> unchanged:
>> --- maradns-1.4.03.orig/libs/functions_MaraHash.h
>> +++ maradns-1.4.03/libs/functions_MaraHash.h
>> @@ -39,3 +39,5 @@
>> */
>> void *mhash_undef(mhash *hash, js_string *key);
>>
>> +/* Read four bytes from a filename and use that as a secret add constant */
>> +int mhash_set_add_constant(char *filename);
>
> Cheers,
> Julien
--
Nicholas Bamber | http://www.periapt.co.uk/
PGP key 3BFFE73C from pgp.mit.edu
Reply to: