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: shared secret should be read from environmental variable #701

Merged
merged 1 commit into from
May 20, 2024

Conversation

schollz
Copy link
Owner

@schollz schollz commented May 20, 2024

To fix this, the shared secret should be read from an environment variable

Fixes #598

@schollz
Copy link
Owner Author

schollz commented May 20, 2024

@schollz schollz merged commit 863dabb into main May 20, 2024
3 checks passed
@nateify
Copy link

nateify commented May 22, 2024

Could we possibly get a flag introduced to revert to pre-9.1.16 behavior? This isn't a concern for me on single-user systems.

@schollz
Copy link
Owner Author

schollz commented May 22, 2024

@nateify that's fine by me. what should the flag be?

@nateify
Copy link

nateify commented May 22, 2024

@nateify that's fine by me. what should the flag be?

I think something like --generate-code or --generate-secret

@adryd325
Copy link

an environment variable to revert behavior would be nice too

@lazylua
Copy link

lazylua commented May 23, 2024

TBH
I don't know how to use croc anymore :'(

I tried

  • export CROC_SECRET="yourcodephrasetouse"
  • croc yourcodephrasetouse

all the time , getting creating securing channel...2024/05/23 13:08:42 could not secure channel

$ CROC_SECRET=23423423 croc recieve
securing channel...2024/05/23 13:17:27 room (secure channel) not ready, maybe peer disconnected

@fanxianluotuo
Copy link

I found that croc became completely unusable after the upgrade.

Both the sender and the receiver are using version v9.6.17. On the receiver side, I have tried

export CROC_SECRET="my-code-phrase"
croc --relay myhost:9009 --pass mypass my-code-phrase

or

croc --relay myhost:9009 --pass mypass
Enter receive code: my-code-phrase

The result is always:
securing channel...2024/05/23 17:07:04 room (secure channel) not ready, maybe peer disconnected

Delete my self-host relay config didn't help.

@schollz
Copy link
Owner Author

schollz commented May 23, 2024

@lazylua @fanxianluotuo make sure both clients are v9.6.17

I can replicate your errors, but only if one client is <=v9.6.15.

also make an issue for your issue please to compartmentalize discussions

@schollz
Copy link
Owner Author

schollz commented May 23, 2024

I updated to v10 to signal a breaking change: https://github.com/schollz/croc/releases

@jonasjelonek
Copy link

@schollz I'd also like to have the old behaviour somewhat back.
What about moving out the code argument out of the send command into the global options, keep its usage if the send command is used and if we want to receive, we can reuse this option for that purpose? The relay command would probably just ignore this option

@schollz
Copy link
Owner Author

schollz commented May 26, 2024

those arguments are still on the command line and show up on ps though?

@jonasjelonek
Copy link

sure, that would be the issue that we reintroduce the vulnerability in case you use this option. As @nateify mentioned this may not be an issue if you're on a single-user system. But I would follow your suggestion, since I also understand the issue with this vulnerability and, though it may take some time to get used to it, would be fine with giving the code via environment variable or in the croc prompt.

but what I don't really understand currently, is macOS not affected by this vulnerability?

@schollz
Copy link
Owner Author

schollz commented May 26, 2024

yes good point, macOS is, fixed in latest v10.0.5

also added --classic with disclaimer and opt-in only to get the classic mode if you accept the risk

@DerDennisOP
Copy link

When just providing the classic flag, the given error message is confusing:

croc --classic hydra.zip
Classic mode is currently DISABLED.

Please note that enabling this mode will make the shared secret visible
on the host's process list when passed via the command line. On a
multi-user system, this could allow other local users to access the
shared secret and receive the files instead of the intended recipient.

Do you wish to continue to enable the classic mode? (Y/n) y

Classic mode ENABLED.

To send and receive, use the code phrase:

  Send: croc send --code *** file.txt

  Receive: croc ***

@schollz
Copy link
Owner Author

schollz commented May 27, 2024

@DerDennisOP why

@jonasjelonek
Copy link

@DerDennisOP I guess you're using croc the wrong way. The --classic flag should be used on the receive side, the sender side is just used as before.

@DerDennisOP
Copy link

I thought that the classic flags needs to be set on both sides (sending and receiving) and the error message when trying to use --classic on the sending sides suggests to use the none classic sending command.

@bb010g
Copy link

bb010g commented Jun 10, 2024

Using an environment variable doesn't help much. A process can parse the plaintext out of /proc/$CROC_PID/environ instead of /proc/$CROC_PID/cmdline now.

@schollz
Copy link
Owner Author

schollz commented Jun 10, 2024

@bb010g using an environmental variable protects against other users on a multiuser system since /proc/$CROC_PID/environ is not available to other users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shared Secret Passed on Command Line Possibly Leaks to other Local Users
8 participants