From 3fbac39a608956e62057eab3aa5ee67ddabceb2f Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 17 Aug 2024 15:22:46 +0200 Subject: [PATCH 1/5] git:// protocol check: add test files --- .../PackageTests/Check/GitProtocol/cabal.out | 5 +++++ .../PackageTests/Check/GitProtocol/cabal.test.hs | 3 +++ .../PackageTests/Check/GitProtocol/pkg.cabal | 16 ++++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out create mode 100644 cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs create mode 100644 cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out new file mode 100644 index 00000000000..1bc8481c2c0 --- /dev/null +++ b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out @@ -0,0 +1,5 @@ +# cabal check +The following errors are likely to affect your build negatively: +Error: [git-protocol] Cloning over git:// might lead to an arbitrary code +execution vulnerability. Use https:// or ssh:// instead. +Error: Hackage would reject this package. diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs new file mode 100644 index 00000000000..d57f8e16590 --- /dev/null +++ b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.test.hs @@ -0,0 +1,3 @@ +import Test.Cabal.Prelude + +main = cabalTest $ fails $ cabal "check" [] diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal b/cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal new file mode 100644 index 00000000000..5622d522ac3 --- /dev/null +++ b/cabal-testsuite/PackageTests/Check/GitProtocol/pkg.cabal @@ -0,0 +1,16 @@ +cabal-version: 3.0 +name: pkg +version: 0 +category: example +maintainer: me@example.com +synopsis: small synopsis +description: longer description +license: GPL-3.0-only + +library + exposed-modules: Foo + default-language: Haskell2010 + +source-repository head + type: git + location: git://www.example.org/my-repo/ From cc65775f5c7aafb9061f054253c016c62b7c79ca Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 17 Aug 2024 16:02:38 +0200 Subject: [PATCH 2/5] git:// protocol check: add docs --- doc/cabal-commands.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/cabal-commands.rst b/doc/cabal-commands.rst index 8c05a9b6593..c1987afbfd8 100644 --- a/doc/cabal-commands.rst +++ b/doc/cabal-commands.rst @@ -1327,6 +1327,9 @@ A list of all warnings with their constructor: - ``unrecognised-repo-type``: unrecognised kind of source-repository. - ``repo-no-type``: missing ``type`` in ``source-repository``. - ``repo-no-location``: missing ``location`` in ``source-repository``. +- ``git-protocol``: using insecure ``git://`` protocol + (`explanation `__ + in Git Book). - ``repo-no-module``: missing ``module`` in ``source-repository``. - ``repo-no-tag``: missing ``tag`` in ``source-repository``. - ``repo-relative-dir``: ``subdir`` in ``source-repository`` must be relative. From e8af2d3859fece095d1d3c9dec34685accb4aaaf Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 17 Aug 2024 15:58:34 +0200 Subject: [PATCH 3/5] Implement git:// protocol check --- Cabal/src/Distribution/PackageDescription/Check.hs | 12 ++++++++++++ .../Distribution/PackageDescription/Check/Warning.hs | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/Cabal/src/Distribution/PackageDescription/Check.hs b/Cabal/src/Distribution/PackageDescription/Check.hs index 5787dec3b77..8bab6ec961a 100644 --- a/Cabal/src/Distribution/PackageDescription/Check.hs +++ b/Cabal/src/Distribution/PackageDescription/Check.hs @@ -684,6 +684,7 @@ checkSourceRepos rs = do checkP (isNothing repoLocation_) (PackageDistInexcusable MissingLocation) + checkGitProtocol repoLocation_ checkP ( repoType_ == Just (KnownRepoType CVS) && isNothing repoModule_ @@ -722,6 +723,17 @@ checkMissingVcsInfo rs = repoTypeDirname Monotone = ["_MTN"] repoTypeDirname Pijul = [".pijul"] +-- git:// lacks TLS or other encryption, see +-- https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_cons_4 +checkGitProtocol + :: Monad m + => Maybe String -- Repository location + -> CheckM m () +checkGitProtocol mloc = + checkP + (fmap (isPrefixOf "git://") mloc == Just True) + (PackageBuildWarning GitProtocol) + -- ------------------------------------------------------------ -- Package and distribution checks -- ------------------------------------------------------------ diff --git a/Cabal/src/Distribution/PackageDescription/Check/Warning.hs b/Cabal/src/Distribution/PackageDescription/Check/Warning.hs index 859b3f12c50..4a587a8772f 100644 --- a/Cabal/src/Distribution/PackageDescription/Check/Warning.hs +++ b/Cabal/src/Distribution/PackageDescription/Check/Warning.hs @@ -193,6 +193,7 @@ data CheckExplanation | UnrecognisedSourceRepo String | MissingType | MissingLocation + | GitProtocol | MissingModule | MissingTag | SubdirRelPath @@ -355,6 +356,7 @@ data CheckExplanationID | CIUnrecognisedSourceRepo | CIMissingType | CIMissingLocation + | CIGitProtocol | CIMissingModule | CIMissingTag | CISubdirRelPath @@ -496,6 +498,7 @@ checkExplanationId (NoLicenseFile{}) = CINoLicenseFile checkExplanationId (UnrecognisedSourceRepo{}) = CIUnrecognisedSourceRepo checkExplanationId (MissingType{}) = CIMissingType checkExplanationId (MissingLocation{}) = CIMissingLocation +checkExplanationId (GitProtocol{}) = CIGitProtocol checkExplanationId (MissingModule{}) = CIMissingModule checkExplanationId (MissingTag{}) = CIMissingTag checkExplanationId (SubdirRelPath{}) = CISubdirRelPath @@ -642,6 +645,7 @@ ppCheckExplanationId CINoLicenseFile = "no-license-file" ppCheckExplanationId CIUnrecognisedSourceRepo = "unrecognised-repo-type" ppCheckExplanationId CIMissingType = "repo-no-type" ppCheckExplanationId CIMissingLocation = "repo-no-location" +ppCheckExplanationId CIGitProtocol = "git-protocol" ppCheckExplanationId CIMissingModule = "repo-no-module" ppCheckExplanationId CIMissingTag = "repo-no-tag" ppCheckExplanationId CISubdirRelPath = "repo-relative-dir" @@ -964,6 +968,9 @@ ppExplanation MissingType = "The source-repository 'type' is a required field." ppExplanation MissingLocation = "The source-repository 'location' is a required field." +ppExplanation GitProtocol = + "Cloning over git:// might lead to an arbitrary code execution " + ++ "vulnerability. Use https:// or ssh:// instead." ppExplanation MissingModule = "For a CVS source-repository, the 'module' is a required field." ppExplanation MissingTag = From c63ebf923a2f01f8d75b5cc73024e108ad74a9f6 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 17 Aug 2024 16:10:41 +0200 Subject: [PATCH 4/5] Add changelog for #10261 --- changelog.d/pr-10261 | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog.d/pr-10261 diff --git a/changelog.d/pr-10261 b/changelog.d/pr-10261 new file mode 100644 index 00000000000..adcae60fd88 --- /dev/null +++ b/changelog.d/pr-10261 @@ -0,0 +1,12 @@ +synopsis: Warn about git:// protocol +packages: cabal-install +prs: #10261 + +description: { + +`cabal check` will warn about insecure git:// protocol in `source-repository`. + +See [Git Book](https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_cons_4) +for an explanation. + +} From e581306758613955b17a239576bd3d479efe5460 Mon Sep 17 00:00:00 2001 From: Francesco Ariis Date: Sat, 17 Aug 2024 18:02:13 +0200 Subject: [PATCH 5/5] Specify GitHub does not support git:// --- Cabal/src/Distribution/PackageDescription/Check/Warning.hs | 3 ++- cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Cabal/src/Distribution/PackageDescription/Check/Warning.hs b/Cabal/src/Distribution/PackageDescription/Check/Warning.hs index 4a587a8772f..778a89ddfbe 100644 --- a/Cabal/src/Distribution/PackageDescription/Check/Warning.hs +++ b/Cabal/src/Distribution/PackageDescription/Check/Warning.hs @@ -970,7 +970,8 @@ ppExplanation MissingLocation = "The source-repository 'location' is a required field." ppExplanation GitProtocol = "Cloning over git:// might lead to an arbitrary code execution " - ++ "vulnerability. Use https:// or ssh:// instead." + ++ "vulnerability. Furthermore, popular forges like GitHub do " + ++ "not support it. Use https:// or ssh:// instead." ppExplanation MissingModule = "For a CVS source-repository, the 'module' is a required field." ppExplanation MissingTag = diff --git a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out index 1bc8481c2c0..eb61b32ab3e 100644 --- a/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out +++ b/cabal-testsuite/PackageTests/Check/GitProtocol/cabal.out @@ -1,5 +1,6 @@ # cabal check The following errors are likely to affect your build negatively: Error: [git-protocol] Cloning over git:// might lead to an arbitrary code -execution vulnerability. Use https:// or ssh:// instead. +execution vulnerability. Furthermore, popular forges like GitHub do not +support it. Use https:// or ssh:// instead. Error: Hackage would reject this package.