From 9db633c111f7c4fb4bdb096de8d0e6c124b09c8f Mon Sep 17 00:00:00 2001 From: Sebastian Marcet Date: Thu, 17 Mar 2016 15:14:49 -0300 Subject: [PATCH] Bug Fixing * fixed bug that prevent translate/ref-stack be able to log in * removed some debug logs * updated DH param checking routine * updated unit tests Change-Id: I7b49167b06bdeceaf6a7e634cac7e9250f0a1553 --- .../openid/OpenIdProviderController.php | 2 -- .../OpenIdDHAssociationSessionRequest.php | 4 +-- app/services/utils/CheckPointService.php | 4 +-- .../IndirectResponseQueryStringStrategy.php | 2 +- .../IndirectResponseUrlFragmentStrategy.php | 2 +- app/tests/OpenIdProtocolTest.php | 25 ++++++++++++++++++- 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/app/controllers/openid/OpenIdProviderController.php b/app/controllers/openid/OpenIdProviderController.php index 96649a7f..121fe757 100644 --- a/app/controllers/openid/OpenIdProviderController.php +++ b/app/controllers/openid/OpenIdProviderController.php @@ -41,8 +41,6 @@ class OpenIdProviderController extends BaseController { $msg = new OpenIdMessage( Input::all() ); - $r = print_r (Input::all()); - Log::debug($r); if($this->memento_service->exists()){ $msg = OpenIdMessage::buildFromMemento( $this->memento_service->load()); } diff --git a/app/libs/openid/requests/OpenIdDHAssociationSessionRequest.php b/app/libs/openid/requests/OpenIdDHAssociationSessionRequest.php index 014de360..901ad32d 100644 --- a/app/libs/openid/requests/OpenIdDHAssociationSessionRequest.php +++ b/app/libs/openid/requests/OpenIdDHAssociationSessionRequest.php @@ -56,8 +56,8 @@ class OpenIdDHAssociationSessionRequest extends OpenIdAssociationSessionRequest $dh_gen = $this->getDHGen(); $dh_consumer_public = $this->getDHConsumerPublic(); - if (!empty($dh_modulus) && !empty($dh_gen) && !empty($dh_consumer_public)) - return true; + if (empty($dh_modulus) || empty($dh_gen) || empty($dh_consumer_public)) + return false; if (!preg_match('/^\d+$/', $dh_modulus) || $dh_modulus < 11) { return false; diff --git a/app/services/utils/CheckPointService.php b/app/services/utils/CheckPointService.php index a3a85469..a3e989f4 100644 --- a/app/services/utils/CheckPointService.php +++ b/app/services/utils/CheckPointService.php @@ -48,12 +48,12 @@ class CheckPointService implements ICheckPointService $user_trail = new UserExceptionTrail(); $user_trail->from_ip = $remote_ip; $user_trail->exception_type = $class_name; - $user_trail->stack_trace = $ex->getTraceAsString(); + $user_trail->stack_trace = $ex->getTraceAsString(); if(Auth::check()){ $user_trail->user_id = Auth::user()->getId(); } $user_trail->Save(); - Log::error(sprintf("* CheckPointService - exception : << %s >> - IP Address: %s",$ex->getMessage(),$remote_ip)); + Log::warning(sprintf("* CheckPointService - exception : << %s >> - IP Address: %s",$ex->getMessage(),$remote_ip)); //applying policies foreach ($this->policies as $policy) { $policy->apply($ex); diff --git a/app/strategies/IndirectResponseQueryStringStrategy.php b/app/strategies/IndirectResponseQueryStringStrategy.php index 145b85a6..af2a0f32 100644 --- a/app/strategies/IndirectResponseQueryStringStrategy.php +++ b/app/strategies/IndirectResponseQueryStringStrategy.php @@ -28,7 +28,7 @@ class IndirectResponseQueryStringStrategy implements IHttpResponseStrategy return Response::view('404', array(), 404); } $return_to = (strpos($return_to, "?") == false) ? $return_to . "?" . $query_string : $return_to . "&" . $query_string; - Log::debug(sprintf("IndirectResponseQueryStringStrategy: return_to %s", $return_to)); + return Redirect::to($return_to) ->header('Cache-Control', 'no-cache, no-store, max-age=0, must-revalidate') ->header('Pragma','no-cache'); diff --git a/app/strategies/IndirectResponseUrlFragmentStrategy.php b/app/strategies/IndirectResponseUrlFragmentStrategy.php index b6561e94..692d4a70 100644 --- a/app/strategies/IndirectResponseUrlFragmentStrategy.php +++ b/app/strategies/IndirectResponseUrlFragmentStrategy.php @@ -28,7 +28,7 @@ class IndirectResponseUrlFragmentStrategy implements IHttpResponseStrategy } $return_to = (strpos($return_to, "#") == false) ? $return_to . "#" . $fragment : $return_to . "&" . $fragment; - Log::debug(sprintf("IndirectResponseUrlFragmentStrategy: return_to %s", $return_to)); + return Redirect::to($return_to) ->header('Cache-Control', 'no-cache, no-store, max-age=0, must-revalidate') ->header('Pragma','no-cache'); diff --git a/app/tests/OpenIdProtocolTest.php b/app/tests/OpenIdProtocolTest.php index 5851b45f..28fad3a6 100644 --- a/app/tests/OpenIdProtocolTest.php +++ b/app/tests/OpenIdProtocolTest.php @@ -26,7 +26,7 @@ class OpenIdProtocolTest extends OpenStackIDBaseTest public function __construct() { //DH openid values - $this->g = '2'; + $this->g = '1'; $this->private = '84009535308644335779530519631942543663544485189066558731295758689838227409144125540638118058012144795574289866857191302071807568041343083679600155026066530597177004145874642611724010339353151653679189142289183802715816551715563883085859667759854344959305451172754264893136955464706052993052626766687910313992'; $this->public = '93500922748114712465435925279613158240858799671601934136793652488458659380414896628304484614933937038790006320444306607890979422427297815641372302594684991758687126229761033142429422299990743006497200988301031430937819368909849994628108111270360657896230712920491471398605159969300956278883668998797148755353'; $this->mod = '155172898181473697471232257763715539915724801966915404479707795314057629378541917580651227423698188993727816152646631438561595825688188889951272158842675419950341258706556549803580104870537681476726513255747040765857479291291572334510643245094715007229621094194349783925984760375594985848253359305585439638443'; @@ -341,6 +341,29 @@ class OpenIdProtocolTest extends OpenStackIDBaseTest $this->assertTrue(!empty($openid_response[OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId)])); } + public function testAuthenticationSetupModeSessionAssociationDHSha256InvalidParams() + { + + $b64_public = base64_encode(OpenIdCryptoHelper::convert($this->public, DiffieHellman::FORMAT_NUMBER, + DiffieHellman::FORMAT_BTWOC)); + + $this->assertTrue($b64_public === 'AIUmVPMheb/hEupD5m6veEEstnBVteyZPy+mlYX7ygxygLG/XuHFa8q4lZERJ9u1DNFOpXHRDq5RbjsaUYRDOtyrbkGbeKo5tPqjsynjXtoMAItxkxCU4jpQLvH85P+u7DeA0h3kKNHFa90ijZTIGSSDRF5wW9N+QPCUCt4G4xWZ'); + + $params = array( + OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS) => OpenIdProtocol::OpenID2MessageType, + OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode) => OpenIdProtocol::AssociateMode, + OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_AssocType) => OpenIdProtocol::SignatureAlgorithmHMAC_SHA256, + OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_SessionType) => OpenIdProtocol::AssociationSessionTypeDHSHA256, + OpenIdProtocol::param(OpenIdProtocol::OpenIdProtocol_DHGen) => base64_encode(OpenIdCryptoHelper::convert(1, DiffieHellman::FORMAT_NUMBER, DiffieHellman::FORMAT_BTWOC)), + OpenIdProtocol::param(OpenIdProtocol::OpenIdProtocol_DHModulus) => base64_encode(OpenIdCryptoHelper::convert(PHP_INT_MAX, DiffieHellman::FORMAT_NUMBER, DiffieHellman::FORMAT_BTWOC)), + OpenIdProtocol::param(OpenIdProtocol::OpenIdProtocol_DHConsumerPublic) => $b64_public, + ); + + $response = $this->action("POST", "OpenIdProviderController@endpoint", $params); + + $this->assertResponseStatus(400); + + } public function testAuthenticationCheckImmediateAuthenticationPrivateSession() {