Create VRF signing key file with correct permissions

[Jimbo4350]

Since we are now checking that the VRF signing key file has the correct owner read/write permissions, it makes sense to create the VRF signing key file with the correct permissions in cardano-cli.

• Related to DoS attack is possible if VRF key is known #1936

[rvl]

Yes please! I found that with version 1.21.2, cardano-node checks permissions, but cardano-cli doesn't set those required permissions.

It might be easier in general if cardano-cli just set a restricted umask on startup. See setFileCreationMask.

Also to be able to use functions from the unix package, without infecting everything with CPP, there are some tricks. Either:

1. Use a facade module like this: cardano-wallet-launcher.cabal. The implementation module for Cardano.Startup.POSIX uses functions from the unix library. The implementation module for Cardano.Startup.Windows has no-op functions. The module Cardano.Startup re-exports one or the other using a single instance of #ifdef WINDOWS CPP.
2. Put the same module in multiple cabal source dirs like this:

   if os(windows)
     build-depends: Win32
     hs-source-dirs:
       src-windows
     cpp-options:   -DWINDOWS
   else
     build-depends: unix
     hs-source-dirs:
       src-posix
   

[dcoutts]

Basic idea is fine.

As far as I can see the owner permission stuff is completely portable, and not unix-specific at all. And there's no reason for us to want to do it on one platform and not another.

I think it would be reasonable to add writeTextEnvelopeFileWithOwnerPermissions to the API itself. We can also simplify the code by having a separate (not exported) writeFileWithOwnerPermissions. That would more clearly separate the concerns. Then writeTextEnvelopeFileWithOwnerPermissions would be implemented like writeTextEnvelopeFile but calling writeFileWithOwnerPermissions rather than writeFile.

Why is it worth adding this function to the API so it can be used for any TextEnvelope format file? Because actually we probably want to use it everywhere that we write secret/signing keys. The auditors asked us to do this for VRF but really it applies universally.

[rvl]

@dcoutts Do you know, is there a cross-platform way of removing non-owner read permissions, if the file already exists?

I couldn't find any helpful functions in System.Directory. Testing in ghci showed me that these functions only touch the owner permissions. I found a user posted to haskell-cafe more than 10 years ago noting the same.

[dcoutts]

@rvl no I don't think there is. Sadly the file permission models are totally different between unix and windows, and indeed between classic unix and unix with extended permissions.

The solution is to create the files with the correct permissions in the first place.

[dcoutts]

Looks much better. Nearly there.

Once the last bits are done I think we can also squash down to one patch.

[dcoutts]

What do we need this for? The openTempFile makes files with the right permissions irrespective of the default file creation mask doesn't it?

[Jimbo4350]

When I run a bash script that uses cardano-cli, the file appears to be created with the incorrect permissions. I either have to put umask 077 at the top of my script or use setFileCreationMask in the cardano-cli exec.

[dcoutts]

I cannot reproduce this.

Irrespective of the initial umask, the file is created without "other" or "group" getting any permissions. The only time I can get the umask to affect things is if I set it to a nonsensical value like 0777 which prevents even the owner from having any permissions. But this is not a case we need to deal with.

That said, irrespective of whether it's needed or not, it may not be a bad idea to override the umask anyway and use 0077 for all files the cli creates, since arguably all the files the cli makes should be readable only to the user creating them.

@disassembler any opinion?

[dcoutts]

Nice.

[Jimbo4350]

Yes please! I found that with version 1.21.2, cardano-node checks permissions, but cardano-cli doesn't set those required permissions.

It might be easier in general if cardano-cli just set a restricted umask on startup. See setFileCreationMask.

Also to be able to use functions from the unix package, without infecting everything with CPP, there are some tricks. Either:

1. Use a facade module like this: [`cardano-wallet-launcher.cabal`](https://github.com/input-output-hk/cardano-wallet/blob/rvl/cardano-node-1.21.2/lib/launcher/cardano-wallet-launcher.cabal#L50-L57). The implementation module for `Cardano.Startup.POSIX` uses functions from the unix library. The implementation module for `Cardano.Startup.Windows` has no-op functions. The module `Cardano.Startup` re-exports one or the other using a single instance of `#ifdef WINDOWS` CPP.

2. Put the same module in multiple cabal source dirs like this:
   ```
   if os(windows)
     build-depends: Win32
     hs-source-dirs:
       src-windows
     cpp-options:   -DWINDOWS
   else
     build-depends: unix
     hs-source-dirs:
       src-posix
   ```

Thanks for the suggestion! Let's get this PR in and then I can look into what you've described.

[dcoutts]

I cannot reproduce this.

Irrespective of the initial umask, the file is created without "other" or "group" getting any permissions. The only time I can get the umask to affect things is if I set it to a nonsensical value like 0777 which prevents even the owner from having any permissions. But this is not a case we need to deal with.

That said, irrespective of whether it's needed or not, it may not be a bad idea to override the umask anyway and use 0077 for all files the cli creates, since arguably all the files the cli makes should be readable only to the user creating them.

@disassembler any opinion?

[dcoutts]

bors merge

[iohk-bors]

Build failed:

• ci/hydra-eval

[dcoutts]

bors merge

[Jimbo4350]

bors r+

[iohk-bors]

Build succeeded:

• ci/hydra-build:required
• buildkite/cardano-node
• ci/hydra-eval