Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplicated policy entries #2738

Open
jpereira opened this issue Jun 11, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@jpereira
Copy link
Contributor

commented Jun 11, 2019

Issue type

  • Defect - Unexpected behavior (obvious or verified by project member).

Defect

How to reproduce the issue

Today we are allowing the user to redeclare a policy entry.

[root@docker-freeradius-ubuntu16 freeradius-server-master.git]# grep ^filter_password -A4 /opt/freeradius4/etc/raddb/policy.d/filter 
filter_password {
	if (&User-Password && \
	   (&User-Password != "%{string:User-Password}")) {
		update request {
			&Tmp-String-0 := "%{string:User-Password}"
--
filter_password {
	accept
}

[root@docker-freeradius-ubuntu16 freeradius-server-master.git]#

It will cause a false-positive behavior. then, we should verify and raise an error if exist any policy name duplicated.

@jpereira

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

What do you think @alandekok ?

@arr2036 arr2036 added the v4.0.x label Jun 11, 2019

@arr2036

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

It's not clear that this is incorrect behaviour, I'm pretty ambivalent... If the first entry took precedence then I'd be absolutely on the side of it being a defect, as the second entry takes precedence, then there could be some use in keeping this to allow site specific overrides, i.e. for some configurations include a local policy.d directory after the main raddb/policy.d directory. Still worth discussing to figure out what we should do.

@arr2036

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

I guess behaviour with modules sets precedence. We don't allow two modules with the same instance name? Or at least I hope we don't.

@jpereira

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

@arr2036 we do not allow to declare the same module instance twice.

e.g: I replicate the linelog log_accounting { ...} entry and the result is:

Tue Jun 11 01:34:11 2019: Duplicate module "log_accounting" in file /opt/freeradius4/etc/raddb/mods-enabled/linelog[405] and file /opt/freeradius4/etc/raddb/mods-enabled/linelog[372]

so, in my perspective, we should do the same behavior for policies. Based on my experience today, it was terrible to figure out that the same policy was duplicated. (I never imagined that it could be the reason)

@arr2036

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

That definitely makes it more in favour of disallowing duplicate policies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.