[dev.icinga.com #6831] Remove broken attribute-based authorization

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/6831

Created by oreggin on 2014-08-01 08:48:47 +00:00

Assignee: ricardo
Status: Resolved (closed on 2015-08-04 18:29:07 +00:00)
Target Version: 1.14
Last Update: 2015-08-04 18:29:07 +00:00 (in Redmine)

Icinga Version: 1.7.1 and above
OS Version: any

---

Hi Folks,

a few years ago we developed attribute based authorization for nagios. Then icinga takes over that patch, nagios team didn't, so we patched our own nagios cgi instances until we migrating to icinga for some months ago. We use Debian stable (icinga ver 1.7.1) monitoring server and we notice that icinga's cgis doesn't authorize well as is should. The issue is the following.

We have cgiauth.cfg, assume that it has 10 lines of rules and at the and there is a "sysop=::w"
When a user attribute matching the 5th rows, cgis shows the hosts (or members of hostgroups) listed in the 5th row but (and here is the problem) not only these hosts will be showing but the hosts in the remained (6-10) lines too and as the last line is "sysop=::w" every user who has a match in either attribute in cgiauth.cfg will see every hosts in icinga hosts.cfg.

More describe:
cgiauth.cfg:

urn:geant:niif.hu:netmonitor:attr1=sw1.site1:*:r
urn:geant:niif.hu:netmonitor:attr2=sw1.site2,rtr1.site2:*:r
urn:geant:niif.hu:netmonitor:attr3=sw1.site3,sw2.site3:*:r
urn:geant:niif.hu:netmonitor:attr4=sw1.site4:*:r
urn:geant:niif.hu:netmonitor:attr5=sw1.site5,rtr1.site5:*:r
urn:geant:niif.hu:netmonitor:attr6=sw1.site6,sw2.site6:*:r
urn:geant:niif.hu:netmonitor:attr7=sw1.site7:*:r
urn:geant:niif.hu:netmonitor:attr8=sw1.site8,rtr1.site8:*:r
urn:geant:niif.hu:netmonitor:attr9=sw1.site9,sw2.site9:*:r
urn:geant:niif.hu:netmonitor:sysop=::w

When comes a user who has an attribute in entitlement field: "urn:geant:niif.hu:netmonitor:attr5" will see all the hosts because of the last line. If I remove the last line from cgiauth.cfg then the user will see the hosts for site5, site6, site7, site8, site9.

I compared the cgiauth.c between nagios-3.2.1 and icinga-1.7.1 and noticed that the most part os the code was rewritten so I don't know where the error is. Compared it between icinga-1.7.1 and icinga-1.11.1 and I don't see any fix related to this issue.

Cheers,
oreggin

Changesets

2015-04-30 20:21:07 +00:00 by ricardo e4e3979

Classic UI: Add DEPRECATED note for attribute based authorization to cgi.cfg

Refs: #6831

2015-04-30 20:28:06 +00:00 by ricardo 4adff3e

Classic UI: removed attribute based authorization entirely

Refs: #6831

2015-08-04 18:28:36 +00:00 by ricardo 5fe21ba

Classic UI: Remove attribute based authorization entirely

fixes #6831

2016-09-22 18:42:19 +00:00 by mskuta da46d25

Remove references to cgiauth.cfg

Template sample-config/cgiauth.cfg.in was deleted in fix for #6831,
but not all references to the file it produced (cgiauth.cfg) were erased.

fixes #6831

Signed-off-by: Michael Friedrich <michael.friedrich@netways.de>

[icinga-migration]

Updated by mfriedrich on 2014-08-01 08:59:09 +00:00

Tbh I took that patch, tested it a bit, and there was no feedback in years by anyone. Since you're the author of it, I'd appreciate a fix by yourself then sent upstream. If not, I'd rather remove/disable the feature if it's broken.

[icinga-migration]

Updated by oreggin on 2014-08-01 09:16:27 +00:00

Hi dnsmichi,

Thanks for your reply!

What was the conception behind the rewrite? There is no sense to apply a patch in production code if you or someone rewrite it later without consequences and testing ;) Debian has not the bleeding edge package repository, so if you have made a modification in the code, it will not comes to the enduser immediately.
BTW we use this feature daily and we can test it if someone made a modification on it.
If you don't want to fix your own rewrite and there are no other users who use this feature, then there is no cause to hold it in icinga because we must use our own patch in any way and we have no more options :(

Regards,
oreggin

[icinga-migration]

Updated by mfriedrich on 2014-08-01 09:33:07 +00:00

