[squid-dev] [PATCH] convert digest_nonce_h to MEMPROXY_CLASS

[Kinkie]

On Sun, Sep 27, 2015 at 12:51 PM, Amos Jeffries <squid3 at treenet.co.nz> wrote:
> On 27/09/2015 10:30 p.m., Kinkie wrote:
>> .. and remove one XXX in the process.
>> Not much to add besides the subject.
>>
>> build- and lightly run-tested, seems to be ok.
>> Big-context patch attached.
>>
>
> Two big problems stand right out.
>
> * The static random and distribution objects were specificaly internal
> to a function in order so that they got initialized on first use, but
> did not get initialized if never used.
>
> That is somewhat important to a) save the machine available entropy bits
> for other useswhen Digest is not configured, and b) decrease the
> probability that the workers initialize them within the same second -
> they only get initialized on first traffic request handled, not during
> the startup second. And so the seeding can survive fork()'ing.

Whoops, I had misinterpreted the intention in this code as stated in
the first comment in authenticateDigestNonceNew. Is that not
intentional?
But if the problem is unique seeding, isn't it also useful to mix time
with a known-unique quantity, e.g. the PID?
In the meantime I've moved these data members as static members of
digest_nonce_data::refreshRandomData() and referenced that from the
ctor .

> * the contents of authenticateDigestNonceNew() should be in the nonce
> object ctor, or at least a method called by it.

Are you sure? It seems to me that it's actually the other way around:
authenticateDigestNonceNew is the only place where digest_nonce_data
gets instantiated, I've factored the POD instantiation code out of
digest_nonce_data to the ctor. The code could certainly be improved,
e.g. by moving the nonces registry and the nonce base64 encoding as a
static member of digest_nonce_h . It may be argued that the comment
block in authenticateDigestNonceNew could be moved elsewhere, I don't
really know what could be the best location for it tho.

> * the contents of authenticateDigestNonceDelete() should be the nonce
> objects dtor.

Not unless we move the cache in the nonce_data class. It's doable but
I consider it out of scope for this effort. I've added an XXX about
that.

Updated patch attached.

-- 
    Francesco
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mempool-nozero-digest_nonce_h-v2.patch
Type: text/x-patch
Size: 11717 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150927/f1bfdbdb/attachment.bin>