haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.63k stars 696 forks source link

Update the documentation of the option `ignore-expiry` to make clear what it actually does #8918

Open gksato opened 1 year ago

gksato commented 1 year ago

We need to revise the documentation of the cabal.project option ignore-expiry to prevent confusion. Setting this option will disable the verification of remote repositories' signature expiration on the command execution of cabal update, and the flag has nothing to do with the following familiar warning:

Warning: The package list for 'hackage.haskell.org' is *** days old.
Run 'cabal update' to get the latest list of available packages.

In fact, I, myself, misunderstood what the option does. This issue, as it was first written, complained that the option didn't do what I expected it to do! This issue has been updated through conversation to reflect reality. I attach below my embarrassingly mistaken original issue.


The initial issue-opening comment. This is based on my misunderstanding.

Describe the bug The ignore-expiry option doesn't have implementation in this codebase. Therefore, our familiar warning

Warning: The package list for 'hackage.haskell.org' is *** days old.
Run 'cabal update' to get the latest list of available packages.

will not go away with cabal --ignore-expiry. However this lack of implementation does not seem to be documented.

The single function readRepoIndex, implemented in lines 363-415 of Distribution.Client.IndexUtils in cabal-install, master as of 2023-04-25T01:37:14Z (5f77754), have the total control of the warning. This function does not have access to the status of ignore-expiry option! Furthermore, the search against this repository does not reveal any code that inspects the field globalIgnoreExpiry of the data type GlobalFlags (defined in lines 58-73 of Distribution.Client.GlobalFlags); this strongly suggests that ignore-expiry option has completely no influence on the behavior of cabal-install.

To Reproduce Steps to reproduce the behavior: for example,

$ REPOCACHE=[your remote-repo-cache value, e.g. $HOME/.cabal/packages]/hackage.haskell.org/01-index.tar
$ touch -d 2000-01-01T00:00:00Z $REPOCACHE
$ cabal --ignore-expiry info hello
Warning: The package list for 'hackage.haskell.org' is 8515 days old.
Run 'cabal update' to get the latest list of available packages.
* hello            (program)
    Synopsis:      Hello World, an example package
    Versions available: 1.0, 1.0.0.1, 1.0.0.2
    Versions installed: [ Unknown ]
    Homepage:      http://www.haskell.org/hello/
    Bug reports:   mailto:marlowsd@gmail.com
    Description:   This is an implementation of the classic "Hello World"
                   program in Haskell, as an example of how to create a minimal
                   Haskell application using Cabal and Hackage. Please submit
                   any suggestions and improvements.
    Category:      Console, Text
    License:       BSD3
    Author:        Simon Marlow
    Maintainer:    Simon Marlow <marlowsd@gmail.com>
    Source repo:   http://darcs.haskell.org/hello/
    Executables:   hello
    Flags:         threaded
    Dependencies:  base >=4.2 && <5
    Cached:        No

Expected behavior

For example, the shell session above should be changed to:

$ REPOCACHE=[your remote-repo-cache value, e.g. $HOME/.cabal/packages]/hackage.haskell.org/01-index.tar
$ touch -d 2000-01-01T00:00:00Z $REPOCACHE
$ cabal --ignore-expiry info hello
* hello            (program)
    Synopsis:      Hello World, an example package
    Versions available: 1.0, 1.0.0.1, 1.0.0.2
    Versions installed: [ Unknown ]
    Homepage:      http://www.haskell.org/hello/
    Bug reports:   mailto:marlowsd@gmail.com
    Description:   This is an implementation of the classic "Hello World"
                   program in Haskell, as an example of how to create a minimal
                   Haskell application using Cabal and Hackage. Please submit
                   any suggestions and improvements.
    Category:      Console, Text
    License:       BSD3
    Author:        Simon Marlow
    Maintainer:    Simon Marlow <marlowsd@gmail.com>
    Source repo:   http://darcs.haskell.org/hello/
    Executables:   hello
    Flags:         threaded
    Dependencies:  base >=4.2 && <5
    Cached:        No

System information

Additional context I'm writing a Haskell build environment for the judge server of a competitive programming website AtCoder. The server forces an offline build for each submission; we need to download and set up all dependencies on the installation time, so this feature is absolutely desirable!

gbaz commented 1 year ago

Ignore-expiry does have an implementation, but it does something different than what you want, and we can update the docs to make this clearer.

In particular, hackage-security means that repos such as hackage have signing keys that they use to authenticate the integrity of the index. These keys periodically need to be resigned and authenticated by the root keyholders. Ignoring expiry is about ignoring the expiry of these keys, should keyholders not reauthenticate those keys in a timely fashion...

gksato commented 1 year ago

Signing keys! I've got it wrong, thank you. Would just closing this suffice, or should I leave this issue as is for the ease of update of documentation?

gbaz commented 1 year ago

We can maybe make it a ticket to improve the docs?

gksato commented 1 year ago

Sounds like a good idea. However if I find difficulty in updating this issue, I might be going to need to leave this to you guys. I'll try anyway.

BTW, For a better understanding of this option, I just searched for expiry in this codebase (I searched for globalIgnoreExpiry when writing the issue-opening comment. Of course it only led to a mistake 😅 ). I found the following:

https://github.com/haskell/cabal/blob/5f77754c5ed6037e04022b03b1bb0a5e46acecd6/cabal-install/src/Distribution/Client/CmdUpdate.hs#L202-L205

It looks like the current time for verification is passed only if "ignoreExpiry" is TRUE. The documentation for Hackage.Security.Client.checkForUpdates says

You should pass Nothing for the UTCTime only under exceptional circumstances (such as when the main server is down for longer than the expiry dates used in the timestamp files on mirrors).

Is this the correct implementation?

gksato commented 1 year ago

@gbaz I updated the issue opener. Does it look good to you? Also, would you take a look at my previous comment? It is severe if I'm correct.

gbaz commented 1 year ago

Thanks for looking into this. The new description seems correct.

I think the issue you found may also be correct. If so it opens an attack vector to be certain, but not necessarily an extremely severe one. In particular, it would mean that in the case of a compromise of an older key, then that could be combined with a MITM attack to subvert the repository. I think it certainly should be investigated and fixed. I however don't want to do that in a minor release, because it has been that way since the start, so this is effectively turning on a new layer of security, and we should give that plenty of burn-in time. It appears that @angerman made the initial commit, and perhaps @adamgundry or the like could speak to confirming the diagnosis?

gksato commented 1 year ago

In particular, it would mean that in the case of a compromise of an older key,

😌 Yes, I now see it. It is a problem only if a key is compromised. I feel more relieved. Thanks.

gksato commented 1 year ago

Is it better to split out a probable mis-implementation of ignore-expiry as a separate issue now?

andreabedini commented 1 year ago

Just a drive by comment mentioning that we don't have any test covering TUF and index-state (to my knowledge).

(I wrote some code suitable for the test suite prelude but it still needs to be integrated; and of course tests needs to be written.)