Opened 7 years ago

Closed 7 years ago

#1559 closed bug (fixed)

SSL not used for sending passwords in 'Verify member'

Reported by: elf-pavlik Owned by: planetcruiser
Priority: critical Milestone: 0.5.6 - bugfixing
Component: unknown Keywords: security
Cc: meinhard

Description

at this moment we send both passwords over unsecure connection, i guess we can just simply alter form to use https

Change History (9)

comment:1 Changed 7 years ago by elf-pavlik

  • Milestone changed from unassigned to 0.5.6 - bugfixing

comment:2 Changed 7 years ago by micha

  • Owner set to micha
  • Status changed from new to accepted

comment:3 Changed 7 years ago by micha

  • follow_up changed from none to review code

Changes commited in 31f548db. Please test. Unfortunately, on localhost and alpha this can't be tested properly. Any idea how to unsure it works before moving it to live?

comment:4 Changed 7 years ago by elf-pavlik

why we don't use ssl on alpha? getting cert from startssl shouldn't make a problem... i would also feel comfortable with using SNI on alpha to not 'waste' IPs

comment:5 Changed 7 years ago by planetcruiser

@elf:

ssl on alpha works fine, see https://alpha.bewelcome.org/

@micha:

not sure what your fix does, but if i fill in the form https://alpha.bewelcome.org/verification/Abyssin and press send i get to a http (as in not https) response page. in the html source code it says:

<form name="entermembertoverify" action="http://alpha.bewelcome.org/verification/Abyssin" id="prepareverifymember" method="post">

..so that can't work. want to take another look?

comment:6 follow-up: Changed 7 years ago by elf-pavlik

@planetcruiser

ssl on alpha works indeed but since you use cert for www it gives nasty warning which i don't like people get habit of ignoring... how about generating another cert for alpha to please browsers and don't contribute to developing possibly dangerous habits among people using alpha?

@micha

i remember looking at diff of related commit and you have quite 'crazy' conditional there checking for subdomains - https://gitorious.org/bewelcome/rox/commit/31f548dbffb2a5d1e07d611d3240fab453caa086 i would suggest refactoring to have 'current_environment' set somewhere and than just check in_array(current_enviroment, ['live', 'alpha']) or something like that... me not very php ;)

comment:7 in reply to: ↑ 6 Changed 7 years ago by planetcruiser

Replying to elf-pavlik:

@planetcruiser

ssl on alpha works indeed but since you use cert for www it gives nasty warning which i don't like people get habit of ignoring... how about generating another cert for alpha to please browsers and don't contribute to developing possibly dangerous habits among people using alpha?

i see where you are coming from and i agree, but even if i generate a cert for alpha browsers will still warn, because we are using the same ip address for www and alpha. so this won't solve it. however, i suggest to move this to a separate ticket. want to create one? :)

@micha

i remember looking at diff of related commit and you have quite 'crazy' conditional there checking for subdomains - https://gitorious.org/bewelcome/rox/commit/31f548dbffb2a5d1e07d611d3240fab453caa086 i would suggest refactoring to have 'current_environment' set somewhere and than just check in_array(current_enviroment, ['live', 'alpha']) or something like that... me not very php ;)

hehe, the hack is pretty funky indeed. you are testing for strrpos($page_url, 'alpha') === false - so this can't work on alpha.

i suggest to at least add a TODO comment with a suggestion on how this could be done better. a "force_https" property in the rox routing for the verification page would be cool. so if someone opens http://www.bewelcome.org/verification/<user> they would automatically be redirected to https://www.bewelcome.org/verification/<user> and we could keep the empty form action.

comment:8 Changed 7 years ago by planetcruiser

  • Owner changed from micha to planetcruiser

taking over

comment:9 Changed 7 years ago by planetcruiser

  • Resolution set to fixed
  • Status changed from accepted to closed

fixed via https://gitorious.org/bewelcome/rox/commit/779342d52fb79de4e2dd9e5df6e038a31d26fef4 - i used a config option instead of guessing around hostnames

deployed and tested on alpha - note: your passwords are being sent to a HTTPS page, but you are redirected to a HTTP page afterwards, so you don't actually see the encrypted page - this can only be solved in a bigger refactoring of either routing or all verification steps

Note: See TracTickets for help on using tickets.