On Tue, Nov 1, 2016 at 1:32 PM, George Kadianakis desnacked@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_...
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.