On 06 Nov (22:35:32), meejah wrote:
David Goulet dgoulet@ev0ke.net 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]) CRLFNew "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@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev