Add black list and white list support for migrating repositories #8040

lunny · 2019-08-30T06:58:47Z

This PR will add two options blacklist and whitelist for migrating repositories from external URLs.

When you set whitelist then blacklist will be ignored and all users on this gitea instance could only migrate repositories from the domains on whitelist.

When whitelist is empty and you have domains on blacklist, all users on this gitea instance could migrate repositories any domain except the domains on blacklist.

guillep2k · 2019-08-30T15:35:33Z

custom/conf/app.ini.sample

+WHITELISTED_DOMAINS =
+; Blacklist for migrating, default is blank. Multiple domains could be separated by commas.
+; When WHITELISTED_DOMAINS is not blank, this option will be ignored.
+BLACKLISTED_DOMAINS =


Missing EOL

docs/content/doc/advanced/config-cheat-sheet.en-us.md

guillep2k · 2019-08-30T15:50:07Z

modules/migrations/migrate.go

+		return false, err
+	}
+
+	if strings.EqualFold(u.Scheme, "http") || strings.EqualFold(u.Scheme, "https") {


Why check the protocol? This will let ftp:, git: and ssh: pass if one day they are supported. Perhaps it's better to check if the url is local and make any other protocol go through whitelisting/blacklisting.

guillep2k · 2019-08-30T15:54:46Z

modules/migrations/migrate.go

+	if strings.EqualFold(u.Scheme, "http") || strings.EqualFold(u.Scheme, "https") {
+		if len(setting.Migration.WhitelistedDomains) > 0 {
+			if !whitelist.Match(u.Host) {
+				return false, fmt.Errorf("Migrate from %v is not allowed", u.Host)


Perhaps a translatable error should be better?

guillep2k · 2019-08-30T16:00:44Z

modules/migrations/migrate.go

+	var err error
+	whitelist, err = matchlist.NewMatchlist(setting.Migration.WhitelistedDomains...)
+	if err != nil {
+		return fmt.Errorf("Init migration whitelist domains failed: %v", err)


This error will show up when the instance starts, right? I mean, the admin needs to see the errors as soon as possible.

Yes. It will display when gitea start.

docs/content/doc/advanced/config-cheat-sheet.en-us.md

guillep2k · 2019-09-11T04:43:36Z

A new rebase + make fmt to pick up the latest changes?

codecov-io · 2019-09-11T15:11:30Z

Codecov Report

Merging #8040 into master will increase coverage by <.01%.
The diff coverage is 48.27%.

@@            Coverage Diff             @@
##           master    #8040      +/-   ##
==========================================
+ Coverage   41.81%   41.81%   +<.01%     
==========================================
  Files         482      484       +2     
  Lines       64557    64615      +58     
==========================================
+ Hits        26993    27018      +25     
- Misses      34095    34121      +26     
- Partials     3469     3476       +7

Impacted Files	Coverage Δ
modules/setting/migrate.go	`0% <0%> (ø)`
routers/init.go	`65.78% <25%> (-2.27%)`	⬇️
modules/matchlist/matchlist.go	`47.36% <47.36%> (ø)`
modules/migrations/migrate.go	`27% <60%> (+5.82%)`	⬆️
models/unit.go	`62.16% <0%> (-5.41%)`	⬇️
modules/process/manager.go	`76.81% <0%> (-4.35%)`	⬇️
models/repo_list.go	`74.14% <0%> (+0.97%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3fd0eec...cdff51c. Read the comment docs.

lunny · 2019-09-11T16:56:03Z

@guillep2k done.

docs/content/doc/advanced/config-cheat-sheet.en-us.md

guillep2k · 2019-09-11T17:45:32Z

modules/matchlist/matchlist.go

+	}
+
+	for _, rule := range list.rules {
+		rg, err := glob.Compile(rule)


Suggested change

rg, err := glob.Compile(rule)

rg, err := glob.Compile(rule,[]string{".","/"})

I think that separators should work better if specified, so the tokenization is aware of the different parts of the glob.

guillep2k · 2019-09-11T17:51:17Z

modules/matchlist/matchlist.go

+	}
+
+	for _, rule := range list.rules {
+		rg, err := glob.Compile(rule)


Some case normalization should be used. For instance, if I black list: "google.com", someone could work around that by using "GOOGLE.COM".

guillep2k · 2019-09-11T17:53:10Z

Thinking about it, I can set up a DNS of mine pointing to whatever site I want and make it work around the domain blacklist.

For example, the administrator wants to blacklist google.com (currently 172.217.30.238 for me); if I have access to a subdomain (e.g. myuser.dyndns.org), I could set up that to point to 172.217.30.238. Gitea wouldn't know anything about my setup, so I can import from myuser.dyndns.org (actually google.com) freely.

This is not easy to do, but we should be aware of the limitations of this kind of black list.
I think white list is much safer.
EDIT: rewording for clarity

I know domain aliasing is not an easy problem to solve for the blacklist functionality, but we should at least know that there's such vulnerability.
The whitelist part of this PR is OK from the safety point of view.

lafriks · 2019-09-11T17:56:00Z

@guillep2k in this PR there is also support for whitelist

guillep2k · 2019-09-11T17:59:08Z

@guillep2k in this PR there is also support for whitelist

@lafriks Yes. The whitelist part is OK. The blacklist part has some holes, as noted above. I think they require some rework.

stale · 2019-11-24T02:26:46Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

aschrijver · 2020-02-27T10:53:33Z

I maintain awesome-humane-tech and was advised to use ‘blocklist’ and ‘allowlist’ terminology instead (from a better inclusion / diversity perspective), and I wonder whether that would be better here too.

From WikiDiff:

blacklist: (legal) a list or collection of people or entities to be shunned or banned.

blocklist: (computing) a list of websites or other material to be blocked.

(and see also: http://garysaid.com/are-the-terms-whitelist-and-blacklist-racist/ )

Edit: See also IETF Recommendation (draft)

6543 · 2020-11-16T23:32:28Z

@lunny do you work on this or should I catch it up?

lunny · 2020-11-17T07:02:26Z

@6543 Please pick it up.

6543 · 2020-11-17T22:41:35Z

follow up pull: #13610

aschrijver · 2020-11-18T11:50:09Z

Thank you for switching terminology, much appreciated 🙏

lunny added the type/enhancement An improvement of existing functionality label Aug 30, 2019

lunny added this to the 1.10.0 milestone Aug 30, 2019

guillep2k reviewed Aug 30, 2019

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 30, 2019

guillep2k suggested changes Sep 3, 2019

View reviewed changes

docs/content/doc/advanced/config-cheat-sheet.en-us.md Show resolved Hide resolved

guillep2k mentioned this pull request Sep 4, 2019

Strict name matching for Repository.GetTagID() #8074

Merged

lunny force-pushed the lunny/black_list_migrate_mirror branch from 47936c5 to 63a2947 Compare September 7, 2019 03:36

lunny added 5 commits September 11, 2019 22:51

add black list and white list support for migrating repositories

7a083b9

fix fmt

3f2b34b

fix lint

ad5a226

fix vendor

7280964

fix modules.txt

cdff51c

lunny force-pushed the lunny/black_list_migrate_mirror branch from 63a2947 to cdff51c Compare September 11, 2019 14:52

guillep2k reviewed Sep 11, 2019

View reviewed changes

guillep2k suggested changes Sep 11, 2019

View reviewed changes

lunny modified the milestones: 1.10.0, 1.11.0 Sep 25, 2019

stale bot added the issue/stale label Nov 24, 2019

lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Nov 24, 2019

stale bot removed the issue/stale label Nov 24, 2019

lunny modified the milestones: 1.11.0, 1.12.0 Dec 13, 2019

lunny modified the milestones: 1.12.0, 1.13.0 May 2, 2020

lunny modified the milestones: 1.13.0, 1.14.0 Sep 1, 2020

6543 mentioned this pull request Nov 17, 2020

Add Allow-/Block-List for Migrate & Mirrors #13610

Merged

2 tasks

6543 closed this Nov 17, 2020

lunny deleted the lunny/black_list_migrate_mirror branch November 18, 2020 03:41

lunny removed this from the 1.14.0 milestone Nov 18, 2020

go-gitea locked and limited conversation to collaborators Jan 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add black list and white list support for migrating repositories #8040

Add black list and white list support for migrating repositories #8040

lunny commented Aug 30, 2019

guillep2k Aug 30, 2019

guillep2k Aug 30, 2019

guillep2k Aug 30, 2019

guillep2k Aug 30, 2019

lunny Aug 30, 2019

guillep2k commented Sep 11, 2019

codecov-io commented Sep 11, 2019

lunny commented Sep 11, 2019

guillep2k Sep 11, 2019

guillep2k Sep 11, 2019

guillep2k commented Sep 11, 2019 •

edited

Loading

lafriks commented Sep 11, 2019

guillep2k commented Sep 11, 2019

stale bot commented Nov 24, 2019

aschrijver commented Feb 27, 2020 •

edited

Loading

6543 commented Nov 16, 2020

lunny commented Nov 17, 2020

6543 commented Nov 17, 2020

aschrijver commented Nov 18, 2020

	rg, err := glob.Compile(rule)
	rg, err := glob.Compile(rule,[]string{".","/"})

Add black list and white list support for migrating repositories #8040

Add black list and white list support for migrating repositories #8040

Conversation

lunny commented Aug 30, 2019

guillep2k Aug 30, 2019

Choose a reason for hiding this comment

guillep2k Aug 30, 2019

Choose a reason for hiding this comment

guillep2k Aug 30, 2019

Choose a reason for hiding this comment

guillep2k Aug 30, 2019

Choose a reason for hiding this comment

lunny Aug 30, 2019

Choose a reason for hiding this comment

guillep2k commented Sep 11, 2019

codecov-io commented Sep 11, 2019

Codecov Report

lunny commented Sep 11, 2019

guillep2k Sep 11, 2019

Choose a reason for hiding this comment

guillep2k Sep 11, 2019

Choose a reason for hiding this comment

guillep2k commented Sep 11, 2019 • edited Loading

lafriks commented Sep 11, 2019

guillep2k commented Sep 11, 2019

stale bot commented Nov 24, 2019

aschrijver commented Feb 27, 2020 • edited Loading

6543 commented Nov 16, 2020

lunny commented Nov 17, 2020

6543 commented Nov 17, 2020

aschrijver commented Nov 18, 2020

guillep2k commented Sep 11, 2019 •

edited

Loading

aschrijver commented Feb 27, 2020 •

edited

Loading