On Tue, Nov 1, 2016 at 1:32 PM, George Kadianakis <desnac...@riseup.net> wrote: > I worked some more on the descriptor part of client authorization and > prepared a torspec patch. You can find it at `prop224_client_auth_2` in > my repo: > > https://gitweb.torproject.org/user/asn/torspec.git/commit/?h=prop224_client_auth_2&id=a85ffa341cc6c493ad031972f466a6dd5d8510e2 > > Based on received feedback, I went with the double-layer encryption > style, where the first layer is encrypted using the HS pubkey and the > second layer is encrypted using the descriptor cookie. Inside the first > layer plaintext is the information for authorized clients to derive the > descriptor cookie. > > I also included the padding suggestions from my previous post that > should help with hiding the number of client auths.
Hi, George! This looks like solid stuff. I'll try to answer your questions and > I'd like feedback on the following: > > a) Do you like the descriptor format and logic? Can we make it nicer or > easier to implement? No objections. > b) Do you like the proposal format? Is it messy and/or hard to > understand? Ideas on how can it be improved? I think it's fine. > I think we have reached the point where every subsystem of prop224 is > complex enough to warrant its own proposal, but I'm resisting the > urge to dig into this rabbithole right now. Maybe we can clean it all up when we turn it from "prop224" to "rend-spec-v2.txt" ? :) > c) Is the descriptor cookie encryption format good enough Namely: > encrypted_cookie = STREAM(iv, client_auth_cookie) XOR descriptor_cookie I don't much care for the lack of a MAC here. I haven't found any actual vulnerability here, but every time in my life that I have omitted a MAC from a malleable ciphertext, I have turned out to regret it. > d) Current changes: I changed "authentication-required" to > "intro-auth-required" in the descriptor, to make it more clear its > about introduction-layer authentication. > > Feedback on the patch and the above points is very much welcome! > > BTW, I'm not done with this thread yet, there are still some more points > that need to be handled wrt client authorization. But this spec patch is > the most important and lengthiest of them all, so let's get it out of > the way first. So, here's my feedback on the branch itself. wrt 3da540606a85e6 "Make subcredential actually change every time period." This change is safe, I think, but not necessary: Note that the blinded_public_key input already changes every time period because of the nonce value N used in blinding the public key. wrt a85ffa341cc6c4 "Use per-client desc auth keys" What is the format of "IV" and "client-id" and "encrypted-cookie"? Base64? How long are they? I would guess "base64-encoded, 32 bytes each"? The IV has to be random, right? The spec ought to say so, but I didn't see where. Malleability on the encrypted descriptor_cookie bothers me for some reason I can't figure out; see note above. The syntax on "superencrypted" doesn't seem right. The "encrypted-string" part should probably be after an NL, not an SP, right? Descriptor-cookie needs to be random each time, yeah? Does the spec say so? In all, this looks fine to me. I like the part where we do two layers of encryption unconditionally. -- Nick _______________________________________________ tor-dev mailing list tor-dev@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev