search

cyberperspectives / sagacity

Security Assessment Data Management and Analysis Tool
http://www.cyberperspectives.com
Apache License 2.0
38 stars 13 forks source link

CKL Exports with Duplicate PDI IDs #78

Closed JeffOdegard closed 5 years ago

JeffOdegard commented 5 years ago

We never did open a bug for this. We have 3 hosts with the McAfee Virus Scan Local Client STIG applied, export an eChecklist, fill it out, import it, and then export ckls, and the ckls have several duplicate PDIs. Chasing them down, they are all PDIs where there is the same STIG ID with another entry in the GoldDisk Table from a similar checklist (McAfee Managed Client STIG). Oddly, with only the Local Client STIG attached, Sagacity also exported a ckl for the Manage Client STIG.

Importing the following eChecklist should reproduce the results. If you check, there are a several PDIs that are duplicated in the ckl.

Matt has another bug in the same checklist I haven't yet been able to reproduce - where two Not a Finding PDIs (DTAM162, 163)get changed to Not Reviewed. I'll post another bug for that when I can reproduce.

Win7-McAfee-original-eChecklist-1.xlsx

godsgood33 commented 5 years ago

So it looks like one issue is that when I add the McAfee VS Local Client STIG for some reason it adds the Managed Client STIG as a Orphan. That is probably one reason why the STIGs are being duplicated. Couple troubleshooting steps...

The secondary problem that Matt was probably running into was probably because the STIG status was being overwritten by the Managed Client STIG.

If what I think is happening is happening, then I think we should push this off to the next release as this would be cleared by up rewriting the eChecklist export methods to not export the Orphan findings. If this HAS to be handled, then it is going to take me a while to track it down because this is getting into very complicated territory with the SQL queries. In splitting the export functionality, it will allow me to simplify the queries quite a bit.

JeffOdegard commented 5 years ago

I'm afraid it has to be fixed. It's both changing random findings and creating unusable ckl files.

Why are the PDIs for the managed client STIG being added? It seems like that is the root of the problem - not the orphan checklist itself.

On Wed, Dec 5, 2018 at 12:48 PM Ryan P notifications@github.com wrote:

So it looks like one issue is that when I add the McAfee VS Local Client STIG for some reason it adds the Managed Client STIG as a Orphan. That is probably one reason why the STIGs are being duplicated. Couple troubleshooting steps...

  • On a fresh target (only OS assigned), add the McAfee VS Local Client STIG.
  • Export the target to an eChecklist
  • Verify the Orphan worksheet appears and delete it
  • Change statuses to some of the McAfee STIGs
  • Reimport the eChecklist
  • Review target to see if the Managed Client STIG was added
  • Export the target CKL file and see if the data is incorrect.

The secondary problem that Matt was probably running into was probably because the STIG status was being overwritten by the Managed Client STIG.

If what I think is happening is happening, then I think we should push this off to the next release as this would be cleared by up rewriting the eChecklist export methods to not export the Orphan findings. If this HAS to be handled, then it is going to take me a while to track it down because this is getting into very complicated territory with the SQL queries. In splitting the export functionality, it will allow me to simplify the queries quite a bit.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cyberperspectives/sagacity/issues/78#issuecomment-444620613, or mute the thread https://github.com/notifications/unsubscribe-auth/Aoyyh94RFn-7mViJnViHcJcaTAQALKRsks5u2CMhgaJpZM4ZDc1f .

--

Jeff Odegard jeff.odegard@gmail.com

[image: LinkedIn] https://www.linkedin.com/in/jeffodegard/ [image: Fakebook] https://www.facebook.com/jeff.odegard.98 [image: YouTube] https://www.youtube.com/user/OdegardOnline

godsgood33 commented 5 years ago

Yes, that is the root of the problem and that is what I'm trying to get you to test to see if we have a work around. If deleting the orphan worksheet before reimporting fixes the issue as far as creating duplicate STIGs or overwriting statuses, then we can work with that until I can separate the orphan export.

JeffOdegard commented 5 years ago

After talking with Matt, I agree that we should just suppress exporting or importing the orphan checklist for now.

On Wed, Dec 5, 2018 at 1:24 PM Ryan P notifications@github.com wrote:

