[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-dev] Proposal 284: Hidden Service v3 Control Port



On 06 Nov (22:35:32), meejah wrote:
> David Goulet <dgoulet@xxxxxxxxx> writes:
> 
> Hi David,
> 
> Overall looks good! A few comments inline:
> 
> >      "onions/{current,detached}"
> >        No change. This command can support v3 hidden service without changes
> >        returning v3 address(es).
> 
> Does the control-spec need a note pointing out that you might get some
> "longer" (v3) addresses?

Yes, once this proposal is merged to control-spec.txt, it will mention it for
sure what to expect.

> 
> > 3.1.3. ADD_ONION
> >
> >    For this command to support version 3, new values are added but the syntax
> >    is unchanged:
> >
> >      "ADD_ONION" SP KeyType ":" KeyBlob
> >                  [SP "Flags=" Flag *("," Flag)]
> >                  1*(SP "Port=" VirtPort ["," Target])
> >                  *(SP "ClientAuth=" ClientName [":" ClientBlob]) CRLF
> >
> >    New "KeyType" value to "ED25519-V3" which identifies the key type to be a
> >    v3 ed25519 key.
> >
> >    New "KeyBlob" value to support the new "ED25519-V3", if specified, will
> >    generate a new ed25519 private key.
> 
> This might need a couple more details; as-is ADD_ONION can take
> "NEW:BEST" (which should now return a v3 service?) or "NEW:ED25519-V3"
> for explicitly asking for a V3 key, or "ED25519-V3:<56 base32 chars>"
> for adding an already-existing v3 service.

Oh good point! I failed to notice that "RSA1024:<key>" was even possible.
Actually, it is not specified in the spec but the code expects this:

    "RSA1024:<Base64 Blob>" - Loading a pre-existing RSA1024 key.

Ok fun! I'll add this. Good catch! And control-spec.txt should be updated.

To be consistent then we could ask for a <Base64 Blob> as well:

    "ED25519-V3:<Base64 Blob>"

... which contains the ed25519 private key.

> 
> >    Because client authentication is not yet implemented, the "ClientAuth"
> >    field is ignored as well as "Flags=BasicAuth".
> 
> I think these should generate a 500-level error (if used for a v3
> service) instead of being ignored. That is, if you try to use auth with
> v3, you get an error.

Indeed.

I'm unsure between
    "512 Syntax error in command argument"

    "552 Unrecognized entity"
        [A configuration key, a stream ID, circuit ID, event,
         mentioned in the command did not actually exist.]

But overall yes!

> 
> >    For this event to support vesrion 3, one optional field and new
> >    values are added:
> 
> I echo Damian's comments on the positional-arg; making it [SP
> "DescriptorID=" ] or similar (i.e. an optional kwarg) would mean easier
> later extending and also it *should* then "just work" with most
> controller libs already at least as far as parsing goes (because
> controller libs in general have to accept new, unknown kwargs).

See other thread about this.

Big thanks!
David

> 
> The rest all sounds good to me!
> 
> thanks,
> meejah
> _______________________________________________
> tor-dev mailing list
> tor-dev@xxxxxxxxxxxxxxxxxxxx
> https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev

-- 
1ThD0Y7lJWfAN3qxos27iPGUdHQS5sZ4kMwlov3un5k=

Attachment: signature.asc
Description: PGP signature

_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev