Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Atomic create new files with permissions to owner in ethstore #8896

Merged
merged 2 commits into from
Jun 14, 2018

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Jun 14, 2018

In ethstore, there's a small window where a new key file is created but its permission bits have not been set. This PR fixes by setting the permission bits when creating the files. std::os::unix::OpenOptionsExt is used to replace the small piece of unsafe code. OptionOptions is changed to use create_new(true) instead of create(true) -- we check whether the file exists before creating it, so if in that small window another process creates the file with the same name, it's better to fail rather than "stealing" that file.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jun 14, 2018
@sorpaas sorpaas added this to the 1.12 milestone Jun 14, 2018
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 14, 2018
@andresilva andresilva added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jun 14, 2018
We have two behaviors for `insert_with_filename` depending on whether `dedup` is true.  Add
`replace_file_with_permissions_to_owner` which use `OpenOptions::create(true)` instead of `create_new`.
@andresilva andresilva added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Jun 14, 2018
@debris debris merged commit fc86b17 into master Jun 14, 2018
@debris debris deleted the sorpaas/ethstore-owner-permission branch June 14, 2018 11:54
dvdplm added a commit that referenced this pull request Jun 16, 2018
* master:
  Minor fix in chain supplier and light provider (#8906)
  Block 0 is valid in queries (#8891)
  fixed osx permissions (#8901)
  Atomic create new files with permissions to owner in ethstore (#8896)
  Add ETC Cooperative-run load balanced parity node (#8892)
  Add support for --chain tobalaba (#8870)
  fix some warns on nightly (#8889)
  Add new ovh bootnodes and fix port for foundation bootnode 3.2 (#8886)
  SecretStore: service pack 1 (#8435)
tavakyan referenced this pull request in C4Coin/c4coin-parity Jun 18, 2018
* Atomic create new files with permissions to owner in ethstore

* Allow replacing existing files

We have two behaviors for `insert_with_filename` depending on whether `dedup` is true.  Add
`replace_file_with_permissions_to_owner` which use `OpenOptions::create(true)` instead of `create_new`.
ordian added a commit to ordian/parity that referenced this pull request Jun 20, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity: (29 commits)
  Block 0 is valid in queries (openethereum#8891)
  fixed osx permissions (openethereum#8901)
  Atomic create new files with permissions to owner in ethstore (openethereum#8896)
  Add ETC Cooperative-run load balanced parity node (openethereum#8892)
  Add support for --chain tobalaba (openethereum#8870)
  fix some warns on nightly (openethereum#8889)
  Add new ovh bootnodes and fix port for foundation bootnode 3.2 (openethereum#8886)
  SecretStore: service pack 1 (openethereum#8435)
  Handle removed logs in filter changes and add geth compatibility field (openethereum#8796)
  fixed ipc leak, closes openethereum#8774 (openethereum#8876)
  scripts: remove md5 checksums (openethereum#8884)
  hardware_wallet/Ledger `Sign messages` + some refactoring (openethereum#8868)
  Check whether we need resealing in miner and unwrap has_account in account_provider (openethereum#8853)
  docker: Fix alpine build (openethereum#8878)
  Remove mac os installers etc (openethereum#8875)
  README.md: update the list of dependencies (openethereum#8864)
  Fix concurrent access to signer queue (openethereum#8854)
  Tx permission contract improvement (openethereum#8400)
  Limit the number of transactions in pending set (openethereum#8777)
  Use sealing.enabled to emit eth_mining information (openethereum#8844)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants