cocasse / phantom Goto Github PK

Rather than using RC4 in your PRNG, consider using AES in CTR mode to reduce 
your trusted base. CTR mode allows using the same cipher as is used elsewhere 
in your design (AES) in places where you need a stream cipher. As long as your 
key and IV are sufficiently unpredictable, it is considered secure.

Using git as your version control system has several benefits. One is enabling 
more distributed development, but I feel the more important one is that the 
commit history in git is inherently a hash chain all the way to the first 
commit, and both tags and pull requests can be signed. This should provide some 
additional protection against possible future attempts to sneak anything 
malicious in.

The current design has the hash in the setup package stored as a plain byte 
array. I presume that the plan is to support longer hashes in the future by 
checking the length of the array, but this has one major flaw.

Presume that a SHA-3 algorithm is selected. It will likely be reccommended for 
widespread use shortly thereafter. If a subsequent flaw is found, any 
replacement chosen or tweaks applied will keep the API invariant, *including* 
the digest length. The versioning inherent in protocol buffers will not help 
because the current protocol is insufficiently self-descriptive and has 
unwarranted assumptions. I would suggest including some sort of identifier of 
which hash is in use.

The vanilla Kademlia protocol is weak against certain classes of attacks which 
may be relevant for this use case. These are elaborated on in a paper that 
introduces S/Kademlia, which tries to mitigate them.

Of particular interest may be the measures they take to avoid adversarial 
routing in the DHT by way of multiple disjoint lookups. I have attached the 
relevant paper.

What steps will reproduce the problem?
1. checkout the read-only repo

setuppackage.pb-c.[h|c]
kademlia.pb-c.[h|c]

are missing.

Not a bug, just a small patch. Replaces strcpy with strncpy in a couple places.

Compiles cleanly, but not tested. Copyright dedicated to original authors.

This is not a bug, but a proposed patch.

This patch concerns certificate-related code in helper.c, kad_contacts.c, and 
x509_flat.c.

Lines 314,363, and 500 fix present bugs: uninitialized memory, memory leak, 
NULL pointer dereference.

The rest is hardening against future potential memory bugs. Calls to malloc() 
are hardened against potential integer overflows. Whenever possible, 
free_X509_flat() should now be used without hesitation (no fear of double-free, 
etc.), and all related pointers should be initialized to NULL.

Compiles cleanly, but not tested.

Copyright dedicated to the original authors of phantom.

Following the instructions in the README, I get the following error when I try 
to compile.

main.c: In function ‘main’:
main.c:137:18: error: variable ‘tun’ set but not used 
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors

Perhaps -Werror=unused-but-set-variable should be turned off? 

I fully understand that using anything other than standard TLS/SSL would make 
it easier for the protocol to be fingerprinted and potentially blocked. 
FOrtunately, another project with a completely different aim has already worked 
on the idea of transporting unreliable message-based protocols efficiently on 
top of reliable stream-based protocols including TLS/SSL, without making it 
possible to dinstinguish such behavior by means other than traffic flow 
analysis. I recommend looking at http://dedis.cs.yale.edu/2009/tng/ - 
especially their paper "Minion: Unordered Delivery Wire-Compatible with TCP and 
TLS" and the drafts "Improving OpenSSL to Process Out of Order Data" and 
"Unordered Delivery in TLS-Encrypted Connections".

I recommend looking at libnl-route. It should be much easier than speaking the 
netlink protocol directly, and much less fragile than calling out to ip. 
http://www.infradead.org/~tgr/libnl/doc/route.html