node-minecraft-protocol icon indicating copy to clipboard operation
node-minecraft-protocol copied to clipboard

Published 20 hours ago • verified publisher icon PrismarineJS

fix: wrap base64-encoded PEM with 64-char line boundary

Open littledivy opened this issue 11 months ago • 7 comments

According to RFC7468:

Generators MUST wrap the base64-encoded lines so that each line consists of exactly 64 characters except for the final line, which will encode the remainder of the data (within the 64-character line boundary), and they MUST NOT emit extraneous whitespace.

Parsers can avoid branching and prevent timing sidechannel attacks. Ref https://arxiv.org/pdf/2108.04600.pdf

Fixes compatibility with Deno as it enforces stricter handling of PEM.

littledivy avatar Mar 14 '24 11:03 littledivy

how did you check this improve something?

rom1504 avatar Mar 17 '24 14:03 rom1504

Hi @littledivy if nobody can verify that fix solve some issues, I think we should close this PR

rom1504 avatar Jun 02 '24 16:06 rom1504

Hey @rom1504 this solves Deno compatibility. Without this fix, the node-minecraft-protocol library would only work with the NodeJS runtime and not others that have properly implemented RFC7468 (i.e. Deno). I've verified that this is a blocker towards that goal.

ozaner avatar Jul 24 '24 22:07 ozaner

What example were you able to run with deno?

rom1504 avatar Jul 25 '24 02:07 rom1504

It may be the standard length, but for Minecraft we'd likely want to match the encoding behavior to be like vanilla Minecraft otherwise it's one of the areas that can be easily flagged by server software (like anticheat, antibot, etc).

I'm looking into the vanilla behavior here.

extremeheat avatar Jul 25 '24 09:07 extremeheat

I see. Honestly, even though I didn't make this PR, I think it should be closed.

The main purpose of this PR was to enable Deno support, but since changing these widths seems to go against the actual Minecraft protocol, and since more work beyond changing these widths would probably need to be done to make it compatible with Deno, it doesn't seem worth it.

A full on proper port of this package is probably necessary for anyone wanting to use it with Deno (or better Deno-Node compatibility) which is definitely not in the scope of this project.

Thanks @extremeheat for helping to investigate this though.

ozaner avatar Jul 25 '24 11:07 ozaner

No, what I said was that only the client PEM encoder method needs to be changed, and that changing it here would have no effect on anything (beyond maybe fixing Deno). The width doesn't matter to nmp as long as it can be read by the crypto call.

On the server code there is nothing to change

extremeheat avatar Jul 26 '24 00:07 extremeheat