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

Support: Why does Event "Populate from..." trigger CSRF black-holed? #8204

Closed
1 task done
github-germ opened this issue Mar 11, 2022 · 40 comments
Closed
1 task done
Labels
needs triage This issue has been automatically labelled and needs further triage support

Comments

@github-germ
Copy link

Support Questions

When adding a new event and then attempting Populate from... with a freetext list of IPs, the browser responds with

You have tripped the cross-site request forgery protection of MISP

See error.log this triggers below:

How can we make this work?

Thanks!

MISP version

2.4.153

Operating System

RedHat

Operating System version

8.4

PHP version

7.4

Browser

Chrome

Browser version

No response

Relevant log output

2022-03-11 23:29:50 Error: [BadRequestException] The request has been black-holed
Request URL: /events/freeTextImport/292379?_=1647041377363
Stack Trace:
#0 /var/www/MISP/app/Lib/cakephp/lib/Cake/Controller/Component/SecurityComponent.php(831): AppController->blackhole()
#1 /var/www/MISP/app/Lib/cakephp/lib/Cake/Controller/Component/SecurityComponent.php(351): SecurityComponent->_callback()
#2 /var/www/MISP/app/Lib/cakephp/lib/Cake/Controller/Component/SecurityComponent.php(255): SecurityComponent->blackHole()
#3 /var/www/MISP/app/Lib/cakephp/lib/Cake/Utility/ObjectCollection.php(129): SecurityComponent->startup()
#4 /var/www/MISP/app/Lib/cakephp/lib/Cake/Event/CakeEventManager.php(244): ObjectCollection->trigger()
#5 /var/www/MISP/app/Lib/cakephp/lib/Cake/Controller/Controller.php(683): CakeEventManager->dispatch()
#6 /var/www/MISP/app/Lib/cakephp/lib/Cake/Routing/Dispatcher.php(189): Controller->startupProcess()
#7 /var/www/MISP/app/Lib/cakephp/lib/Cake/Routing/Dispatcher.php(167): Dispatcher->_invoke()
#8 /var/www/MISP/app/webroot/index.php(99): Dispatcher->dispatch()
#9 {main}

Extra attachments

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@github-germ github-germ added needs triage This issue has been automatically labelled and needs further triage support labels Mar 11, 2022
@github-germ
Copy link
Author

This has become a big issue for us as Populate from... is a common use case for our analysts. Anyone else seeing this?

@adulau
Copy link
Member

adulau commented Mar 17, 2022

Not really. I did some tests on recent version and I was not able to trigger it. Just wondering, is this only on that interface or can you trigger it at some other places?

@github-germ
Copy link
Author

@adulau -- really appreciate your attention on this!!

  • Our User Acceptance Test is seeing in Populate from..., Enrich Event, and perhaps other features.
  • We suspect these may be all the same root cause.
  • Did you test this on 2.4.153, which is our release? We did not see this in 2.4.148.

@github-germ
Copy link
Author

Just tested with 2.5.155 -- issue persists.
Building 2.4.156 now -- will test and report back later.

If this is not a MISP bug, then please help us understand how to diagnose to isolate root cause for the CSRF as this is a show stopper for a production deployment we have planned for this Monday.

As always, thanks a ton for all your help!!

@packet-rat
Copy link

I can also trigger it with [Publish].
So that's
(1) [Populate from]
(2) [Enrich]
and now,
(3) [Publish]

@packet-rat
Copy link

2022-03-17_15-58-50
2022-03-17_15-44-24
2022-03-17_15-44-41
2022-03-17_15-44-57

@packet-rat
Copy link

Any data can be pasted into Freetext Import window, it doesn't impact the outcome. Use "1.2.3.4" is/as required

@packet-rat
Copy link

(2) Enrich

2022-03-17_15-58-50
2022-03-17_17-03-34
2022-03-17_17-03-59
2022-03-17_17-04-10

@packet-rat
Copy link

Note that unlike previous spurious CSRF errors, these are constant and repeatable. Tested with multiple browsers - same repeatable outcomes. Most clicks to get to failed state are 1, 2, 3 (in seconds)

@github-germ
Copy link
Author

github-germ commented Mar 22, 2022

I'm new to CSRF detection but perhaps someone can guide me here... Thanks in advance!!

I've started adding debug prints to _validateCsrf in SecurityComponent.php. When printing the Session array I see the following difference with Populate from... and Add Attribute although the browser HTML form for both does have the hidden token details as below:

<div style="display:none;">
 <input type="hidden"
  name="data[_Token][fields]"
  value="71ebb078de0b3587a34c4a5f0ca776c9ec459911%3AAttribute.event_id"
  id="TokenFields1018849856"
  autocomplete="off">
