PR#50803: Issue 50800 - Fix parsing of wildcards in rootdn-allow-ip attribute - 389-ds-base

389-ds-base

#50803 Issue 50800 - Fix parsing of wildcards in rootdn-allow-ip attribute

Closed 3 years ago by spichugi. Opened 4 years ago by yrro.

yrro/389-ds-base rootdn-allow-ip-wildcards into master

WIP tests

Sam Morris • 4 years ago

f726116

Issue 50800 - Fix parsing of wildcards in rootdn-allow-ip attribute

Sam Morris • 4 years ago

28a6e84

dirsrvtests/tests/suites/plugins/rootdn_plugin_test.py

file modified

+71

		`@@ -219,6 +219,41 @@`
		`rootdn_bind(topology_st.standalone, uri=uri)`


		`+ def test_rootdn_access_denied_ip_wildcard(topology_st, rootdn_setup, rootdn_cleanup):`
		`+ """Test denied IP feature with a wildcard`
		`+`
		`+ :id: 73c74f62-9ac2-4bb6-8a63-bacc8d8bbf93`
		`+ :setup: Standalone instance, rootdn plugin set up`
		`+ :steps:`
		`+ 1. Set rootdn-deny-ip to '127.*'`
		`+ 2. Bind as Root DN`
		`+ 3. Change the denied IP so root DN succeeds`
		`+ 4. Bind as Root DN`
		`+ :expectedresults:`
		`+ 1. Success`
		`+ 2. Should fail`
		`+ 3. Success`
		`+ 4. Success`
		`+ """`
		`+`
		`+ log.info('Running test_rootdn_access_denied_ip_wildcard...')`
		`+`
		`+ plugin.add_deny_ip('127.*')`
		`+ time.sleep(.5)`
		`+`
		`+ # Bind as root DN - should fail`
		`+ uri = 'ldap://{}:{}'.format('127.0.0.1', topology_st.standalone.port)`
		`+ with pytest.raises(ldap.UNWILLING_TO_PERFORM):`
		`+ rootdn_bind(topology_st.standalone, uri=uri)`
		`+`
		`+ # Change the denied IP so root DN succeeds`
		`+ plugin.apply_mods([(ldap.MOD_REPLACE, 'rootdn-deny-ip', '255.255.255.255')])`
		`+ time.sleep(.5)`
		`+`
		`+ # Bind should succeed`
		`+ rootdn_bind(topology_st.standalone, uri=uri)`
		`+`
		`+`
		`def test_rootdn_access_denied_host(topology_st, rootdn_setup, rootdn_cleanup):`
		`"""Test denied Host feature - we can just test denying localhost`

		`@@ -293,6 +328,42 @@`
		`rootdn_bind(topology_st.standalone, uri=uri)`


		`+ def test_rootdn_access_allowed_ip_wildcard(topology_st, rootdn_setup, rootdn_cleanup):`
		`+ """Test allowed ip feature`
		`+`
		`+ :id: c3e22c61-9ed2-4e89-8243-6ff686ecad9b`
		`+ :setup: Standalone instance, rootdn plugin set up`
		`+ :steps:`
		`+ 1. Set allowed ip to 255.255.255.255 - blocks the Root DN`
		`+ 2. Bind as Root DN`
		`+ 3. Allow 127.*`
		`+ 4. Bind as Root DN`
		`+ :expectedresults:`
		`+ 1. Success`
		`+ 2. Should fail`
		`+ 3. Success`
		`+ 4. Success`
		`+ """`
		`+`
		`+ log.info('Running test_rootdn_access_allowed_ip...')`
		`+`
		`+ # Set allowed ip to 255.255.255.255 - blocks the Root DN`
		`+ plugin.add_allow_ip('255.255.255.255')`
		`+ time.sleep(.5)`
		`+`
		`+ # Bind as Root DN - should fail`
		`+ uri = 'ldap://{}:{}'.format(localhost, topology_st.standalone.port)`
		`+ with pytest.raises(ldap.UNWILLING_TO_PERFORM):`
		`+ rootdn_bind(topology_st.standalone, uri=uri)`
		`+`
		`+ # Allow localhost`
		`+ plugin.add_allow_ip('127.*')`
		`+ time.sleep(.5)`
		`+`
		`+ # Bind should succeed`
		`+ rootdn_bind(topology_st.standalone, uri=uri)`
		`+`
		`+`
		`def test_rootdn_access_allowed_host(topology_st, rootdn_setup, rootdn_cleanup):`
		`"""Test allowed host feature`

ldap/servers/plugins/rootdn_access/rootdn_access.c

file modified

+1 -1

		`@@ -371,7 +371,7 @@`
		`}`
		`if (ips_tmp) {`
		`for (i = 0; ips_tmp[i] != NULL; i++) {`
		`- end = strspn(ips_tmp[i], "0123456789:ABCDEFabcdef.");`
		`+ end = strspn(ips_tmp[i], "0123456789:ABCDEFabcdef.*");`
		`if (!end \|\| ips_tmp[i][end] != '\0') {`
		`slapi_log_err(SLAPI_LOG_ERR, ROOTDN_PLUGIN_SUBSYSTEM, "rootdn_load_config - "`
		`"IP address contains invalid characters (%s), skipping\n",`

yrro commented 4 years ago

The changes in ticket 48027 caused IP addresses containing wildcards to
be rejected.

This changes the set of allowable characters to match that used for the
rootdn-deny-ip attribute.

firstyear commented 4 years ago

@mreynolds You seem to be the person who knows the most about this area (I think), did you mind taking a look? How would we construct a test to validate this behaviour?

tbordaz commented 4 years ago

Thanks @yrro for the finding and the patch. IMHO the fix looks good, it was a missing allowed wildcard that only affects the branch checking ip address. The 'host' branch accepts wildcard. Let's wait for @mreynolds final approval.

mreynolds commented 4 years ago

Not sure why the "*" was removed. @yrro did you confirm if this change works for you? If it works, then this gets my ack.

firstyear commented 4 years ago

@mreynolds Is it also worth getting some tests for this later? I'm not sure how we'd do it in py.test, we'd probably need @vashirov's advice here.

vashirov commented 4 years ago

We have a test suite for root DN plugin at https://pagure.io/389-ds-base/blob/master/f/dirsrvtests/tests/suites/plugins/rootdn_plugin_test.py

test_rootdn_config_validate can be extended with this particular case.

1 new commit added

WIP tests

4 years ago

firstyear commented 4 years ago

@yrro Is there anything we can do to help you with the test case development so we can merge this fix :)

yrro commented 4 years ago

I'm sorry to say I just haven't had time to set up an environment to build and run the tests yet, that's all.

mreynolds commented 3 years ago

This was a fixed in a different PR. Thanks for the contribution, closing this one...

Pull-Request has been closed by mreynolds

3 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3857

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 2

+71 -0

file changed

dirsrvtests/tests/suites/plugins/rootdn_plugin_test.py

+1 -1

file changed

ldap/servers/plugins/rootdn_access/rootdn_access.c

389-ds-base

Source Code

#50803 Issue 50800 - Fix parsing of wildcards in rootdn-allow-ip attribute Closed 3 years ago by spichugi. Opened 4 years ago by yrro. yrro/389-ds-base rootdn-allow-ip-wildcards into master

Metadata

Changes Summary 2

#50803 Issue 50800 - Fix parsing of wildcards in rootdn-allow-ip attribute

Closed 3 years ago by spichugi. Opened 4 years ago by yrro.

yrro/389-ds-base rootdn-allow-ip-wildcards into master