MightyPirates / OpenComputers

Home of the OpenComputers mod for Minecraft.
https://oc.cil.li
Other
1.59k stars 430 forks source link

[Feature Request] Signed Bytecode Loading #2048

Closed SoniEx2 closed 6 years ago

SoniEx2 commented 8 years ago

string.dump() should be replaced with a function that signs the resulting bytecode. load should verify bytecode signatures.

The signature should be generated server-side. There's no need to have more than one signature per server. The whole signature process should be done in Java/Scala so as to not leak the key. The key should be loaded into a byte[] every time it is to be used, and the byte[] should be cleared (set to 0) every time after use, so as to not leak the key. This byte[] should be global and synchronized, to reduce the risk of leaking the key.

This lets you get all the benefits of bytecode (smaller size, faster loading, etc) without any of the drawbacks (because only bytecode produced by the server can be loaded).

SoniEx2 commented 8 years ago

Some additional notes: Signing may be overkill. HMAC might also work just fine. Can also use Scrypt or something, but that might be about as overkill as Ed25519 signatures...

Not sure if there is a concept of "overkill crypto" tho. You'd think stronger crypto is always better...

magik6k commented 8 years ago

Just encrypting Sha hash with AES would be enough. As for security considerations I'm not actually sure, but just appending signature to the blob looks good enough to me.

SoniEx2 commented 8 years ago

@magik6k HMAC is simpler as it only requires a hash function.

Also who even rolls their own crypto (MAC in this case)? D:

SoniEx2 commented 8 years ago

Am I the only one who cares about this?

payonel commented 8 years ago

Actually I like this idea, no need to bump, but I'll add feature suggested, and i'll let sangar decide if it is feature accepted/open-for-adoption

fnuecke commented 8 years ago

I suppose that could work, in principle. Is it worth it though? What are the actual, practical use-cases?

SoniEx2 commented 8 years ago

Saves space, loads faster (might wanna compile the OpenOS .lua files when installing - and/or add a cache to it, altho that'd be much more complex), etc.

The EEPROMs would get to fit a bit more Lua code than they currently do, I think. Assuming they still work the same way - I haven't played this mod in ages...

payonel commented 8 years ago

I'd use it for swap space memory

SoniEx2 commented 8 years ago

Ok so, assuming HMAC:

If string: (use cases: stored procedures or something)

  1. Check if the string describes a binary chunk.
  2. Split the string into "data" and "hmac".
  3. Compute the hmac over "data".
  4. Compare the computed hmac with the hmac from step 1.
  5. If they match, load "data", otherwise fail.

If stream (function): (use cases: loading from file)

  1. Create a ring buffer, the size of the HMAC.
  2. Call the function, loading the bytes into the buffer. (this is also where you check if the return value describes a binary chunk, if it doesn't you skip all the next steps.)
  3. If the function returns an end of chunk (nil, empty string, no value), compare the calculated HMAC with the provided HMAC, and continue/error as needed.
  4. As the bytes come out of the buffer, use them to update the HMAC, and pass them on to the original load() function.

If mode is just "b" or just "t" you can skip the type checks (remember to pass the mode on to the original load() function!), but if mode is "bt" you need to do the checks.

LordFokas commented 8 years ago

Why would you need it to be signed though? To prevent code injection?

SoniEx2 commented 8 years ago

See also https://gist.github.com/corsix/6575486

payonel commented 6 years ago

I investigated this feature in my emulator and found that using string.dump for memory swap would actually be an enormous undertaking and we'd have to parse the byte ourselves in some cases (for example serializing upvalues) So I don't find this worth it to support string.dump

SoniEx2 commented 6 years ago

swap? nah. this is just for stripping debug symbols and optionally precompiling each .lua file in OpenOS during install (so it can be installed on a machine with very little RAM and disk space).

y'know what would work for runtime RAM management? shrinking buffers/disk cache (buffer.lua) if the RAM is low.

skyem123 commented 6 years ago

Why not make it just check against a list of SHA hashes? The list could be provided by OC and plugin mods as they are already trusted to run code. While it'd be okay to allow dumping the string and adding that to a list of hashes in the save file, as it is kinda already loading right into memory from there with the whole persistence system, it's pointless because any modifications you'd want to make would fail the hash.

SoniEx2 commented 6 years ago

The point is not to make modifications. The point is to eliminate the "Lua parser" step (saves CPU time) and strip debug information (saves RAM and disk space). The former is done by simply loading bytecode. The latter is done with string.dump(f, true) - this is built-in, at least in Lua 5.3.

skyem123 commented 6 years ago

Alright, in that case, then it could work and if implemented correctly should no worse in security than the current system of saving state.