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

Fix: env variable decoding and added error messages #5

Merged

Conversation

mrauhu
Copy link
Contributor

@mrauhu mrauhu commented Mar 4, 2024

Hello @orph3usLyre.

Fix for an error:

no method named as_bytes found for struct OsString in the current scope [E0599]

let Ok(bytes) = <[u8; 32]>::from_hex(var.as_bytes()) else {

As far as I know, it's Windows related problem, because the OsStrExt trait on Windows https://doc.rust-lang.org/std/os/windows/ffi/trait.OsStrExt.html is missing as_bytes() function.

Related: #2 and #3.

Changes

  • Fix: using var.as_encoded_bytes() instead of var.as_bytes() in the build_env_cipher_block() function.
  • Added panic messages in the build_env_cipher_block() function.
  • Added cross-platform information how to set env variables in the build_obfuscation_mod() function.

Best wishes,
Sergey.

mrauhu added 3 commits March 4, 2024 15:26
* `Need to set {} env variable`
* `Can't get bytes from {} env variable, secret key needs to be a hex string with 64 symbols length.`
@orph3usLyre
Copy link
Owner

Hey again @mrauhu,

Thanks for working on this! I'd like to have a look at the differences between the unix as_bytes() and the as_encoded_bytes() before merging this, so I'll check when I'm off work over the next few days. (But it appears as if as_encoded_bytes() is the more flexible method, which is nice.)

I'm not a big fan of the panic!() messages, however. The core idea of muddy was to provide all the information required to run the program at build-time, and to avoid (as much as possible) any indications of how to de-obfuscate the binary after it's built. This way, it would be more obscure to people who might have the binary, but did not compile it. The panic messages would be quite the big hint to anyone attempting to decrypt the strings inside the binary - I'd rather avoid that and allow the implementers to decide what messages to display at runtime.

Thanks again! 🦀

@mrauhu
Copy link
Contributor Author

mrauhu commented Mar 4, 2024

Thank you for the context.

@orph3usLyre
Copy link
Owner

Hey @mrauhu,

I checked the implementation of as_encoded_bytes() and this seems like it would be fine for the use case here.
I made a few suggestions based on my comment from yesterday, and if these look good to you, I'd be happy to merge this.

Thanks again!

…release build to avoid (as much as possible) any indications of how to de-obfuscate the binary after it's built
@mrauhu
Copy link
Contributor Author

mrauhu commented Mar 6, 2024

Hello @orph3usLyre,

About the changes, that you requested:

  1. In my opinion, it is better to leave information about what the developer needs to do during development, because this will be a good error message that adds a context of fix, not only indicating how to fix it.

  2. I read your suggestion comment and original message, quote:

I'm not a big fan of the panic!() messages, however. The core idea of muddy was to provide all the information required to run the program at build-time, and to avoid (as much as possible) any indications of how to de-obfuscate the binary after it's built. This way, it would be more obscure to people who might have the binary, but did not compile it.

And pushed a small update that will help developers to understand what to do when they debug code, but hide it for a release build.

In your opinion this is suitable?

Thank you.

@orph3usLyre
Copy link
Owner

Hey @mrauhu,

I really like the idea of using the #[cfg(debug_assertions)] to provide more info for debug builds, let's go with that. Just to double check, the text doesn't show up in the release binary if you run strings on it, right?

I also hadn't realized that the //language=sh was a RustRover thing. If that's the case, I don't mind it. 👍

Just a final question then, is there any particular reason for the extra empty eprintln!()?

Cheers!

@mrauhu
Copy link
Contributor Author

mrauhu commented Mar 6, 2024

Just to double check, the text doesn't show up in the release binary if you run strings on it, right?

The only way it can be in the release build, if a developer will to change default release profile settings in Cargo.toml to:

[profile.release]
debug-assertions = true

Or set it for rustc with -C debug-assertions=true.

https://doc.rust-lang.org/cargo/reference/profiles.html#debug-assertions

Just a final question then, is there any particular reason for the extra empty eprintln!()?

@orph3usLyre thank you for noticing, removed it.

@orph3usLyre orph3usLyre merged commit e441338 into orph3usLyre:main Mar 7, 2024
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

Successfully merging this pull request may close these issues.

2 participants