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

a modification suggest #1045

Closed
jackzhp opened this issue Aug 3, 2018 · 9 comments
Closed

a modification suggest #1045

jackzhp opened this issue Aug 3, 2018 · 9 comments
Labels
P3 Low priority
Milestone

Comments

@jackzhp
Copy link

jackzhp commented Aug 3, 2018

In the file onion_announe.c, in the handle_announce_request function, there are following lines:

if (crypto_memcmp(ping_id1, plain, ONION_PING_ID_SIZE) == 0
        || crypto_memcmp(ping_id2, plain, ONION_PING_ID_SIZE) == 0) {
    index = add_to_entries(onion_a, source, packet_public_key, data_public_key,
                           packet + (ANNOUNCE_REQUEST_SIZE_RECV - ONION_RETURN_3));
} else {
    index = in_entries(onion_a, plain + ONION_PING_ID_SIZE);
}

I hope that packet_public_key could be replaced with plain+ONION_PING_ID_SIZE. For tox, it does not change anything effectively. Since when the first case is executed, they are exactly same! In fact, when the first case is executed, the message being handled is sent by onion_client.c(method client_send_announce_request) and there for the case num==0 to call create_announce_request.

Even though effectively, this proposed change does not change anything for tox. But it provides some consistency: either cases, entry is identified by 32 bytes starts at plain + ONION_PING_ID_SIZE. Furthermore, with this modification, further improvement becomes possible.

With the above change, to distinguish setting up an entry from detecting an entry is purely by the pingid. Then I can use packet_public_key for some other purpose since it is not tied to identifier of the entry.

so please!

Jack

@iphydf
Copy link
Member

iphydf commented Aug 3, 2018

Feel free to make a Pull Request with the proposed change, so we can discuss it there.

@iphydf iphydf added this to the v0.2.x milestone Aug 4, 2018
@jackzhp
Copy link
Author

jackzhp commented Aug 11, 2018

In order to support multidevices, I suggest to implement it in two phases.

Multidevices mean a real PK with several DHT(facade) device PKs. Each DHT(facade) PK corresponds to one device, they share the same real PK.

Phase #1: change 0x83 & 0x84 messages, this modification makes effectively no change. I will explain later.
but would allow smooth transision to phase #2.
Phase #2: based on stage #1, add multi device support. change 0x83, 0x84, 0x85, 0x86 messages.

Phase #1:
To set up an onion entry(I prefer to call it to setup a worm hole):
0x83 + nonce24 + #0pk32 + encrypted120 while encrypted120 = pingid32 + pkTargetReal32 + pkWormHole32 + reqID8 + mac16.
Current implementation #0pk32 and pkTargetReal32 are same, and because of this, wherever pkTargetReal32 is needed, #0pk32 is used.
So let's change it to use pkTargetReal32 instead.
To query an onion entry(to detect whether exists a worm hole to the specified target):
0x83 + nonce24 + #0pk32 + encrypted120 while encrypted120 = pingid32(0) + pkTargetReal32 + pkWormHole32(0) + reqID8 + mac16.

Phase #2:
Replace pkTargetReal32 for epkTargetReal + deviceID
This change applies to 0x83, 0x84, 0x85, 0x86 messages, as follows.
for 0x83 message, its original format is shown in Phase #1, to support multidevices, its encrypted part could be redefined as
encrypted =format1(0) + pingid32 + epkTargetReal + deviceID + pkWormHole32 + reqID8 + mac16
encrypted =format1(1) + epkTargetReal + deviceID + reqID8 + mac16
it is easy to distinguish new format from old format since old format is 120 bytes exactly.

for 0x84 message, its current format is
0x84 + reqID8 + nonce24 + encrypted while encrypted = indicator1 + pingid_O_pkWormHole32 + nodes + mac16
originally, indicator1 =0,1,2 are used.
to support multidevices, indicator1 ==3,4,5,6 are defined:
encrypted = indicator1(3) + deviceID + pingid32 + nodes + mac16 to indicate not setup
encrypted = indicator1(4) + deviceID + pingid32 + nodes + mac16 to indicate setup
encrypted = indicator1(5) + numberOfDevices1 + deviceID + pkWormHole32 + deviceID + ... + nodes + mac16 to indicate not found
encrypted = indicator1(6) + numberOfDevices1 + deviceID + pkWormHole32 + deviceID + ... + nodes + mac16 to indicate found
when deviceID is null, then 0x84 returns all entries to multidevices of the same real PK.

for 0x85 message, its current format is
0x85 + pkRecvReal32 + nonce24 + pkRandom32 + #0encrypted
while #0encrypted = pkRealSender32+ #1encrypted + mac16
while #1encrypted = someData + mac16
in order to support multidevice pkRecvReal32 & pkRealSender32 should be replaced.
I suggest the following new format
0x85 + 0xFF + type1 + ....
0xFF indicates new format is in effect, very unlikely pkRecvReal32 starts with 0xFF
and type1==0 is defined
0x85 + 0xFF + type1(0) + epkRecvReal + deviceID + nonce24 + pkRandom32 + #0encrypted
while #0encrypted = epkRealSender + deviceID + #1encrypted + mac16

for 0x86 message, it follows from 0x85 message.

epkTargetReal is defined as follows:
type1 + pk
type1==0, pk is of sec256k1, epkTargetReal will be 1+33=34 bytes
type1==1, pk is of curve25519, epkTargetReal will be 1+32=33 bytes
512bits elliptic curve might be used in the future.
DeviceID is defined as follows:
type1 + utf8bytes
type1 means 1 byte to indicate how many bytes following.

