Bug 484828

Created attachment 331394 [details]
corrected version of op.cgi, restricts approval requests to those allocated to his/her role(/groups)

Description of problem:
op.cgi uses different code to both (docroot/agent/request) index.cgi and read.cgi for retrieving a request from the $queue. As a result, contrary to the other two scripts it fails to enforce limitations on RA's to only approve requests assigned to their group/role.

I also noticed two cases where the database/queue connection could be left hanging (open) and added $queue->close(); calls immediately before the "return" statements in question.

Version-Release number of selected component (if applicable):
yum info pki-ra says
Version: 1.0.0
Release: 2.fc8


How reproducible:
every time.

Steps to Reproduce:
1. configure CS.cfg to assign different requests (user/server) to different (ra agent interface) groups OR as in our case modify the autoassign plugin to assign different requests to different agent groups.
2. add agents to the agent groups
3. edit CS.cfg to authorize the new agent groups i.e. add the new groups to:
agent.authorized_groups=admin,agent,serverAgents,userAgents
4. request a certificate that gets assigned to one agent group e.g. serverAgents
5. notice the list and read screens correctly show the request for agents in the correct group (e.g. serverAgents) and don't show to other agents
6. as the other agent (e.g. an agent not in the serverAgents), paste the approve url for the new request into your browser.
7. The request is approved, when it should not be.
  
Actual results:
The request is approved, when it should not be.

Expected results:
The approval request should be ignore and potentially logged.

Additional info:

The fix is simple and based on the other similar cgi's. I have tested and it works for both the success and failure case. The fix is no more elegant than the code in the index.cgi and read.cgi - but works similarly. Input validation across the RA interface is poor. I am not a perl programmer so someone should check it, but it is a very simple fix.

Here is a diff fixed_file orig_file
 diff op.cgi op.cgi.orig 
71d70
< 
80,87c79
<   #Security bug, previously allowed agents to approve requests 
<   #they can't see replace with same code as index.cgi and read.cgi
<   #to limit approvals to requests assigned to their role/group.
<   #my $pref = $queue->read_request($id);
<   my @roles = $self->get_current_roles($cfg);
<   my $pref = $queue->read_request_by_roles(\@roles, $id);
<   #consider adding an error check and redirect
< 
---
>   my $pref = $queue->read_request($id);
92d83
<       $queue->close();    #close previously hanging database connection
100d90
<       $queue->close();    #close previously hanging database connection

I am attaching the fixed version of the file.