> 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. > > Flavio Percoco wrote: > 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 > > Kenneth Giusti wrote: > The _simplest_ approach is to break the pn_messenger() method's API. If > the user doesn't supply a name, fail. > > The best choice of a name should be left up to the application, which is > the only place that can determine what 'uniqueness' and 'traceability' mean > for a given application. > > IMHO the whole purpose of this patch is to remove what I consider a > needless hard dependency. Currently, if the user does not have the uuid > development library installed, they cannot build proton-c. Dead stop. > Worse, the component that has the libuuid dependency is _optional_ > (messenger). Folks who want to use the reactive api or the engine are > subject to a hard dependency that they have to address even though they're > not really subject to it. > > Sure, the libuuid library is pretty much ubiquitous on newer distros, but > the same cannot be said for the _headers_. So a user who wishes to build > proton better have these installed, either by having admin rights or by > downloading and installing locally the headers. > > Does this inconvenience make up for the convenience of being able to call > pn_messenger() without providing a well thought out name? And, in the > process getting a name that has really _no_ traceability/debugability merits? > > I'd say break the pn_messenger() call. Make the users _think_ about what > a good container name should be - such as an IP address or DNS name. They > will be better off for it.
This is turning into a massive bikeshed discussion. I say do whatever is the easiest to get rid of libuuid (home-grown UUID, random numbers, PIDs, name of your favourite song, "this-container-has-no-id", whatever) but *don't* break the API . Anyone who cares can provide a better solution later. I'm pretty sure *nobody cares*. Ken's right that no default would have been a better API to begin with, so why not break the API now? Because people don't bother specifying a name, not because they depend on UUID uniqueness or traceability, but because *they do not know what it is for and they quite rightly do not care!"*. - Alan ----------------------------------------------------------- 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 > >
