comparison roundup/cgi/client.py @ 5210:7da56980754d

Remove csrf keys used with get The original code didn't delete keys used with a get. Detect this and invalidate the keys. Get keys are more likely to leak (in urls etc.) so they have to be removed once used. Also a little code cleanup and added testing.
author John Rouillard <rouilj@ieee.org>
date Sun, 19 Mar 2017 15:32:14 -0400
parents 47bd81998ddc
children f4b6a2a3e605
comparison
equal deleted inserted replaced
5209:729e703a6c5a 5210:7da56980754d
927 given the newness of this code notifying the 927 given the newness of this code notifying the
928 user and having them notify the admins for 928 user and having them notify the admins for
929 debugging seems to be an advantage. 929 debugging seems to be an advantage.
930 930
931 ''' 931 '''
932 # Create the otks handle here as we need it almost immediately.
933 # If this is perf issue, set to None here and check below
934 # once all header checks have passed if it needs to be opened.
935 otks=self.db.getOTKManager()
936
932 # Assume: never allow changes via GET 937 # Assume: never allow changes via GET
933 if self.env['REQUEST_METHOD'] not in ['POST', 'PUT', 'DELETE']: 938 if self.env['REQUEST_METHOD'] not in ['POST', 'PUT', 'DELETE']:
939 if "@csrf" in self.form:
940 # We have a nonce being used with a method it should
941 # not be. If the nonce exists, report to admin so they
942 # can fix the nonce leakage and destroy it. (nonces
943 # used in a get are more exposed than those used in a
944 # post.) Note, I don't attempt to validate here since
945 # existance here is the sign of a failure. If nonce
946 # exists try to report the referer header to try to
947 # find where this comes from so it can be fixed. If
948 # nonce doesn't exist just ignore it. Maybe we should
949 # report, but somebody could spam us with a ton of
950 # invalid keys and fill up the logs.
951 if 'HTTP_REFERER' in self.env:
952 referer = self.env['HTTP_REFERER']
953 else:
954 referer = self._("Referer header not available.")
955 key=self.form['@csrf'].value
956 if otks.exists(key):
957 logger.error(
958 self._("csrf key used with wrong method from: %s"),
959 referer)
960 otks.destroy(key)
961 self.db.commit()
934 return True 962 return True
963
935 config=self.instance.config 964 config=self.instance.config
936 user=self.db.getuid() 965 user=self.db.getuid()
937 966
938 # the HTTP headers we check. 967 # the HTTP headers we check.
939 # Note that the xmlrpc header is missing. Its enforcement is 968 # Note that the xmlrpc header is missing. Its enforcement is
1043 if 'HTTP_X-REQUESTED-WITH' not in self.env: 1072 if 'HTTP_X-REQUESTED-WITH' not in self.env:
1044 self.add_error_message("Required Header Missing") 1073 self.add_error_message("Required Header Missing")
1045 logger.error(self._("csrf X-REQUESED-WITH xmlrpc required header check failed for user%s."), user) 1074 logger.error(self._("csrf X-REQUESED-WITH xmlrpc required header check failed for user%s."), user)
1046 return False 1075 return False
1047 1076
1048 otks=self.db.getOTKManager()
1049 enforce=config['WEB_CSRF_ENFORCE_TOKEN'] 1077 enforce=config['WEB_CSRF_ENFORCE_TOKEN']
1050 if "@csrf" not in self.form: 1078 if "@csrf" not in self.form:
1051 if enforce == 'required': 1079 if enforce == 'required':
1052 self.add_error_message(self._("No csrf token found")) 1080 self.add_error_message(self._("No csrf token found"))
1053 logger.error(self._("required csrf field missing for user%s"), user) 1081 logger.error(self._("required csrf field missing for user%s"), user)
1059 return True 1087 return True
1060 1088
1061 key=self.form['@csrf'].value 1089 key=self.form['@csrf'].value
1062 uid = otks.get(key, 'uid', default=None) 1090 uid = otks.get(key, 'uid', default=None)
1063 sid = otks.get(key, 'sid', default=None) 1091 sid = otks.get(key, 'sid', default=None)
1092 if __debug__:
1093 ts = otks.get(key, '__timestamp', default=None)
1094 print("Found key %s for user%s sess: %s, ts %s, time %s"%(key, uid, sid, ts, time.time()))
1064 current_session = self.session_api._sid 1095 current_session = self.session_api._sid
1065 # validate against user and session 1096
1097 # The key has been used or compromised. Delete it to prevent replay.
1098 otks.destroy(key)
1099 self.db.commit()
1066 1100
1067 ''' 1101 '''
1068 # I think now that LogoutAction redirects to 1102 # I think now that LogoutAction redirects to
1069 # self.base ([tracker] web parameter in config.ini), 1103 # self.base ([tracker] web parameter in config.ini),
1070 # this code is not needed. However I am keeping it 1104 # this code is not needed. However I am keeping it
1098 return True 1132 return True
1099 else: 1133 else:
1100 self.add_error_message("Reload window before logging in.") 1134 self.add_error_message("Reload window before logging in.")
1101 ''' 1135 '''
1102 1136
1103 # The key has been used or compromised. Delete it to prevent replay. 1137 # validate against user and session
1104 otks.destroy(key)
1105 self.db.commit()
1106
1107 if uid != user: 1138 if uid != user:
1108 if enforce in ('required', "yes"): 1139 if enforce in ('required', "yes"):
1109 self.add_error_message(self._("Invalid csrf token found: %s")%key) 1140 self.add_error_message(self._("Invalid csrf token found: %s")%key)
1141 logger.error(self._("csrf uid mismatch for user%s."), user)
1110 return False 1142 return False
1111 elif enforce == 'logfailure': 1143 elif enforce == 'logfailure':
1112 logger.warning(self._("csrf uid mismatch for user%s."), user) 1144 logger.warning(self._("csrf uid mismatch for user%s."), user)
1113 if current_session != sid: 1145 if current_session != sid:
1114 if enforce in ('required', "yes"): 1146 if enforce in ('required', "yes"):
1115 self.add_error_message(self._( 1147 self.add_error_message(self._(
1116 "Invalid csrf session found: %s")%key) 1148 "Invalid csrf session found: %s")%key)
1149 logger.error(self._("csrf session mismatch for user%s."), user)
1117 return False 1150 return False
1118 elif enforce == 'logfailure': 1151 elif enforce == 'logfailure':
1119 logger.warning(self._("csrf session mismatch for user%s."), user) 1152 logger.warning(self._("csrf session mismatch for user%s."), user)
1120 return True 1153 return True
1121 1154

Roundup Issue Tracker: http://roundup-tracker.org/