any comments?

Jack

@jackzhp
Copy link
Author

jackzhp commented Aug 11, 2018

My original post was incomplete. I am sorry. And I did not mention what further improvement was planned. Further improvements are to support multidevices, and to support 512bits real(onion) PK or other type curve.

@jackzhp
Copy link
Author

jackzhp commented Aug 13, 2018

Furthermore,
for 0x9C message, its current format: 0x9C + tsSeconds8 + pkDHTsender32+nodes
I would like to suggest new format:
new format
0x9C + tsSeconds8 + 0xFF + type1 + ...
when type1==0
0x9C + tsSeconds8 + 0xFF + type1(0x00) + deviceID + epkDHTsender + nodes

I guess then the multiple devices support is complete.

One Onion_Friend(one tox ID) will be mapped to several DHT_Friends(one for each device, i.e. one for each Crypto_Connection).

@jackzhp
Copy link
Author

jackzhp commented Aug 13, 2018

A little bit more explanation.

Since I do not know how familiar readers are with the source code, let me put into two sections: section I --- single device, section II --- multidevices. Since before fully understands the mechanism of current single device, there is no point to talk about extension to multidevices.

suppose 2 users: user A with two devices(device_a1, device_a2), user X with 1 single device(device_x).

Section I --- single device.

device_x(of User X) announce the dhtpk which the user X wants to use. The user X should send this announce to user A. This announcement is the 0x9C message, which is wrapped into 0x85 message.
Now the detail: this 0x85 message is not sent to device_a(of User A), but to a point P which is "close" to PK_A. P then turns 0x85 message into 0x86 message and sends 0x86 message to user A.

Before device_x sends 0x85(0x9C) message to P, device_x asked P whether P can forward messages to user A. device_x asked P with message 0x83, P responded with message 0x84. P can respond with yes/no. If P responded yes, then user A must have had made the arrangement. the arrangement request is 0x83 message, and P responds with 0x84 message.

Now it is the time to investigate the structure/format of 0x83,0x84,0x85,0x86, and 0x9C messages in chronical order as follows.

User A arranges message forwarding with P with 0x83 message, user A has to send 0x83 message twice.
first time with pingid==0, 2nd time with right pindid. pindid is generated by P.
key parameter: the PK of user A(pkTargetReal, i.e. PK_A)
response 0x84: indicator==0 means not arranged, indicator==2 means arranged.

User X asks P whether P can forward messages to user A with 0x83 message
key parameter: the PK of user A(pkTargetReal)
response 0x84: indicator==0 means not able to forward, indicator==1 means being able to forward

user X announce its dhtpk with 0x9C message,
key parameter: dhtpk
for security reason, this message will be wrapped into 0x85 message.

User X sends 0x85 message to P
2 key parameters: PK of user A, X has to tell P where to forward
PK of user X, once user A received the message, the user A must know who is sending this.
By the way for security reason, 0x85 is wrapped into 0x80, 0x81, and 0x82(i.e. the onion) to reach P.

P processes 0x85 message, turns it to 0x86, and sends 0x86 to user A.
By the way 0x86 is sent to user A through 0x8C(0x8D(0x8E)) message.

Through the above process, the map between DHTPK and realPK(userID) is established.
When the user X wants to change DHTPK, the user X can go throught the above process again.

Section II -- multiple devices

In the section I, wheverver the userIDs take place, should be replaced for(with) userID and deviceID(a string will be good), and default single device is the case of empty deviceID. And wherever the dhtpk takes place, should be replaced for(with) dhtpk and deviceID.

With these replacement, the map between (deviceID,DHTPK) and (userID,deviceID) is established.

When user X asks P whether P can forward messages to user A, P responds with 0x84.
this 0x84 should tells P user X that P can forward messages to device_a1 and device_a2.

Hope what I presented is understandable.
Jack

@jackzhp
Copy link
Author

jackzhp commented Aug 13, 2018

@iphydf says:

   Hi Jack, can you formulate your proposal in the form of a design document? See https://toktok.ltd/designs for how to submit one for review. We can continue discussing the ideas on the pull request with the design doc.

If it is wiki, yes, I can participate. and I can learn a bit from example. Seems that I am not able to formulate a proposal in the form of a design document. I feel what I suggest is a simple thing(just a little extension), there is no need to have a design document.

I think that the little extension I proposed is easy to understand if you understand the mechanism to be extended. So maybe what is needed is an explanation of the original mechanism.

I can write a word or pdf document, or html document.

Jack

@iphydf
Copy link
Member

iphydf commented Aug 13, 2018

Please write a markdown document and make a PR into this repository with that document. Put the file into docs/ in this repository.

@jackzhp
Copy link
Author

jackzhp commented Aug 14, 2018

I hope someone can take this over, and I do not have to participate any more.
I do not pull since rarely succeeded because of the Great Fire Wall, and I am not using any VPN.

WormHole.docx

@iphydf iphydf added the P3 Low priority label Apr 27, 2020
@iphydf iphydf modified the milestones: v0.2.x, meta Feb 4, 2022
@iphydf
Copy link
Member

iphydf commented Feb 4, 2022

It's unclear to me where to go with this. #1108 is unsafe. Closing this for now. Reopen if you have specific things you want to change.

@iphydf iphydf closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low priority
Projects
None yet
Development

No branches or pull requests

2 participants