mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Implement SMS MFA
Summary: Depends on D20021. Ref T13222. This has a few rough edges, including: - The challenges theselves are CSRF-able. - You can go disable/edit your contact number after setting up SMS MFA and lock yourself out of your account. - SMS doesn't require MFA so an attacker can just swap your number to their number. ...but mostly works. Test Plan: - Added SMS MFA to my account. - Typed in the number I was texted. - Typed in some other different numbers (didn't work). - Cancelled/resumed the workflow, used SMS in conjunction with other factors, tried old codes, etc. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D20022
This commit is contained in:
parent
6c11f37396
commit
ada8a56bb7
4 changed files with 414 additions and 6 deletions
|
@ -4297,6 +4297,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorResourceSite' => 'aphront/site/PhabricatorResourceSite.php',
|
'PhabricatorResourceSite' => 'aphront/site/PhabricatorResourceSite.php',
|
||||||
'PhabricatorRobotsController' => 'applications/system/controller/PhabricatorRobotsController.php',
|
'PhabricatorRobotsController' => 'applications/system/controller/PhabricatorRobotsController.php',
|
||||||
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
|
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
|
||||||
|
'PhabricatorSMSAuthFactor' => 'applications/auth/factor/PhabricatorSMSAuthFactor.php',
|
||||||
'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php',
|
'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php',
|
||||||
'PhabricatorSSHKeyGenerator' => 'infrastructure/util/PhabricatorSSHKeyGenerator.php',
|
'PhabricatorSSHKeyGenerator' => 'infrastructure/util/PhabricatorSSHKeyGenerator.php',
|
||||||
'PhabricatorSSHKeysSettingsPanel' => 'applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php',
|
'PhabricatorSSHKeysSettingsPanel' => 'applications/settings/panel/PhabricatorSSHKeysSettingsPanel.php',
|
||||||
|
@ -10393,6 +10394,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorResourceSite' => 'PhabricatorSite',
|
'PhabricatorResourceSite' => 'PhabricatorSite',
|
||||||
'PhabricatorRobotsController' => 'PhabricatorController',
|
'PhabricatorRobotsController' => 'PhabricatorController',
|
||||||
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
|
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
|
||||||
|
'PhabricatorSMSAuthFactor' => 'PhabricatorAuthFactor',
|
||||||
'PhabricatorSQLPatchList' => 'Phobject',
|
'PhabricatorSQLPatchList' => 'Phobject',
|
||||||
'PhabricatorSSHKeyGenerator' => 'Phobject',
|
'PhabricatorSSHKeyGenerator' => 'Phobject',
|
||||||
'PhabricatorSSHKeysSettingsPanel' => 'PhabricatorSettingsPanel',
|
'PhabricatorSSHKeysSettingsPanel' => 'PhabricatorSettingsPanel',
|
||||||
|
|
|
@ -33,7 +33,8 @@ abstract class PhabricatorAuthFactor extends Phobject {
|
||||||
|
|
||||||
protected function newConfigForUser(PhabricatorUser $user) {
|
protected function newConfigForUser(PhabricatorUser $user) {
|
||||||
return id(new PhabricatorAuthFactorConfig())
|
return id(new PhabricatorAuthFactorConfig())
|
||||||
->setUserPHID($user->getPHID());
|
->setUserPHID($user->getPHID())
|
||||||
|
->setFactorSecret('');
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function newResult() {
|
protected function newResult() {
|
||||||
|
@ -107,6 +108,10 @@ abstract class PhabricatorAuthFactor extends Phobject {
|
||||||
|
|
||||||
$now = PhabricatorTime::getNow();
|
$now = PhabricatorTime::getNow();
|
||||||
|
|
||||||
|
// Factor implementations may need to perform writes in order to issue
|
||||||
|
// challenges, particularly push factors like SMS.
|
||||||
|
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||||
|
|
||||||
$new_challenges = $this->newIssuedChallenges(
|
$new_challenges = $this->newIssuedChallenges(
|
||||||
$config,
|
$config,
|
||||||
$viewer,
|
$viewer,
|
||||||
|
@ -131,10 +136,10 @@ abstract class PhabricatorAuthFactor extends Phobject {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
foreach ($new_challenges as $challenge) {
|
||||||
foreach ($new_challenges as $challenge) {
|
$challenge->save();
|
||||||
$challenge->save();
|
}
|
||||||
}
|
|
||||||
unset($unguarded);
|
unset($unguarded);
|
||||||
|
|
||||||
return $new_challenges;
|
return $new_challenges;
|
||||||
|
@ -351,4 +356,36 @@ abstract class PhabricatorAuthFactor extends Phobject {
|
||||||
return phutil_units('1 hour in seconds');
|
return phutil_units('1 hour in seconds');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final protected function getChallengeForCurrentContext(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
array $challenges) {
|
||||||
|
|
||||||
|
$session_phid = $viewer->getSession()->getPHID();
|
||||||
|
$engine = $config->getSessionEngine();
|
||||||
|
$workflow_key = $engine->getWorkflowKey();
|
||||||
|
|
||||||
|
foreach ($challenges as $challenge) {
|
||||||
|
if ($challenge->getSessionPHID() !== $session_phid) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($challenge->getWorkflowKey() !== $workflow_key) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($challenge->getIsCompleted()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($challenge->getIsReusedChallenge()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $challenge;
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
367
src/applications/auth/factor/PhabricatorSMSAuthFactor.php
Normal file
367
src/applications/auth/factor/PhabricatorSMSAuthFactor.php
Normal file
|
@ -0,0 +1,367 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class PhabricatorSMSAuthFactor
|
||||||
|
extends PhabricatorAuthFactor {
|
||||||
|
|
||||||
|
public function getFactorKey() {
|
||||||
|
return 'sms';
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getFactorName() {
|
||||||
|
return pht('SMS');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getFactorCreateHelp() {
|
||||||
|
return pht(
|
||||||
|
'Allow users to receive a code via SMS.');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getFactorDescription() {
|
||||||
|
return pht(
|
||||||
|
'When you need to authenticate, a text message with a code will '.
|
||||||
|
'be sent to your phone.');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getFactorOrder() {
|
||||||
|
// Sort this factor toward the end of the list because SMS is relatively
|
||||||
|
// weak.
|
||||||
|
return 2000;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function canCreateNewProvider() {
|
||||||
|
return $this->isSMSMailerConfigured();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getProviderCreateDescription() {
|
||||||
|
$messages = array();
|
||||||
|
|
||||||
|
if (!$this->isSMSMailerConfigured()) {
|
||||||
|
$messages[] = id(new PHUIInfoView())
|
||||||
|
->setErrors(
|
||||||
|
array(
|
||||||
|
pht(
|
||||||
|
'You have not configured an outbound SMS mailer. You must '.
|
||||||
|
'configure one before you can set up SMS. See: %s',
|
||||||
|
phutil_tag(
|
||||||
|
'a',
|
||||||
|
array(
|
||||||
|
'href' => '/config/edit/cluster.mailers/',
|
||||||
|
),
|
||||||
|
'cluster.mailers')),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
$messages[] = id(new PHUIInfoView())
|
||||||
|
->setSeverity(PHUIInfoView::SEVERITY_WARNING)
|
||||||
|
->setErrors(
|
||||||
|
array(
|
||||||
|
pht(
|
||||||
|
'SMS is weak, and relatively easy for attackers to compromise. '.
|
||||||
|
'Strongly consider using a different MFA provider.'),
|
||||||
|
));
|
||||||
|
|
||||||
|
return $messages;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function canCreateNewConfiguration(PhabricatorUser $user) {
|
||||||
|
if (!$this->loadUserContactNumber($user)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getConfigurationCreateDescription(PhabricatorUser $user) {
|
||||||
|
|
||||||
|
$messages = array();
|
||||||
|
|
||||||
|
if (!$this->loadUserContactNumber($user)) {
|
||||||
|
$messages[] = id(new PHUIInfoView())
|
||||||
|
->setSeverity(PHUIInfoView::SEVERITY_WARNING)
|
||||||
|
->setErrors(
|
||||||
|
array(
|
||||||
|
pht(
|
||||||
|
'You have not configured a primary contact number. Configure '.
|
||||||
|
'a contact number before adding SMS as an authentication '.
|
||||||
|
'factor.'),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
return $messages;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getEnrollDescription(
|
||||||
|
PhabricatorAuthFactorProvider $provider,
|
||||||
|
PhabricatorUser $user) {
|
||||||
|
return pht(
|
||||||
|
'To verify your phone as an authentication factor, a text message with '.
|
||||||
|
'a secret code will be sent to the phone number you have listed as '.
|
||||||
|
'your primary contact number.');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getEnrollButtonText(
|
||||||
|
PhabricatorAuthFactorProvider $provider,
|
||||||
|
PhabricatorUser $user) {
|
||||||
|
$contact_number = $this->loadUserContactNumber($user);
|
||||||
|
|
||||||
|
return pht('Send SMS: %s', $contact_number->getDisplayName());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function processAddFactorForm(
|
||||||
|
PhabricatorAuthFactorProvider $provider,
|
||||||
|
AphrontFormView $form,
|
||||||
|
AphrontRequest $request,
|
||||||
|
PhabricatorUser $user) {
|
||||||
|
|
||||||
|
$token = $this->loadMFASyncToken($request, $form, $user);
|
||||||
|
$code = $request->getStr('sms.code');
|
||||||
|
|
||||||
|
$e_code = true;
|
||||||
|
if (!$token->getIsNewTemporaryToken()) {
|
||||||
|
$expect_code = $token->getTemporaryTokenProperty('code');
|
||||||
|
|
||||||
|
$okay = phutil_hashes_are_identical(
|
||||||
|
$this->normalizeSMSCode($code),
|
||||||
|
$this->normalizeSMSCode($expect_code));
|
||||||
|
|
||||||
|
if ($okay) {
|
||||||
|
$config = $this->newConfigForUser($user)
|
||||||
|
->setFactorName(pht('SMS'));
|
||||||
|
|
||||||
|
return $config;
|
||||||
|
} else {
|
||||||
|
if (!strlen($code)) {
|
||||||
|
$e_code = pht('Required');
|
||||||
|
} else {
|
||||||
|
$e_code = pht('Invalid');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$form->appendRemarkupInstructions(
|
||||||
|
pht(
|
||||||
|
'Enter the code from the text message which was sent to your '.
|
||||||
|
'primary contact number.'));
|
||||||
|
|
||||||
|
$form->appendChild(
|
||||||
|
id(new PHUIFormNumberControl())
|
||||||
|
->setLabel(pht('SMS Code'))
|
||||||
|
->setName('sms.code')
|
||||||
|
->setValue($code)
|
||||||
|
->setError($e_code));
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newIssuedChallenges(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
array $challenges) {
|
||||||
|
|
||||||
|
// If we already issued a valid challenge for this workflow and session,
|
||||||
|
// don't issue a new one.
|
||||||
|
|
||||||
|
$challenge = $this->getChallengeForCurrentContext(
|
||||||
|
$config,
|
||||||
|
$viewer,
|
||||||
|
$challenges);
|
||||||
|
if ($challenge) {
|
||||||
|
return array();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Otherwise, issue a new challenge.
|
||||||
|
|
||||||
|
$challenge_code = $this->newSMSChallengeCode();
|
||||||
|
$envelope = new PhutilOpaqueEnvelope($challenge_code);
|
||||||
|
$this->sendSMSCodeToUser($envelope, $viewer);
|
||||||
|
|
||||||
|
$ttl_seconds = phutil_units('15 minutes in seconds');
|
||||||
|
|
||||||
|
return array(
|
||||||
|
$this->newChallenge($config, $viewer)
|
||||||
|
->setChallengeKey($challenge_code)
|
||||||
|
->setChallengeTTL(PhabricatorTime::getNow() + $ttl_seconds),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newResultFromIssuedChallenges(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
array $challenges) {
|
||||||
|
|
||||||
|
$challenge = $this->getChallengeForCurrentContext(
|
||||||
|
$config,
|
||||||
|
$viewer,
|
||||||
|
$challenges);
|
||||||
|
|
||||||
|
if ($challenge->getIsAnsweredChallenge()) {
|
||||||
|
return $this->newResult()
|
||||||
|
->setAnsweredChallenge($challenge);
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function renderValidateFactorForm(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
AphrontFormView $form,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
PhabricatorAuthFactorResult $result) {
|
||||||
|
|
||||||
|
$control = $this->newAutomaticControl($result);
|
||||||
|
if (!$control) {
|
||||||
|
$value = $result->getValue();
|
||||||
|
$error = $result->getErrorMessage();
|
||||||
|
$name = $this->getChallengeResponseParameterName($config);
|
||||||
|
|
||||||
|
$control = id(new PHUIFormNumberControl())
|
||||||
|
->setName($name)
|
||||||
|
->setDisableAutocomplete(true)
|
||||||
|
->setValue($value)
|
||||||
|
->setError($error);
|
||||||
|
}
|
||||||
|
|
||||||
|
$control
|
||||||
|
->setLabel(pht('SMS Code'))
|
||||||
|
->setCaption(pht('Factor Name: %s', $config->getFactorName()));
|
||||||
|
|
||||||
|
$form->appendChild($control);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getRequestHasChallengeResponse(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
AphrontRequest $request) {
|
||||||
|
$value = $this->getChallengeResponseFromRequest($config, $request);
|
||||||
|
return (bool)strlen($value);
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newResultFromChallengeResponse(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
PhabricatorUser $viewer,
|
||||||
|
AphrontRequest $request,
|
||||||
|
array $challenges) {
|
||||||
|
|
||||||
|
$challenge = $this->getChallengeForCurrentContext(
|
||||||
|
$config,
|
||||||
|
$viewer,
|
||||||
|
$challenges);
|
||||||
|
|
||||||
|
$code = $this->getChallengeResponseFromRequest(
|
||||||
|
$config,
|
||||||
|
$request);
|
||||||
|
|
||||||
|
$result = $this->newResult()
|
||||||
|
->setValue($code);
|
||||||
|
|
||||||
|
if ($challenge->getIsAnsweredChallenge()) {
|
||||||
|
return $result->setAnsweredChallenge($challenge);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (phutil_hashes_are_identical($code, $challenge->getChallengeKey())) {
|
||||||
|
$ttl = PhabricatorTime::getNow() + phutil_units('15 minutes in seconds');
|
||||||
|
|
||||||
|
$challenge
|
||||||
|
->markChallengeAsAnswered($ttl);
|
||||||
|
|
||||||
|
return $result->setAnsweredChallenge($challenge);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (strlen($code)) {
|
||||||
|
$error_message = pht('Invalid');
|
||||||
|
} else {
|
||||||
|
$error_message = pht('Required');
|
||||||
|
}
|
||||||
|
|
||||||
|
$result->setErrorMessage($error_message);
|
||||||
|
|
||||||
|
return $result;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function newSMSChallengeCode() {
|
||||||
|
$value = Filesystem::readRandomInteger(0, 99999999);
|
||||||
|
$value = sprintf('%08d', $value);
|
||||||
|
return $value;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function isSMSMailerConfigured() {
|
||||||
|
$mailers = PhabricatorMetaMTAMail::newMailers(
|
||||||
|
array(
|
||||||
|
'outbound' => true,
|
||||||
|
'media' => array(
|
||||||
|
PhabricatorMailSMSMessage::MESSAGETYPE,
|
||||||
|
),
|
||||||
|
));
|
||||||
|
|
||||||
|
return (bool)$mailers;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function loadUserContactNumber(PhabricatorUser $user) {
|
||||||
|
$contact_numbers = id(new PhabricatorAuthContactNumberQuery())
|
||||||
|
->setViewer($user)
|
||||||
|
->withObjectPHIDs(array($user->getPHID()))
|
||||||
|
->withStatuses(
|
||||||
|
array(
|
||||||
|
PhabricatorAuthContactNumber::STATUS_ACTIVE,
|
||||||
|
))
|
||||||
|
->withIsPrimary(true)
|
||||||
|
->execute();
|
||||||
|
|
||||||
|
if (count($contact_numbers) !== 1) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return head($contact_numbers);
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function newMFASyncTokenProperties(PhabricatorUser $user) {
|
||||||
|
$sms_code = $this->newSMSChallengeCode();
|
||||||
|
|
||||||
|
$envelope = new PhutilOpaqueEnvelope($sms_code);
|
||||||
|
$this->sendSMSCodeToUser($envelope, $user);
|
||||||
|
|
||||||
|
return array(
|
||||||
|
'code' => $sms_code,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function sendSMSCodeToUser(
|
||||||
|
PhutilOpaqueEnvelope $envelope,
|
||||||
|
PhabricatorUser $user) {
|
||||||
|
|
||||||
|
$uri = PhabricatorEnv::getURI('/');
|
||||||
|
$uri = new PhutilURI($uri);
|
||||||
|
|
||||||
|
return id(new PhabricatorMetaMTAMail())
|
||||||
|
->setMessageType(PhabricatorMailSMSMessage::MESSAGETYPE)
|
||||||
|
->addTos(array($user->getPHID()))
|
||||||
|
->setForceDelivery(true)
|
||||||
|
->setSensitiveContent(true)
|
||||||
|
->setBody(
|
||||||
|
pht(
|
||||||
|
'Phabricator (%s) MFA Code: %s',
|
||||||
|
$uri->getDomain(),
|
||||||
|
$envelope->openEnvelope()))
|
||||||
|
->save();
|
||||||
|
}
|
||||||
|
|
||||||
|
private function normalizeSMSCode($code) {
|
||||||
|
return trim($code);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function getChallengeResponseParameterName(
|
||||||
|
PhabricatorAuthFactorConfig $config) {
|
||||||
|
return $this->getParameterName($config, 'sms.code');
|
||||||
|
}
|
||||||
|
|
||||||
|
private function getChallengeResponseFromRequest(
|
||||||
|
PhabricatorAuthFactorConfig $config,
|
||||||
|
AphrontRequest $request) {
|
||||||
|
|
||||||
|
$name = $this->getChallengeResponseParameterName($config);
|
||||||
|
|
||||||
|
$value = $request->getStr($name);
|
||||||
|
$value = (string)$value;
|
||||||
|
$value = trim($value);
|
||||||
|
|
||||||
|
return $value;
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -19,7 +19,9 @@ final class PhabricatorAuthMessagePHIDType extends PhabricatorPHIDType {
|
||||||
protected function buildQueryForObjects(
|
protected function buildQueryForObjects(
|
||||||
PhabricatorObjectQuery $query,
|
PhabricatorObjectQuery $query,
|
||||||
array $phids) {
|
array $phids) {
|
||||||
return new PhabricatorAuthMessageQuery();
|
|
||||||
|
return id(new PhabricatorAuthMessageQuery())
|
||||||
|
->withPHIDs($phids);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function loadHandles(
|
public function loadHandles(
|
||||||
|
|
Loading…
Reference in a new issue