Mercurial > p > roundup > code

comparison roundup/cgi/client.py @ 5220:14d8f61e6ef2

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Reimplemented anti-csrf measures by raising exceptions rather than returning booleans. Redoing it using exceptions was the easiest way to return proper xmlrpc fault messages to the clients. Also this code should now properly make values set in the form override values from the database. So no lost work under some circumstances if the csrf requirements are not met. Also this code does a better job of cleaning up old csrf tokens.
author John Rouillard <rouilj@ieee.org>
date Wed, 05 Apr 2017 20:56:08 -0400
parents 44f7e6b958fe
children 8743b7226dc7
comparison
equal deleted inserted replaced
5219:ade4bbc2716d 5220:14d8f61e6ef2
45 45
46 from email.MIMEBase import MIMEBase 46 from email.MIMEBase import MIMEBase
47 from email.MIMEText import MIMEText 47 from email.MIMEText import MIMEText
48 from email.MIMEMultipart import MIMEMultipart 48 from email.MIMEMultipart import MIMEMultipart
49 import roundup.anypy.email_ 49 import roundup.anypy.email_
50 import xmlrpclib
50 51
51 def initialiseSecurity(security): 52 def initialiseSecurity(security):
52 '''Create some Permissions and Roles on the security object 53 '''Create some Permissions and Roles on the security object
53 54
54 This function is directly invoked by security.Security.__init__() 55 This function is directly invoked by security.Security.__init__()
434 self.determine_language() 435 self.determine_language()
435 # Open the database as the correct user. 436 # Open the database as the correct user.
436 self.determine_user() 437 self.determine_user()
437 self.check_anonymous_access() 438 self.check_anonymous_access()
438 439
439 if self.handle_csrf(xmlrpc=True): 440 try:
440 # Call the appropriate XML-RPC method. 441 # coverting from function returning true/false to
442 # raising exceptions
443 # Call csrf with xmlrpc checks enabled.
444 # It will return True if everything is ok,
445 # raises exception on check failure.
446 csrf_ok = self.handle_csrf(xmlrpc=True)
447 except (Unauthorised, UsageError), msg:
448 # report exception back to server
449 exc_type, exc_value, exc_tb = sys.exc_info()
450 output = xmlrpclib.dumps(
451 xmlrpclib.Fault(1, "%s:%s" % (exc_type, exc_value)),
452 allow_none=True)
453 csrf_ok = False # we had an error, failed check
454
455 if csrf_ok == True:
441 handler = xmlrpc.RoundupDispatcher(self.db, 456 handler = xmlrpc.RoundupDispatcher(self.db,
442 self.instance.actions, 457 self.instance.actions,
443 self.translator, 458 self.translator,
444 allow_none=True) 459 allow_none=True)
445 output = handler.dispatch(input) 460 output = handler.dispatch(input)
446 else:
447 raise Unauthorised, self._error_message
448 461
449 self.setHeader("Content-Type", "text/xml") 462 self.setHeader("Content-Type", "text/xml")
450 self.setHeader("Content-Length", str(len(output))) 463 self.setHeader("Content-Length", str(len(output)))
451 self.write(output) 464 self.write(output)
452 465
508 # Roundup's real web interface (not a file being served up) 521 # Roundup's real web interface (not a file being served up)
509 # so do the Anonymous Web Acess check now 522 # so do the Anonymous Web Acess check now
510 self.check_anonymous_access() 523 self.check_anonymous_access()
511 524
512 # check for a valid csrf token identifying the right user 525 # check for a valid csrf token identifying the right user
513 if self.handle_csrf(): 526 csrf_ok = True
527 try:
528 # coverting from function returning true/false to
529 # raising exceptions
530 csrf_ok = self.handle_csrf()
531 except (UsageError, Unauthorised), msg:
532 csrf_ok = False
533 self.form_wins = True
534 self._error_message = msg
535
536 if csrf_ok:
514 # csrf checks pass. Run actions etc. 537 # csrf checks pass. Run actions etc.
515 # possibly handle a form submit action (may change 538 # possibly handle a form submit action (may change
516 # self.classname and self.template, and may also 539 # self.classname and self.template, and may also
517 # append error/ok_messages) 540 # append error/ok_messages)
518 html = self.handle_action() 541 html = self.handle_action()
941 # We have a nonce being used with a method it should 964 # We have a nonce being used with a method it should
942 # not be. If the nonce exists, report to admin so they 965 # not be. If the nonce exists, report to admin so they
943 # can fix the nonce leakage and destroy it. (nonces 966 # can fix the nonce leakage and destroy it. (nonces
944 # used in a get are more exposed than those used in a 967 # used in a get are more exposed than those used in a
945 # post.) Note, I don't attempt to validate here since 968 # post.) Note, I don't attempt to validate here since
946 # existance here is the sign of a failure. If nonce 969 # existence here is the sign of a failure. If nonce
947 # exists try to report the referer header to try to 970 # exists try to report the referer header to try to
948 # find where this comes from so it can be fixed. If 971 # find where this comes from so it can be fixed. If
949 # nonce doesn't exist just ignore it. Maybe we should 972 # nonce doesn't exist just ignore it. Maybe we should
950 # report, but somebody could spam us with a ton of 973 # report, but somebody could spam us with a ton of
951 # invalid keys and fill up the logs. 974 # invalid keys and fill up the logs.
958 logger.error( 981 logger.error(
959 self._("csrf key used with wrong method from: %s"), 982 self._("csrf key used with wrong method from: %s"),
960 referer) 983 referer)
961 otks.destroy(key) 984 otks.destroy(key)
962 self.db.commit() 985 self.db.commit()
986 # do return here. Keys have been obsoleted.
987 # we didn't do a expire cycle of session keys,
988 # but that's ok.
963 return True 989 return True
964 990
965 config=self.instance.config 991 config=self.instance.config
966 user=self.db.getuid() 992 current_user=self.db.getuid()
967 993
968 # the HTTP headers we check. 994 # List HTTP headers we check. Note that the xmlrpc header is
969 # Note that the xmlrpc header is missing. Its enforcement is 995 # missing. Its enforcement is different (yes/required are the
970 # different (yes/required are the same for example) 996 # same for example) so we don't include here.
971 # so we don't include here.
972 header_names = [ 997 header_names = [
973 "ORIGIN", 998 "ORIGIN",
974 "REFERER", 999 "REFERER",
975 "X-FORWARDED-HOST", 1000 "X-FORWARDED-HOST",
976 "HOST" 1001 "HOST"
977 ] 1002 ]
978 1003
979 header_pass = 0 # count of passing header checks 1004 header_pass = 0 # count of passing header checks
980 1005
981 # if any headers are required and missing, report 1006 # If required headers are missing, raise an error
982 # an error
983 for header in header_names: 1007 for header in header_names:
984 if (config["WEB_CSRF_ENFORCE_HEADER_%s"%header] == 'required' 1008 if (config["WEB_CSRF_ENFORCE_HEADER_%s"%header] == 'required'
985 and "HTTP_%s"%header not in self.env): 1009 and "HTTP_%s"%header not in self.env):
986 self.add_error_message(self._("Missing header: %s")%header) 1010 logger.error(self._("csrf header %s required but missing for user%s."), header, current_user)
987 logger.error(self._("csrf header %s required but missing for user%s."), header, user) 1011 raise Unauthorised, self._("Missing header: %s")%header
988 return False
989 1012
990 # if you change these make sure to consider what
991 # happens if header variable exists but is empty.
992 # self.base.find("") returns 0 for example not -1
993 #
994 # self.base always matches: ^https?://hostname 1013 # self.base always matches: ^https?://hostname
995 enforce=config['WEB_CSRF_ENFORCE_HEADER_ORIGIN']
996 if 'HTTP_ORIGIN' in self.env and enforce != "no":
997 origin = self.env['HTTP_ORIGIN']
998 foundat = self.base.find(origin +'/')
999 if foundat != 0:
1000 if enforce in ('required', 'yes'):
1001 self.add_error_message("Invalid Origin %s"%origin)
1002 logger.error(self._("csrf Origin header check failed for user%s. Value=%s"), user, origin)
1003 return False
1004 elif enforce == 'logfailure':
1005 logger.warning(self._("csrf Origin header check failed for user%s. Value=%s"), user, origin)
1006 else:
1007 header_pass += 1
1008
1009 enforce=config['WEB_CSRF_ENFORCE_HEADER_REFERER'] 1014 enforce=config['WEB_CSRF_ENFORCE_HEADER_REFERER']
1010 if 'HTTP_REFERER' in self.env and enforce != "no": 1015 if 'HTTP_REFERER' in self.env and enforce != "no":
1011 referer = self.env['HTTP_REFERER'] 1016 referer = self.env['HTTP_REFERER']
1012 # self.base always has trailing / 1017 # self.base always has trailing /
1013 foundat = referer.find(self.base) 1018 foundat = referer.find(self.base)
1014 if foundat != 0: 1019 if foundat != 0:
1015 if enforce in ('required', 'yes'): 1020 if enforce in ('required', 'yes'):
1016 self.add_error_message(self._("Invalid Referer %s, %s")%(referer,self.base)) 1021 logger.error(self._("csrf Referer header check failed for user%s. Value=%s"), current_user, referer)
1017 logger.error(self._("csrf Referer header check failed for user%s. Value=%s"), user, referer) 1022 raise Unauthorised, self._("Invalid Referer %s, %s")%(referer,self.base)
1018 return False
1019 elif enforce == 'logfailure': 1023 elif enforce == 'logfailure':
1020 logger.warning(self._("csrf Referer header check failed for user%s. Value=%s"), user, referer) 1024 logger.warning(self._("csrf Referer header check failed for user%s. Value=%s"), current_user, referer)
1021 else: 1025 else:
1022 header_pass += 1 1026 header_pass += 1
1023 1027
1024 enforce=config['WEB_CSRF_ENFORCE_HEADER_X-FORWARDED-HOST'] 1028 # if you change these make sure to consider what
1025 if 'HTTP_X-FORWARDED-HOST' in self.env and enforce != "no": 1029 # happens if header variable exists but is empty.
1026 host = self.env['HTTP_X-FORWARDED-HOST'] 1030 # self.base.find("") returns 0 for example not -1
1027 foundat = self.base.find('://' + host + '/') 1031 enforce=config['WEB_CSRF_ENFORCE_HEADER_ORIGIN']
1028 # 4 means self.base has http:/ prefix, 5 means https:/ prefix 1032 if 'HTTP_ORIGIN' in self.env and enforce != "no":
1029 if foundat not in [4, 5]: 1033 origin = self.env['HTTP_ORIGIN']
1034 foundat = self.base.find(origin +'/')
1035 if foundat != 0:
1030 if enforce in ('required', 'yes'): 1036 if enforce in ('required', 'yes'):
1031 self.add_error_message(self._("Invalid X-FORWARDED-HOST %s")%host) 1037 logger.error(self._("csrf Origin header check failed for user%s. Value=%s"), current_user, origin)
1032 logger.error(self._("csrf X-FORWARDED-HOST header check failed for user%s. Value=%s"), user, host) 1038 raise Unauthorised, self._("Invalid Origin %s"%origin)
1033 return False
1034 elif enforce == 'logfailure': 1039 elif enforce == 'logfailure':
1035 logger.warning(self._("csrf X-FORWARDED-HOST header check failed for user%s. Value=%s"), user, host) 1040 logger.warning(self._("csrf Origin header check failed for user%s. Value=%s"), current_user, origin)
1036 else: 1041 else:
1037 header_pass += 1 1042 header_pass += 1
1043
1044 enforce=config['WEB_CSRF_ENFORCE_HEADER_X-FORWARDED-HOST']
1045 if 'HTTP_X-FORWARDED-HOST' in self.env:
1046 if enforce != "no":
1047 host = self.env['HTTP_X-FORWARDED-HOST']
1048 foundat = self.base.find('://' + host + '/')
1049 # 4 means self.base has http:/ prefix, 5 means https:/ prefix
1050 if foundat not in [4, 5]:
1051 if enforce in ('required', 'yes'):
1052 logger.error(self._("csrf X-FORWARDED-HOST header check failed for user%s. Value=%s"), current_user, host)
1053 raise Unauthorised, self._("Invalid X-FORWARDED-HOST %s")%host
1054 elif enforce == 'logfailure':
1055 logger.warning(self._("csrf X-FORWARDED-HOST header check failed for user%s. Value=%s"), current_user, host)
1056 else:
1057 header_pass += 1
1038 else: 1058 else:
1039 # https://seclab.stanford.edu/websec/csrf/csrf.pdf 1059 # https://seclab.stanford.edu/websec/csrf/csrf.pdf
1040 # recommends checking HTTP HOST header as well. 1060 # recommends checking HTTP HOST header as well.
1041 # If there is an X-FORWARDED-HOST header, check 1061 # If there is an X-FORWARDED-HOST header, check
1042 # that only. The proxy setting X-F-H has probably set 1062 # that only. The proxy setting X-F-H has probably set
1047 host = self.env['HTTP_HOST'] 1067 host = self.env['HTTP_HOST']
1048 foundat = self.base.find('://' + host + '/') 1068 foundat = self.base.find('://' + host + '/')
1049 # 4 means http:// prefix, 5 means https:// prefix 1069 # 4 means http:// prefix, 5 means https:// prefix
1050 if foundat not in [4, 5]: 1070 if foundat not in [4, 5]:
1051 if enforce in ('required', 'yes'): 1071 if enforce in ('required', 'yes'):
1052 self.add_error_message(self._("Invalid HOST %s")%host) 1072 logger.error(self._("csrf HOST header check failed for user%s. Value=%s"), current_user, host)
1053 logger.error(self._("csrf HOST header check failed for user%s. Value=%s"), user, host) 1073 raise Unauthorised, self._("Invalid HOST %s")%host
1054 return False
1055 elif enforce == 'logfailure': 1074 elif enforce == 'logfailure':
1056 logger.warning(self._("csrf HOST header check failed for user%s. Value=%s"), user, host) 1075 logger.warning(self._("csrf HOST header check failed for user%s. Value=%s"), current_user, host)
1057 else: 1076 else:
1058 header_pass += 1 1077 header_pass += 1
1059 1078
1060 enforce=config['WEB_CSRF_HEADER_MIN_COUNT'] 1079 enforce=config['WEB_CSRF_HEADER_MIN_COUNT']
1061 if header_pass < enforce: 1080 if header_pass < enforce:
1062 self.add_error_message(self._("Unable to verify sufficient headers"))
1063 logger.error(self._("Csrf: unable to verify sufficient headers")) 1081 logger.error(self._("Csrf: unable to verify sufficient headers"))
1064 return False 1082 raise UsageError, self._("Unable to verify sufficient headers")
1065 1083
1066 enforce=config['WEB_CSRF_ENFORCE_HEADER_X-REQUESTED-WITH'] 1084 enforce=config['WEB_CSRF_ENFORCE_HEADER_X-REQUESTED-WITH']
1067 if xmlrpc: 1085 if xmlrpc:
1068 if enforce in ['required', 'yes']: 1086 if enforce in ['required', 'yes']:
1069 # if we get here we have usually passed at least one 1087 # if we get here we have usually passed at least one
1072 # E.G. X-Requested-With: XMLHttpRequest 1090 # E.G. X-Requested-With: XMLHttpRequest
1073 # Note we do not use CSRF nonces for xmlrpc requests. 1091 # Note we do not use CSRF nonces for xmlrpc requests.
1074 # 1092 #
1075 # see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers 1093 # see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers
1076 if 'HTTP_X-REQUESTED-WITH' not in self.env: 1094 if 'HTTP_X-REQUESTED-WITH' not in self.env:
1077 logger.error(self._("csrf X-REQUESTED-WITH xmlrpc required header check failed for user%s."), user) 1095 logger.error(self._("csrf X-REQUESTED-WITH xmlrpc required header check failed for user%s."), current_user)
1078 raise UsageError, "Required Header Missing" 1096 raise UsageError, self._("Required Header Missing")
1079 return False # unreached
1080 else:
1081 return True
1082 else:
1083 return True
1084
1085 enforce=config['WEB_CSRF_ENFORCE_TOKEN']
1086 if "@csrf" not in self.form:
1087 if enforce == 'required':
1088 self.add_error_message(self._("No csrf token found"))
1089 logger.error(self._("required csrf field missing for user%s"), user)
1090 return False
1091 else:
1092 if enforce == 'logfailure':
1093 # FIXME include url
1094 logger.warning(self._("required csrf field missing for user%s"), user)
1095 return True
1096 1097
1097 # Expire old csrf tokens now so we don't use them. These will 1098 # Expire old csrf tokens now so we don't use them. These will
1098 # be committed after the otks.destroy below. Note that the 1099 # be committed after the otks.destroy below. Note that the
1099 # self.clean_up run as part of determine_user() will run only 1100 # self.clean_up run as part of determine_user() will run only
1100 # once an hour. If we have short lived (e.g. 5 minute) keys 1101 # once an hour. If we have short lived (e.g. 5 minute) keys
1101 # they will live too long if we depend on clean_up. So we do 1102 # they will live too long if we depend on clean_up. So we do
1102 # our own. 1103 # our own.
1103 otks.clean() 1104 otks.clean()
1104 1105
1105 key=self.form['@csrf'].value 1106 if xmlrpc:
1106 uid = otks.get(key, 'uid', default=None) 1107 # Save removal of expired keys from database.
1107 sid = otks.get(key, 'sid', default=None) 1108 self.db.commit()
1108 1109 # Return from here since we have done housekeeping
1109 # The key has been used or compromised. 1110 # and don't use csrf tokens for xmlrpc.
1110 # Delete it to prevent replay. 1111 return True
1111 otks.destroy(key) 1112
1113 # process @csrf tokens past this point.
1114 key=None
1115 nonce_user = None
1116 nonce_session = None
1117
1118 if '@csrf' in self.form:
1119 key=self.form['@csrf'].value
1120
1121 nonce_user = otks.get(key, 'uid', default=None)
1122 nonce_session = otks.get(key, 'sid', default=None)
1123 # The key has been used or compromised.
1124 # Delete it to prevent replay.
1125 otks.destroy(key)
1126
1127 # commit the deletion/expiration of all keys
1112 self.db.commit() 1128 self.db.commit()
1129
1130 enforce=config['WEB_CSRF_ENFORCE_TOKEN']
1131 if key is None: # we do not have an @csrf token
1132 if enforce == 'required':
1133 logger.error(self._("Required csrf field missing for user%s"), current_user)
1134 raise UsageError, self._("Csrf token is missing.")
1135 elif enforce == 'logfailure':
1136 # FIXME include url
1137 logger.warning(self._("csrf field not supplied by user%s"), current_user)
1138 else:
1139 # enforce is either yes or no. Both permit change if token is
1140 # missing
1141 return True
1113 1142
1114 current_session = self.session_api._sid 1143 current_session = self.session_api._sid
1115 1144
1116 ''' 1145 '''
1117 # I think now that LogoutAction redirects to 1146 # I think now that LogoutAction redirects to
1125 # Handle the case where user logs out and tries to 1154 # Handle the case where user logs out and tries to
1126 # log in again in same window. 1155 # log in again in same window.
1127 # The csrf token for the login button is associated 1156 # The csrf token for the login button is associated
1128 # with the prior login, so it will not validate. 1157 # with the prior login, so it will not validate.
1129 # 1158 #
1130 # To bypass error, Verify that uid != user and that 1159 # To bypass error, Verify that nonce_user != user and that
1131 # user is '2' (anonymous) and there is no current 1160 # user is '2' (anonymous) and there is no current
1132 # session key. Validate that the csrf exists 1161 # session key. Validate that the csrf exists
1133 # in the db and uid and sid are not None. 1162 # in the db and nonce_user and nonce_session are not None.
1134 # Also validate that the action is Login. 1163 # Also validate that the action is Login.
1135 # Lastly requre at least one csrf header check to pass. 1164 # Lastly requre at least one csrf header check to pass.
1136 # If all of those work process the login. 1165 # If all of those work process the login.
1137 if user != uid and \ 1166 if current_user != nonce_user and \
1138 user == '2' and \ 1167 current_user == '2' and \
1139 current_session is None and \ 1168 current_session is None and \
1140 uid is not None and \ 1169 nonce_user is not None and \
1141 sid is not None and \ 1170 nonce_session is not None and \
1142 "@action" in self.form and \ 1171 "@action" in self.form and \
1143 self.form["@action"].value == "Login": 1172 self.form["@action"].value == "Login":
1144 if header_pass > 0: 1173 if header_pass > 0:
1145 otks.destroy(key) 1174 otks.destroy(key)
1146 self.db.commit() 1175 self.db.commit()
1147 return True 1176 return True
1148 else: 1177 else:
1149 self.add_error_message("Reload window before logging in.") 1178 self.add_error_message("Reload window before logging in.")
1150 ''' 1179 '''
1151
1152 # validate against user and session 1180 # validate against user and session
1153 if uid != user: 1181 if current_user != nonce_user:
1154 if enforce in ('required', "yes"): 1182 if enforce in ('required', "yes"):
1155 self.add_error_message(self._("Invalid csrf token found: %s")%key) 1183 logger.error(
1156 logger.error(self._("csrf uid mismatch for user%s."), user) 1184 self._("Csrf mismatch user: current user %s != stored user %s, current session, stored session: %s,%s for key %s."),
1157 return False 1185 current_user, nonce_user, current_session, nonce_session, key)
1186 raise UsageError, self._("Invalid csrf token found: %s")%key
1158 elif enforce == 'logfailure': 1187 elif enforce == 'logfailure':
1159 logger.warning(self._("csrf uid mismatch for user%s."), user) 1188 logger.warning(
1160 if current_session != sid: 1189 self._("logged only: Csrf mismatch user: current user %s != stored user %s, current session, stored session: %s,%s for key %s."),
1190 current_user, nonce_user, current_session, nonce_session, key)
1191 if current_session != nonce_session:
1161 if enforce in ('required', "yes"): 1192 if enforce in ('required', "yes"):
1162 self.add_error_message(self._( 1193 logger.error(
1163 "Invalid csrf session found: %s")%key) 1194 self._("Csrf mismatch user: current session %s != stored session %s, current user/stored user is: %s for key %s."),
1164 logger.error(self._("csrf session mismatch for user%s."), user) 1195 current_session, nonce_session, current_user, key)
1165 return False 1196 raise UsageError, self._("Invalid csrf session found: %s")%key
1166 elif enforce == 'logfailure': 1197 elif enforce == 'logfailure':
1167 logger.warning(self._("csrf session mismatch for user%s."), user) 1198 logger.warning(
1199 self._("logged only: Csrf mismatch user: current session %s != stored session %s, current user/stored user is: %s for key %s."),
1200 current_session, nonce_session, current_user, key)
1201 # we are done and the change can occur.
1168 return True 1202 return True
1169 1203
1170 def opendb(self, username): 1204 def opendb(self, username):
1171 """Open the database and set the current user. 1205 """Open the database and set the current user.
1172 1206
Roundup Issue Tracker: http://roundup-tracker.org/