<input type="hidden"
 name="data[_Token][unlocked]"
 value=""
 id="TokenUnlocked161348251"
 autocomplete="off">
</div>

However...

  • Add Attribute -- Session does have a populated _Token array and CSRF is not triggered
  • Populate from... -- Session does NOT contain a _Token array and therefore _validateCsrf throws SecurityException('CSRF token mismatch')

Does anyone have any hints as to why with Populate from... the _Token is missing from the Session which causes the $token declaration below to not be set triggering the exception, i.e.

$token = $this->Session->read('_Token');

@righel
Copy link
Contributor

righel commented Mar 23, 2022

Hello,
I been trying to reproduce this bug without success.

  1. Which type session storage are you using? PHP, Database or Redis?
  2. Could you please share your MISP session settings values:
    • Session.autoRegenerate
    • Session.checkAgent
    • Session.defaults
    • Session.timeout
    • Session.cookieTimeout
  3. Do you have a load balancer in front of your MISP instance or a HA setup?
  4. Could you please share the output of this command on the MISP instance?
    cat /etc/php/7.4/apache2/php.ini | grep -v '\;' | grep session
    *the location of the php.ini file could differ in your environment.

@github-germ
Copy link
Author

Hi @righel -- Thanks for assisting!! I also cannot reproduce this on a standalone Ubuntu VM running an off-the-shelf MISP 2.4.156. We see this in our MISP RH8.4 docker container in our enterprise environment.

What I find "interesting" is what appears to be the same HTML form hidden token details are POST-ed and something between that and _validateCsrf produces different Session content.

My day is just starting. I will dig in some more, and get back to you with the info you suggested looking at, and any more details I may see.

THANKS.

@github-germ
Copy link
Author

github-germ commented Mar 23, 2022

  1. MISP=2.4.156, OS=RH8.4, PHP=7.4.28, DB='8.0.19-commercial MySQL`, Redis=5.0.6
  2. Session settings: I guess you are looking for this in /var/www/MISP/app/Config/core.php
Configure::write('Session', array(
        'timeout'        => 60,    // Session timeout, default is 1 hour
        'cookie_timeout' => 10080 ,  // Cookie timeout, default is 1 week
        'defaults'       => 'php',
        'autoRegenerate' => false,
        'checkAgent' => false,
));
  1. In this MISP instance environment network inbound firewall is portmapping an alternate inbound port 48752 to 443 and the ip-src of the browsers is always the IP of the firewall. Our MISP.baseurl=https://FQDN:48752. Wondering if the POST request URL being relative and the referrer being absolute with the inbound alternate port (i.e. not 443) could cause this CSRF false positive? Here's a sample Apache log for the Populate from... submit:
10.x.y.z - user [23/Mar/2022:14:39:30 +0000] \
"POST /events/freeTextImport/293075?_=1648046356622 \
HTTP/1.1" 400 14288 \
"https://FQDN:48752/events/view/293075" \
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 \
(KHTML, like Gecko) Chrome/99.0.4844.74 Safari/537.36"
  1. php.ini details
# cat /etc/php.ini | grep -v '\;' | grep session
session.save_handler = redis
session.use_strict_mode = 0
session.use_cookies = 1
session.use_only_cookies = 1
session.name = PHPSESSID
session.auto_start = 0
session.cookie_lifetime = 0
session.cookie_path = /
session.cookie_domain =
session.cookie_httponly =
session.cookie_samesite =
session.serialize_handler = php
session.gc_probability = 1
session.gc_divisor = 1000
session.gc_maxlifetime = 1440
session.referer_check =
session.cache_limiter = nocache
session.cache_expire = 180
session.use_trans_sid = 0
session.sid_length = 26
session.trans_sid_tags = "a=href,area=href,frame=src,form="
session.sid_bits_per_character = 5
# 

@github-germ
Copy link
Author

github-germ commented Mar 23, 2022

Related to item 3 above, here's the POST for an Add Attribute which works (i.e. does not trigger CSRF). The Populate from... request URL (above) includes a QUERY_STRING (which I think is a timestamp) while the Add Attribute (below) does not

10.x.y.z - user [23/Mar/2022:14:51:18 +0000] \
"POST /attributes/add/293075 \
HTTP/1.1" 302 - \
"https://FQDN:48752/attributes/add/293075" \
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 \
(KHTML, like Gecko) Chrome/99.0.4844.74 Safari/537.36"

@righel
Copy link
Contributor

