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