Allow wildcard * to be used in trusted domains

[jernst]

This is to support setups where no reliable DNS entry is available (e.g. mDNS) or for simple-to-setup aliasing (e.g. *.example.com)

This time, I hope, tabs and formatting are all in the right places for your coding standards. We had been doing this patch just for UBOS, but now that you are doing checksums on PHP files, such as private patch leads to warnings in the management console, and I hope it can be merged into the main.

As using a * in a trusted domain is entirely optional, this should have no security implications for the default setup.

If you'd like me to augment some existing tests, I'd appreciate a pointer to which tests.

[mention-bot]

@jernst, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @DeepDiver1975 and @MorrisJobke to be potential reviewers

[LukasReschke]

===

[LukasReschke]

That's a lot of code for a very minor feature and it also contains some syntax errors (which the unit tests have spotted!). Can you by any chance make the code smaller by switching to a regex or so?

[LukasReschke]

Syntax error.

[LukasReschke]

You can add test entries at

server/tests/lib/Security/TrustedDomainHelperTest.php

Lines 46 to 80 in 83ea738

	public function trustedDomainDataProvider() {
	$trustedHostTestList = [
	'host.one.test',
	'host.two.test',
	'[1fff:0:a88:85a3::ac1f]',
	'host.three.test:443',
	];
	return [
	// empty defaults to false with 8.1
	[null, 'host.one.test:8080', false],
	['', 'host.one.test:8080', false],
	[[], 'host.one.test:8080', false],
	// trust list when defined
	[$trustedHostTestList, 'host.two.test:8080', true],
	[$trustedHostTestList, 'host.two.test:9999', true],
	[$trustedHostTestList, 'host.three.test:8080', false],
	[$trustedHostTestList, 'host.two.test:8080:aa:222', false],
	[$trustedHostTestList, '[1fff:0:a88:85a3::ac1f]', true],
	[$trustedHostTestList, '[1fff:0:a88:85a3::ac1f]:801', true],
	[$trustedHostTestList, '[1fff:0:a88:85a3::ac1f]:801:34', false],
	[$trustedHostTestList, 'host.three.test:443', true],
	[$trustedHostTestList, 'host.three.test:80', false],
	[$trustedHostTestList, 'host.three.test', false],
	// trust localhost regardless of trust list
	[$trustedHostTestList, 'localhost', true],
	[$trustedHostTestList, 'localhost:8080', true],
	[$trustedHostTestList, '127.0.0.1', true],
	[$trustedHostTestList, '127.0.0.1:8080', true],
	// do not trust invalid localhosts
	[$trustedHostTestList, 'localhost:1:2', false],
	[$trustedHostTestList, 'localhost: evil.host', false],
	// do not trust casting
	[[1], '1', false],
	];
	}

and run the tests with cd tests && phpunit lib/Security/TrustedDomainHelperTest.php

[LukasReschke]

Can you by any chance make the code smaller by switching to a regex or so?

Something like:

[a-zA-Z0-9]+.foo.com

Wheras you replace all stars with [a-zA-Z0-9]+. Should make the code way simpler.

[jernst]

I'll see what I can do to make it shorter tomorrow. Regexes for security checks of domains containing . and patterns containing * are easy to get wrong ...

[MorrisJobke]

I really would love to see the test cases extended for this: https://github.com/nextcloud/server/blob/master/tests/lib/Security/TrustedDomainHelperTest.php

😉

[MorrisJobke]

@LukasReschke Please recheck ;)

[LukasReschke]

Code looks good to me. Thanks a lot 🚀

Can I ask you to add a small note to the config option in config.sample.php that the * symbol has a special meaning? Then this should be good to get in for Nextcloud Next :)

[jernst]

You mean Next++cloud? :-)

[LukasReschke]

Thanks a lot for your Pull Request. LGTM! 🚀 ❤️ 🎉

[icewind1991]

👍