righel commented Mar 23, 2022

  1. These settings in the core.php can be overwritten if there is a different value set in /var/www/MISP/app/Config/config.php, please check that file too.
  2. Doing some investigation, will get back on this.
  3. If you are using the redis handler, this setting should also be present in the php.ini config file:
    session.save_path = "tcp://localhost:6379"
    *your redis host and port may differ
    I find strange you are able to log-in without this setting, maybe there's something configured elsewhere?

Assuming Redis is your php sessions backend, some debugging to check if the session is somehow destroyed:

  1. Log in into MISP
  2. Grab your session cookie value:
    image
  3. Connect to Redis and check the session id exists.
$ redis-cli
127.0.0.1:6379> keys PHPREDIS_SESSION:efphct2oq840na34sbtkgha6fg
1) "PHPREDIS_SESSION:efphct2oq840na34sbtkgha6fg"
127.0.0.1:6379> TTL PHPREDIS_SESSION:efphct2oq840na34sbtkgha6fg
(integer) 21447
127.0.0.1:6379> 
  1. Reproduce the bug that triggers the CSRF error.
  2. Check 3 again (redis-cli).

Is the bug reproducible on all users roles? Admin/Org admin/User ?

@righel
Copy link
Contributor

righel commented Mar 23, 2022

Following your hunch, @github-germ you could apply the below patch to this file:

echo $this->element('/genericElements/SideMenu/side_menu_link', array(
'element_id' => 'populateFrom',
'onClick' => array(
'function' => 'getPopup',
'params' => array($eventId, 'events', 'importChoice')
),
'text' => __('Populate from…')
));

patch:

diff --git a/app/View/Elements/genericElements/SideMenu/side_menu.ctp b/app/View/Elements/genericElements/SideMenu/side_menu.ctp
index 927ff93e4..20900fef7 100644
--- a/app/View/Elements/genericElements/SideMenu/side_menu.ctp
+++ b/app/View/Elements/genericElements/SideMenu/side_menu.ctp
@@ -135,8 +135,8 @@ $divider = $this->element('/genericElements/SideMenu/side_menu_divider');
                         echo $this->element('/genericElements/SideMenu/side_menu_link', array(
                             'element_id' => 'populateFrom',
                             'onClick' => array(
-                                'function' => 'getPopup',
-                                'params' => array($eventId, 'events', 'importChoice')
+                                'function' => 'genericPopup',
+                                'params' => array($baseurl . '/events/importChoice/' . $eventId, '#popover_form')
                             ),
                             'text' => __('Populate from…')
                         ));

This should prevent the creation of the ?_=TIMESTAMP query string parameter.
After that try to populate the event and check if the bug still occurs.

@github-germ
Copy link
Author

github-germ commented Mar 23, 2022

Re' the patch... That does remove the QUERY_STRING timestamp; however, that's the side menu click to get the importChoice which works fine. Don't we need to apply this to the freeTextImport pop-up form that is submitted?

When I read the function importChoice in EventsController.phpI see this but not sure where to make a similar change to remove the QUERY_STRING:

'freetext' => array(
        'url' => $this->baseurl . '/events/freeTextImport/' . $id,
        'text' => __('Freetext Import'),
        'ajax' => true,
        'target' => 'popover_form'
},

@github-germ
Copy link
Author

Thanks for your continued time and help!

  1. core.php Session possible overrides in config.php
# grep -i session config.php
    'authkey_keep_session' => false,
# 
  1. session.save_path in php.ini is not defined. We have 'CustomAuth_enable' => true for our user access.
# grep '.*session.save_path' /etc/php.ini
;     session.save_path = "N;/path"
;     session.save_path = "N;MODE;/path"
;       (see session.save_path above), then garbage collection does *not*
# grep '^[^;].*session.save_path' /etc/php.ini
# 
  1. The bug exists for multiple users. The ew I know of have role=admin. If you want me to try users with other settings, let me know.
  2. I will get back to you re' your test recommendations re' session cookie.

@righel
Copy link
Contributor

righel commented Mar 23, 2022

Sorry, got confused and thought the importChoice was failing too.
This patch on /var/www/MISP/app/webroot/js/misp.js,on top of the previous one, should remove the timestamp query string in the freeTextImport flow.

diff --git a/app/webroot/js/misp.js b/app/webroot/js/misp.js
index f19018b79..a0e175914 100644
--- a/app/webroot/js/misp.js
+++ b/app/webroot/js/misp.js
@@ -2737,7 +2737,7 @@ function importChoiceSelect(url, elementId, ajax) {
     if (ajax == 'false') {
         document.location.href = url;
     } else {
-        simplePopup(url);
+        genericPopup(url, "#popover_form");
     }
 }

