Whilst implementing v3 hidden services myself I found some inconsistencies between the specs and the current implementation. I wanted to share these in case someone from the Tor organization wants to update the specs and/or the implementation.
# rend-spec-v3.txt
## 2.4
* after decrypting the `superencrypted' object from a descriptor, the resulting document does not end with the NL character. This means that it does not strictly conform to the document meta-format described in section 1.2 of dir-spec.txt.
## A.2
* the blinded key param is defined as H(BLIND_STRING | A | s | B | N). In practice I found that I had to add a null byte after BLIND_STRING in order to reach the same value as the C implementation:
param = H(BLIND_STRING | INT_1(\x00) | A | s | B | N)
In all other cases where a string constant is used like this (e.g. computing the nonce N above), I found that the trailing null byte is not required.
* when clamping the blinding factor, the second bitwise operation is `param[31] &= 127' in the spec but `param[31] &= 63' in the C implementation. These are equivalent in practice when followed by the third operation (`param[31] |= 64'), but it might be nice to use a consistent representation for the benefit of human readers.
# 220-ecc-ids-keys.txt
# 2.1
* 'The signature is formed by signing the first N-64 bytes of the certificate prefixed with the string "Tor node signing key certificate v1".' I found this to be false; the signatures only validate without the string prefix.
## A.1
* I realized that the certificate types here are outdated. The signing-key extension is listed as type [04], when in rend-spec-v3.txt and the C implementation it is type [08].
inkylatenoth inkylatenoth@protonmail.com writes:
Whilst implementing v3 hidden services myself I found some inconsistencies between the specs and the current implementation. I wanted to share these in case someone from the Tor organization wants to update the specs and/or the implementation.
Hello inkylatenoth!
That's a great post and thanks for catching all these issues and innacuracies! We are definitely interested in consistency and fixing the spec (and implementation if needed).
# rend-spec-v3.txt
## 2.4
- after decrypting the `superencrypted' object from a descriptor, the resulting document does not end with the NL character. This means that it does not strictly conform to the document meta-format described in section 1.2 of dir-spec.txt.
Hmm... This might be worth fixing on the implementation if possible (and if it won't break things). Otherwise, let's patch the spec.
## A.2
the blinded key param is defined as H(BLIND_STRING | A | s | B | N). In practice I found that I had to add a null byte after BLIND_STRING in order to reach the same value as the C implementation:
param = H(BLIND_STRING | INT_1(\x00) | A | s | B | N)
In all other cases where a string constant is used like this (e.g. computing the nonce N above), I found that the trailing null byte is not required.
Ouch. This might be an artifact of the way strings are implemented in C.
I guess a spec patch might be the right thing to do here, otherwise too much stuff will break.
- when clamping the blinding factor, the second bitwise operation is `param[31] &= 127' in the spec but `param[31] &= 63' in the C implementation. These are equivalent in practice when followed by the third operation (`param[31] |= 64'), but it might be nice to use a consistent representation for the benefit of human readers.
Hmm... Yeah there are various ways to do the clamping for ed25519 keys.
I think we should edit the spec to reflect the clamping we do on the code.
# 220-ecc-ids-keys.txt
# 2.1
- 'The signature is formed by signing the first N-64 bytes of the certificate prefixed with the string "Tor node signing key certificate v1".' I found this to be false; the signatures only validate without the string prefix.
Ouch... I think we should edit the spec and consider if there are any security risks here.
## A.1
- I realized that the certificate types here are outdated. The signing-key extension is listed as type [04], when in rend-spec-v3.txt and the C implementation it is type [08].
Let's fix the spec here too...
---
Inkylatenoth, let me know if you are interested in drafting a spec/code patch for the issues you found!!! If you are not interested, I can try to do them myself at some point in the next weeks (been pretty busy with stuff lately).
Also, let us know if your independent implementation is a public thing we should know about. Seems interesting :)
Thanks again!
On 29 Oct 2017, at 01:30, George Kadianakis desnacked@riseup.net wrote:
# 220-ecc-ids-keys.txt
Is this the latest version of the ECC ID specification? Usually, our proposals are integrated into the main spec documents after they are implemented.
# 2.1
- 'The signature is formed by signing the first N-64 bytes of the
certificate prefixed with the string "Tor node signing key certificate v1".' I found this to be false; the signatures only validate without the string prefix.
Ouch... I think we should edit the spec and consider if there are any security risks here.
One security risk is that signatures on these certificates are re-usable in other contexts. For example, if two different parts of the Tor code believe signed certificates without prefixes, an adversary can take a certificate signed for one of them, and pass it to the other.
## A.1
- I realized that the certificate types here are outdated. The
signing-key extension is listed as type [04], when in rend-spec-v3.txt and the C implementation it is type [08].
Let's fix the spec here too...
This should definitely be integrated into one of the main specs.
T
On Sat, Oct 28, 2017 at 05:30:51PM +0300, George Kadianakis wrote:
That's a great post and thanks for catching all these issues and innacuracies! We are definitely interested in consistency and fixing the spec (and implementation if needed).
No problem! I'm glad you found my post helpful.
# rend-spec-v3.txt
## 2.4
- after decrypting the `superencrypted' object from a descriptor, the resulting document does not end with the NL character. This means that it does not strictly conform to the document meta-format described in section 1.2 of dir-spec.txt.
Hmm... This might be worth fixing on the implementation if possible (and if it won't break things). Otherwise, let's patch the spec.
I think it should be possible to fix this in the implementation without breaking anything.
# 220-ecc-ids-keys.txt
# 2.1
- 'The signature is formed by signing the first N-64 bytes of the certificate prefixed with the string "Tor node signing key certificate v1".' I found this to be false; the signatures only validate without the string prefix.
Ouch... I think we should edit the spec and consider if there are any security risks here.
I agree with all of your proposed solutions (spec vs implementation) except for here, where I would be much more comfortable with an implementation change. I realize it would be a breaking change, however, and will understand if you decide to update the spec instead.
Inkylatenoth, let me know if you are interested in drafting a spec/code patch for the issues you found!!! If you are not interested, I can try to do them myself at some point in the next weeks (been pretty busy with stuff lately).
At the moment I don't have the time to submit patches, sorry. If you find the time yourself then I'd be pleased to review/test your patches to confirm that they solve the inconsistencies I found.
Also, let us know if your independent implementation is a public thing we should know about. Seems interesting :)
I'm not implementing the full protocol, but I certainly will do when it's finished and open-sourced :)