Yes, that is the root of the problem and that is what I'm trying to get you to test to see if we have a work around. If deleting the orphan worksheet before reimporting fixes the issue as far as creating duplicate STIGs or overwriting statuses, then we can work with that until I can separate the orphan export.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cyberperspectives/sagacity/issues/78#issuecomment-444633427, or mute the thread https://github.com/notifications/unsubscribe-auth/Aoyyh5um-rJpdH4-pszWiUr-IEwxUhY3ks5u2CuHgaJpZM4ZDc1f .

--

Jeff Odegard jeff.odegard@gmail.com

[image: LinkedIn] https://www.linkedin.com/in/jeffodegard/ [image: Fakebook] https://www.facebook.com/jeff.odegard.98 [image: YouTube] https://www.youtube.com/user/OdegardOnline

JeffOdegard commented 5 years ago

On a fresh target (only OS assigned), add the McAfee VS Local Client STIG.

Export the target to an eChecklist

Verify the Orphan worksheet appears and delete it

Change statuses to some of the McAfee STIGs

Reimport the eChecklist

Review target to see if the Managed Client STIG was added

Export the target CKL file and see if the data is incorrect.

To test our theory that it's when new hosts are created on eChecklist import, I modified the eChecklist so all the hostnames were foobar, then imported it.

JeffOdegard commented 5 years ago

I'm still seeing duplicates. I imported the two eChecklists in the attached zip file and then exported ckls, I ran my de-duplication script and had the script output found in the output.txt file. The broken folder has all the original ckl's with duplicates.

ckl-dups-21Dec18.zip

JeffOdegard commented 5 years ago

I just re-verified the existence of this bug using the instructions from 21 Dec 18.

JeffOdegard commented 5 years ago

In export-ckl.php, get_checklist_data() (line 370), if you can include a GROUP BY STIG_ID, it cuts it down to the correct number of findings. I'm not sure how to include that using your db helper, but it might work.

My resulting SQL query for the McAfee AV Local Client STIG is:

SELECT pdi., pcl., s.description AS Description FROM sagacity.pdi LEFT JOIN sagacity.pdi_checklist_lookup pcl ON pcl.pdi_id = pdi.pdi_id LEFT JOIN sagacity.target_checklist tc ON tc.chk_id = pcl.checklist_id LEFT JOIN sagacity.stigs s ON s.pdi_id = pdi.pdi_id WHERE tc.tgt_id = '1' AND tc.chk_id = '191' GROUP BY STIG_ID;

This returns 87 lines, vice the 117 lines returned currently (without the GROUP BY).

JeffOdegard commented 5 years ago

Matt's Solution (reversing table order - probably the better solution):

SELECT pdi., pcl., s.description AS Description FROM sagacity.pdi_checklist_lookup pcl LEFT JOIN sagacity.pdi ON pcl.pdi_id = pdi.pdi_id LEFT JOIN sagacity.target_checklist tc ON tc.chk_id = pcl.checklist_id LEFT JOIN sagacity.stigs s ON s.pdi_id = pdi.pdi_id WHERE tc.tgt_id = '1' AND tc.chk_id = '191' GROUP BY STIG_ID;

mshuter commented 5 years ago

So after playing some more I realized that it's not a JOIN order issue like I thought. My solution only works because I forgot to remove Jeff's "GROUP BY" before I tested, so they're equivalent.

On examination, I believe the root cause is that we're not capturing checklist id as a foreign key when we create records in the golddisk table. We would need that checklist ID to make sure that when we query the golddisk table, only results from the checklist we're trying to populate get returned. Short of that, I don't think there's any good solution (at least not better than Jeff's).

The problem with this is that it's going to result in a random VMS ID being chosen from among the results with the same PDI_ID (whichever happens to be first). This may not be a big impact, but if any other tools (like VMS, eMASS or whatever the ASCA uses) try to key on that VMS_ID, it's going to result in issues.

godsgood33 commented 5 years ago

Added a group by statement and committed. I agree...the best fix will require capturing the checklist id and saving that with the VMS ID. With that we'd be able to just pull where the checklist ID equals what we are pulling. Much simpler. But it will require changing the database and STIG reading routines.

JeffOdegard commented 5 years ago

The latest fix, using the group by STIG_ID, eliminates duplicates, but there is a risk that the VMS ID for the duplicates may be incorrect (e.g. from another similar checklist - like the two McAfee AV checklists). However, I think the best long term solution for this will be in the re-write - going to a flatter database structure (one table for STIG checks, including STIG ID, VMS ID, CCI, etc). Even if we have duplicate STIG ID's in the table, a select including the checklist ID should give us more precise information. This will lead to less complicated database queries, easier troubleshooting, better performance, and simpler code.