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

Use %LOCALAPPDATA% not %APPDATA% on Windows #5660

Closed
wgknowles opened this issue Apr 28, 2020 · 5 comments · Fixed by #8095
Closed

Use %LOCALAPPDATA% not %APPDATA% on Windows #5660

wgknowles opened this issue Apr 28, 2020 · 5 comments · Fixed by #8095
Assignees
Labels
Priority: High High priority item Security Security Related Issues
Milestone

Comments

@wgknowles
Copy link
Contributor

wgknowles commented Apr 28, 2020

🐞 Bug Report

Description

The folder C:\Users\username\AppData\Roaming (%APPDATA%) is used in enterprises where multiple PC's are joined to a domain. Files in this folder are copied to a fileserver on logout, and downloaded from the server on logon. This is not an appropriate location / behaviour for large database files or private keys.

  • %APPDATA%\Eth2 (which expands to C:\Users\username\AppData\Roaming\Eth2) is the default location for:

    • beacon-chain-v1.0.0-alpha.5-windows-amd64 --datadir
    • slasher -v1.0.0-alpha.5-windows-amd64 --datadir
  • A better default location would be %LOCALAPPDATA% (expands to C:\Users\username\AppData\Local)

Currently, validator puts keys in %USERPROFILE%.eth2validator which is ok, but why are we spraying files all over the filesystem? %LOCALAPPDATA%\eth2 seems better to me...
%APPDATA% would not be appropriate for private keys, but %LOCALAPPDATA% would be perfect to put all prysm files.

To nitpick even more, industry standards would use this structure:

  • %LOCALAPPDATA%\PrysmaticLabs\prysm\
    • prysm.bat
    • \beacon-chain
      • \bin\beacon-chain-v1.0.0-alpha.5-windows-amd64.exe
      • beaconchain.db
      • metaData
      • network-keys
    • \validator
      • \bin\validator-v1.0.0-alpha.5-windows-amd64.exe
      • validator.db
      • validatorprivatekey123456789
      • shardwithdrawalkey123456789
    • \slasher
      • \bin\slasher-v1.0.0-alpha.5-windows-amd64.exe
      • slasher.db

What version of Prysm are you running? (Which release)

  
v1.0.0-alpha.5-windows-amd64
  
@wgknowles
Copy link
Contributor Author

@farazdagi

@rauljordan
Copy link
Contributor

Done by prysmaticlabs/documentation#131

@nisdas nisdas reopened this Dec 10, 2020
@nisdas nisdas added Security Security Related Issues Priority: High High priority item labels Dec 10, 2020
@MicahZoltu
Copy link

The above PR just updated the docs, prysm still defaults to putting everything in roaming profile folder. As others mentioned, keys probably shouldn't go there as it means they'll be copied around the network (not E2E encrypted unless Prism is encrypting on disk), and the bulk data definitely shouldn't be copied over the network on login/logout.

The one thing I disagree with the OP about is:

Currently, validator puts keys in %USERPROFILE%.eth2validator which is ok

Pretty much nothing should go in $USERPROFILE% unless the user explicitly puts it there via a save dialog box or manual configuration. The other suggested locations (%LOCALAPPDATA%/...) are good though.

@farazdagi
Copy link
Contributor

Thank you for providing more info/context, I'll produce fixing PR within a session.

@defeedme
Copy link

this bug still exists 4 years later.. would this cause peers to be lost in windows? thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High High priority item Security Security Related Issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants