[tor-dev] Revisiting prop224 client authorization

[Nick Mathewson]

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