NOTE: this is for debugging purposes, but should not be a fix, if it actually prevents the bug from happening, which I'm not very confident it will.

@github-germ
Copy link
Author

Patch.... I understand this is experimental. Here's the URL for the form which still has the QUERY_STRING

<form action="/events/freeTextImport/293075?_=1648064414011" id="id" method="post" accept-charset="utf-8">

@github-germ
Copy link
Author

github-germ commented Mar 23, 2022

Re' session cookie and redis-cli -- this is new to me so perhaps I'm am doing something wrong:

  • I see session cookie changes every 5 secs in browser which I guess is concurrent with GET /events/checkLocks/<eventid>/<timestamp>
  • I am not seeing any sessions in the redis container that our misp container is using
  • Note: there are 294 resque:* keys
127.0.0.1:6379> keys PHPREDIS_SESSION:*
(empty list or set)
127.0.0.1:6379> 

@github-germ
Copy link
Author

github-germ commented Mar 23, 2022

Check this out. I experimented with the following URL in the browser, entered an IP, and guess what? It worked. Notice I removed the QUERY_STRING with this test. So... let's see if you can help me remove that timstamp QUERY_STRING as that may be the fix:

  • https://FQDN:48752/events/freeTextImport/293075
    image

image

@righel
Copy link
Contributor

righel commented Mar 24, 2022

Patch.... I understand this is experimental. Here's the URL for the form which still has the QUERY_STRING

Hm, I tested a couple of times and it works on my end. Do a hard refresh of the site so the misp.js Javascript reloads, it may be cached.
Flow:
image
image
image

@righel
Copy link
Contributor

righel commented Mar 24, 2022

2. session.save_path in php.ini is not defined. We have 'CustomAuth_enable' => true for our user access.
May I ask what kind of CustomAuth? I guess there's something funky going on here with the sessions and Ajax requests.

  • I see session cookie changes every 5 secs in browser which I guess is concurrent with GET /events/checkLocks/<eventid>/<timestamp>
    I cannot reproduce this, each call to checkLocks keeps the same cookie session id, could this be something coming from your CustomAuth code?

The originator of the ?_=TIMESTMAP query is string is jQuery:
https://api.jquery.com/jquery.ajax/

cache (default: true, false for dataType 'script' and 'jsonp')
Type: Boolean
If set to false, it will force requested pages not to be cached by the browser. Note: Setting cache to false will only work correctly with HEAD and GET requests. It works by appending "_={timestamp}" to the GET parameters. The parameter is not needed for other types of requests, except in IE8 when a POST is made to a URL that has already been requested by a GET.

A simple query string parameter should not cause issues, unless it's triggering some kind of WAF but even in that case the outcome should be different.

@github-germ
Copy link
Author

github-germ commented Mar 24, 2022

Thanks for more details. Correct: browser hard-refresh reloaded that change in misp.js and the QUERY_STRING (?_=nnnn timestamp was removed). However, that still triggered CSRF.

Puzzling diffs:

  • Why does that fail when the same URL as the form action succeeds as I wrote above?
  • And why does Add Attribute which does not use a popup succeed?

After doing some more comparison probing for things that work and those that don't ...

THIS MIGHT BE RELATED TO ROOT CAUSE... Our CustomAuth is a corporate developed cookie-based system. Interesting that you do not see the token changed every 5 secs with checkLocks. Here's what I see when observing the cookie token with DevTools for these pages:

  • /events/view/<eventid> -- token revised every 5 secs; I see GET checkLocks in Apache log every 5 secs.
  • /attributes/add/<eventid> -- token not revised; no GET checkLocks in Apache log every 5 secs.
  • Populate from... with Freetext Import Tool popup loaded -- token revised every 5 secs; I see GET checkLocks in Apache log every 5 secs.
  • manual browser load of https://FQDN:48752/events/freeTextImport/<eventid> -- token not revised; no GET checkLocks in Apache log every 5 secs.

Based on this it does appear that the 5 sec revision of the cookie session token triggered by checkLocks may be root cause.

Do you think our CustomAuth causes this behavior, and if so, let me know if you have any hints on why that could be while I ponder and probe more here.

For example, does the browser sending the GET /events/checkLocks/<eventid>/<timestamp> traverse through our CustomAuth somehow (btw, the <timestamp> remains the same in that corrGET every 5 secs while on that page).

Hmmm, I do see these corresponding rows in the db logs table every 5 secs:

mysql> select * from logs order by id desc limit 1 \G
*************************** 1. row ***************************
         id: 672709381
      title: Successful authentication using <OUR> External authentication key
    created: 2022-03-24 14:22:29
      model: User
   model_id: 14
     action: auth
    user_id: 0
     change: HTTP method: GET
