> On June 9, 2015, 4:03 p.m., Andrew Stitcher wrote: > > proton-c/src/platform.c, line 77 > > <https://reviews.apache.org/r/35252/diff/2/?file=981447#file981447line77> > > > > My very limited knowledge of cryptographic code tells me that the > > rand() family chould **never** be used, as they aren't very good - where > > did you get this code from? > > > > Can we have someone who know something about cryptography check this > > out? > > Flavio Percoco wrote: > You make a very good point about cryptography and we having this custom > implementation. The reason I feel comfortable with this is because it's just > being used to generate the messenger name but having it in the source code > means that someone might use it in the future. This is exactly the reason why > I think we shouldn't be using UUID's at all to generate the messenger name > but just some random number. That said, considering this is an UUID4 - as > also explicitly state by the function name itself - I think it doesn't matter > much. The UUID4 is based on truly-random or pseudo-random numbers generation > as stated in the RFC. > > http://tools.ietf.org/html/rfc4122#section-4.4 > > Kenneth Giusti wrote: > He got this code from me. > > We don't need a cryptographically secure generator - at least for our > usage. The only place this is used is messenger, and its used to pick a > name for the container IFF the application fails to provide one. > > So let's move this function to messenger.c and call it > a_somewhat_random_name(). > > Alan Conway wrote: > Why are we using UUIDs at all? Just barf if the user doesn't supply a > name, or use a simple global atomic counter to number them. What, if any, is > the actual uniqueness requirement for messenger names? Is it ever passed > outside the process? > > Andrew Stitcher wrote: > The uuid is used as the local container name I think (sent to the peer) > and so does actually need to be globally unique. > > Alan Conway wrote: > The spec says nothing (that I can find) about container-id being unique. > All it says is that it is a string and it SHOULD be used for "traceability" > and "identification". I don't think it is or can be used for any form of > automated routing as presently specified, only for human-readable debugging > purposes. In which case a UUID is the worst default. Perhaps > IP-address:PID:number would be a nice helpful thing to use instead. > > Alan Conway wrote: > Lets not forget, we are providing a *default* for a highly underspecified > and AFAIK almost entirely unused name that the user is free to set themselves > if they care about it. It really doesn't matter what we do, so it should be > simple.
The very very very first patch I had proposed removing UUID completely in favor of some random, number-based, name generation... but again I preferred to take small steps forward. As Alan said in the previous review "be agressive, beee agressive". The reason I'm bringing the above up is because I think that providing something that is unique enough to make requests traceable should be ok (and that's certainly not just a random number but something like Alan proposed). However, since this patch is already here and the uuid like name has been implemented already, I'd probably give this a go first. The reason I'd prefer this to land instead of something in the form of "IP-address:PID:number" is that I'd expect people to start making assumptions on this new form and the information it carries, then some apps might even end up relying on it for some operations that they shouldn't and that ends up in a mess. I'd rather have a generated ID that provides no information whatsoever about the node that it was generated in since that information should not be used at al and it may change if users decide to provide a name. That said, we could create a hash out of the format Alan's proposed... :D - Flavio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35252/#review87213 ----------------------------------------------------------- On June 9, 2015, 5:37 p.m., Flavio Percoco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35252/ > ----------------------------------------------------------- > > (Updated June 9, 2015, 5:37 p.m.) > > > Review request for qpid, Alan Conway, Chug Rolke, Gordon Sim, and Rafael > Schloming. > > > Repository: qpid-proton-git > > > Description > ------- > > Instead of relying on libuuid for uuid generation, let proton-c have a > built-in uuid4 generator to do this job. > > > Diffs > ----- > > proton-c/CMakeLists.txt b534e86 > proton-c/bindings/python/setup.py 79168d2 > proton-c/src/messenger/messenger.c f226f7b > proton-c/src/platform.h 6962493 > proton-c/src/platform.c 8f8ac5f > > Diff: https://reviews.apache.org/r/35252/diff/ > > > Testing > ------- > > > Thanks, > > Flavio Percoco > >
