i2pd icon indicating copy to clipboard operation
i2pd copied to clipboard

Weird padding calculation for SSU2Session Token Request

Open webhive opened this issue 1 year ago • 6 comments

We have the following code: https://github.com/PurpleI2P/i2pd/blob/6439e227f67d88cb9bc6ac95c04b397faa148727/libi2pd/SSU2Session.cpp#L1151-L1152

Here is a CreatePaddingBlock function implementation: https://github.com/PurpleI2P/i2pd/blob/6439e227f67d88cb9bc6ac95c04b397faa148727/libi2pd/SSU2Session.cpp#L2640-L2650

So we have the flowing params which are rally a constants:

 size_t SSU2Session::CreatePaddingBlock (uint8_t * buf, size_t len = 18, size_t minSize = 1) 

And I see no way to this condition to mean something - it will never be called

if (len < 3 || len < minSize) return 0; 

Same for this block

size_t paddingSize = rand () & 0x0F; // 0 - 15 
// paddingSize + 3 can have maximum value 18 and with len = 7 this condition always false
 if (paddingSize + 3 > len) paddingSize = len - 3; 
// with minSize = 1 this condition always false
 else if (paddingSize + 3 < minSize) paddingSize = minSize - 3; 

Beside this weirdness it is really confusing why minSize value for padding block is 1. According Spec for TokenRequest https://geti2p.net/spec/proposals/159-ssu2#token-request-type-10 Padding block is mandatory (at least not marked as optional), but same time minimal payload size is 8.

It looks really contradictory - payload consist of DateTime block (7 bytes) and Padding block (at least 3 bytes). So minimal size should be 10 bytes at least. It mentioned there, what size can be zero, but zero IMO does not mean absence of size field.

Can someone clarify why it working such a tricky way?

PS: also why nothing specified in documentation about padding size should be 0-15? What is really a correct value?

webhive avatar Feb 16 '24 09:02 webhive

And even more. According https://geti2p.net/spec/proposals/159-ssu2#padding padding data should be random one, but in code we can see memset (buf + 3, 0, paddingSize); so it zero instead.

webhive avatar Feb 16 '24 09:02 webhive

You are right, minimal size of 1 it not required, it can be zero. Zero padding data is right, because only size matters.

orignal avatar Feb 19 '24 19:02 orignal

What about random vs zero padding?

webhive avatar Feb 20 '24 00:02 webhive

What about random vs zero padding? I mean filling of padding data.

webhive avatar Feb 20 '24 00:02 webhive

You don't need actual random data there, all zeros are fine.

orignal avatar Feb 20 '24 00:02 orignal

You don't need actual random data there, all zeros are fine.

I understand, but I mean difference between official specification and implementation. May be update specification then to avoid confusion.

webhive avatar Feb 20 '24 00:02 webhive