Target: /events/checkLocks/eventid>/<timestamp>
      email: <user>@local.net
        org: <ORG>
description:
         ip: 10.<x.y.z>
1 row in set (0.00 sec)

mysql>

@github-germ
Copy link
Author

github-germ commented Mar 24, 2022

Maybe I can try a test to disable the following in event_contents.ctp which calls queryEventLock in misp.js to stop the token revisions:

$(function () {
queryEventLock('<?= h($event['Event']['id']); ?>', <?= (int)$event['Event']['timestamp'] ?>);
popoverStartup();

OK, yes, that stopped the CSRF!!! So, this proves that the cookie token revision causes the false CSRF trigger. Now, I realize checkLocks is enabled to detect if the event has changed. But now that we see the cause, need to understand how to prevent checkLocks from causing the token update.

@github-germ
Copy link
Author

github-germ commented Mar 24, 2022

With checkLocks again activated the browser's DevTools shows that token cookie with a expiration always 5 secs from "now". If I understood you, it is that token that is used when testing CSRF. If that's true, and the expectation is that it should not be revised, what is causing it to expire?
image

@github-germ
Copy link
Author

github-germ commented Mar 24, 2022

When I test same on my MISP 2.4.156 in a local VM that is not using CustomAuth:

  • I see checkLocks every 5 secs as expected;
  • However, the token cookie does not change and has the expiration to to +1 hours from "now".

@righel
Copy link
Contributor

righel commented Mar 24, 2022

Indeed when Plugin.CustomAuth_enable the session is review in every request:

$this->Session->renew();

So what's probably happening is:

  1. You load the Events page and the CSRF tokens are saved to your session.
  2. Every 5 seconds checkLocks is called via Ajax and a new session is created, so the CSRF token of the previous session is lost.

Then this code gets executed:
https://github.com/MISP/cakephp/blob/7f64ea37f96b9ace5a0cb8c97832963c50b21ab4/lib/Cake/Model/Datasource/CakeSession.php#L779-L786

  • session_regenerate_id(true) deletes the previous session. And it somehow results in a new session cookie with a expiration date of now+5sec (unexpected)

Now I have a proper setup where I can reproduce the bug, I'll keep investigating and come back with some results later/tomorrow.

Thanks for taking the time to share your findings.

@github-germ
Copy link
Author

Excellent!!!! Thanks again for sticking with me on this journey.

@packet-rat
Copy link

I always hated that annoying 'checkLocks'. clutters up logs, Web Developer debug. measurements, etc.

'Successful authentication using MyLogins External authentication key | HTTP method: GETTarget: /events/checkLocks/nnnnnnn/nnnnn'

yada....yada....yada...Wash-rinse-repeat...

@packet-rat
Copy link

;-)

@iglocska
Copy link
Member

I always hated that annoying 'checkLocks'. clutters up logs, Web Developer debug. measurements, etc.

'Successful authentication using MyLogins External authentication key | HTTP method: GETTarget: /events/checkLocks/nnnnnnn/nnnnn'

yada....yada....yada...Wash-rinse-repeat...

Yeah I hate it too, makes debugging frustrating at best.

iglocska added a commit that referenced this issue Mar 25, 2022
- it's annoying and causes headaches
- as discussed in #8204
@iglocska
Copy link
Member

Added a setting to disable it, look for MISP.disable_event_locks

@iglocska
Copy link
Member

Also, for the issue identified by @righel, perhaps enabling Security.authkey_keep_session could help in general.

@github-germ
Copy link
Author

I will now test this change and report back. THANKS!!!

Btw, setting Security.authkey_keep_session = true before the patch is applied did not help.

@github-germ
Copy link
Author

!!
W
O
R
K
S
!!

@github-germ
Copy link
Author

@righel Your support on this one has been world-class. Thank you VERY much!! And @iglocska thanks for jumping in.

I imagine this will be included in 2.4.157 once that is officially released. Our Production OPs policy will require some exception bureaucracy on our end to deploy a patch. Might .157 be offered soon?

@github-germ
Copy link
Author

2.5.157 released! You guys are the best. Case closed. THANKS AGAIN.

@vletoux
Copy link

vletoux commented Mar 20, 2023

I got a similar issue.
Basically, session cookie were enforced with "HttpOnly" and we had http headers authentication (so a new session was created at each jquery call, prohiting the load of the CSRF token server side).

I've open a suggestion in cakephp to improve the error message
MISP/cakephp#2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has been automatically labelled and needs further triage support
Projects
None yet
Development

No branches or pull requests

6 participants