I didn't modify the authorization system, Ricardo did large code rewrites. He may tell more about the intentions and ideas about it. So to speak, he's the lead developer and may decide what to do.

I personally don't use the cgiauth patch, and am in progress in moving forward to the in-development icinga web 2 for my development boxes. Although I'd be happy if someone fixes your problem, best by a patch applied to 1.11.x. And as said, since you're the author who had the idea about it, it is more than reasonable that you do the fix.

[icinga-migration]

Updated by oreggin on 2014-08-01 09:48:54 +00:00

Thanks for your reply dnsmichi,

I'm not the author of this feature but my collegue. He knows C language ;) We thinks over to possibilities and will made a decision what is the easiest way for us to move forward :)
As I see Icinga Classical UI has is C/binary based CGIs. Icinga WEB is an alternative UI for it and PHP based? If it is, it has the migrated attribute based authorization code as well?

Cheers,
oreggin

[icinga-migration]

Updated by mfriedrich on 2014-08-01 09:51:55 +00:00

I haven't looked into that feature set direction for some years, as I changed jobs and don't have any shibboleth around to testdrive too.

Maybe it's reasonable to add a feature request (or sponsor one). But I would rather do that for Icinga Web 2 once it becomes ready - https://dev.icinga.org/projects/icingaweb2?jump=welcome

[icinga-migration]

Updated by gbeutner on 2014-08-04 06:31:23 +00:00

• Subject changed from attribute based authorization doeasn't works good to Attribute-based authorization doesn't work well

[icinga-migration]

Updated by mfriedrich on 2014-10-24 22:54:16 +00:00

• Priority changed from Normal to Low

[icinga-migration]

Updated by ricardo on 2014-11-13 19:39:24 +00:00

Hi,

can you send your current path you are using in your environment?

Cheers
Ricardo

[icinga-migration]

Updated by mfriedrich on 2014-12-08 09:22:04 +00:00

• Project changed from 19 to Core, Classic UI, IDOUtils
• Category changed from 84 to Classic UI
• OS Version set to any

[icinga-migration]

Updated by ricardo on 2014-12-08 09:45:58 +00:00

• Status changed from New to Feedback

Hi oreggin,

If it's OK for you we will declare this feature DEPRECATED and remove it in 1.14.
We don't have an environment to test this and we would need someone else to test and maintain.

Therefore it's be better to remove this broken bit.

Cheers
Ricardo

[icinga-migration]

Updated by matecs@niif.hu on 2014-12-08 10:08:43 +00:00

hi,
i'm currently trying to make a patch for this authorization code so
please wait a bit more... :)
thanks,
cs

[icinga-migration]

Updated by ricardo on 2014-12-08 10:11:24 +00:00

\o/

that's great.

Thank you

Cheers
Ricardo

[icinga-migration]

Updated by mfriedrich on 2015-01-24 14:58:01 +00:00

• Status changed from Feedback to New

[icinga-migration]

Updated by mfriedrich on 2015-03-12 19:02:46 +00:00

If you're planning to submit a patch any time soon, please consider using the git master as base, and keep in mind it will target 1.14 then. If not, I'll close the issue.

[icinga-migration]

Updated by ricardo on 2015-04-25 19:30:10 +00:00

• Status changed from New to Feedback

Hi,

after putting some thought into it my suggestion is to mark this feature DEPRECATED and remove it in one of the next version.

A: it's not working
B: no one noticed that it is broken except you
C: even if you supply a working patch, no one in the Icinga team will be able to maintain it.
D: You should look into using Icinga 2 / Icinga Web 2

jm2c
Ricardo

[icinga-migration]

Updated by mfriedrich on 2015-04-26 07:17:11 +00:00

• Status changed from Feedback to Assigned
• Assigned to set to ricardo

+1

@ricardo

Can you add a deprecation warning for 1.13.3 for now please? I'll pick that into 1.13.3 then. We might remove it entirely in 1.14

[icinga-migration]

Updated by berk on 2015-05-18 12:18:04 +00:00

• Target Version set to Backlog

[icinga-migration]

Updated by mfriedrich on 2015-06-02 12:06:00 +00:00

• Subject changed from Attribute-based authorization doesn't work well to Remove broken attribute-based authorization
• Target Version changed from Backlog to 1.14

[icinga-migration]

Updated by ricardo on 2015-08-04 18:29:07 +00:00

• Status changed from Assigned to Resolved
• Done % changed from 0 to 100

Applied in changeset 5fe21ba.