Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BEP 44: Signature verification algorithm is invalid if the data is not a string #152

Open
jcea opened this issue Apr 6, 2024 · 4 comments

Comments

@jcea
Copy link

jcea commented Apr 6, 2024

In section "Signature Verification", the data to check the signature is described as

Concatenate ("4:salt" length-of-salt ":" salt) "3:seqi" seq "e1:v" len ":" and the encoded value.

The result of that is only valid if the "encoded value" is actually a string, since "1:vLEN:DATA" is valid only if data is a string.

For example, what if DATA is a dictionary?. The resulting bencoding build by hand as suggested in the BEP would be invalid.

Checking my implementation in the mainline DHT, the algorithm actually implemented (at least by UT) seems to be:

("4:salt" length-of-salt ":" salt) "3:seqi" seq "e1:v" and the encoded value

Notice the lack of the "len" value.

If the encoded value was a string, the encoding will start with "LEN:", exactly as described by the standard. But the suggested change allows to encode more sophisticated structures, like dictionaries (as tested in the wild).

@jcea jcea changed the title BEP 44: Signatire verification algorithm is invalid if the data is not an string BEP 44: Signature verification algorithm is invalid if the data is not an string Apr 6, 2024
@jcea jcea changed the title BEP 44: Signature verification algorithm is invalid if the data is not an string BEP 44: Signature verification algorithm is invalid if the data is not a string Apr 6, 2024
@jcea
Copy link
Author

jcea commented Apr 6, 2024

Moreover, the verification described and the test vectors don't agree. For instance, if data is "Hello World!", the test vectors are described as:

3:seqi1e1:v12:Hello World!

But according to the verification algorithm, the required (but wrong) data would be:

3:seqi1e1:v15:12:Hello World!

@the8472
Copy link
Contributor

the8472 commented Apr 6, 2024

Yeah, the operation could be better described as building a bencoded dictionary with a raw (non-encoded) value for the v key and then removing the d prefix and e suffix from the dictionary.
At least that's how I have implemented the encoding side

@jcea
Copy link
Author

jcea commented Apr 7, 2024

I think that more test vectors should be added, specifically more complex datatypes like dictionaries, lists, etc.

I am publishing complex mutable structures to current mainline DHT and current explanation in the BEP is not correct for complex datatypes, neither follows current implementations in the wild.

@jcea
Copy link
Author

jcea commented Apr 7, 2024

For example, I am correctly publishing this to the mainline DHT, both on libttorrent clients and UT:

{'kAFKA': 'Hello World!', 'v': 'En un lugar de La Mancha'}

The signed data must be:

3:seqi2e1:vd5:kAFKA12:Hello World!1:v24:En un lugar de La Manchae

Note that this works in practice BUT it is not the prescriptive algorithm described in the BEP and the BEP algorithm, as describes, simply can't encode such a data structure.

May I suggest that that section in the BEP must be redone and more complex test vectors should be provided?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants