CSteipp has submitted this change and it was merged.
Change subject: Added an HMAC step to secret key usage
......................................................................
Added an HMAC step to secret key usage
* Also, secret keys are no longer exposed even to priviledged users.
Change-Id: Iec9f0f77426d0c868a3d8725643b64cbc96bfa11
---
M backend/MWOAuthConsumer.php
M backend/MWOAuthConsumerAcceptance.php
M backend/MWOAuthDataStore.php
M backend/MWOAuthUtils.php
M frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
M frontend/specialpages/SpecialMWOAuthManageConsumers.php
6 files changed, 31 insertions(+), 22 deletions(-)
Approvals:
CSteipp: Verified; Looks good to me, approved
diff --git a/backend/MWOAuthConsumer.php b/backend/MWOAuthConsumer.php
index 6a26d1c..399af32 100644
--- a/backend/MWOAuthConsumer.php
+++ b/backend/MWOAuthConsumer.php
@@ -222,7 +222,7 @@
if ( $prop === 'key' ) {
return $this->consumerKey;
} elseif ( $prop === 'secret' ) {
- return $this->secretKey;
+ return MWOAuthUtils::hmacDBSecret( $this->secretKey );
} else {
return $this->$prop;
}
diff --git a/backend/MWOAuthConsumerAcceptance.php
b/backend/MWOAuthConsumerAcceptance.php
index 2cd107b..d62b6b9 100644
--- a/backend/MWOAuthConsumerAcceptance.php
+++ b/backend/MWOAuthConsumerAcceptance.php
@@ -94,7 +94,9 @@
* @param integer $flags MWOAuthConsumerAcceptance::READ_* bitfield
* @return MWOAuthConsumerAcceptance|bool
*/
- public static function newFromUserConsumerWiki( DBConnRef $db, $userId,
$consumer, $wiki, $flags = 0 ) {
+ public static function newFromUserConsumerWiki(
+ DBConnRef $db, $userId, $consumer, $wiki, $flags = 0
+ ) {
$row = $db->selectRow( static::getTable(),
array_values( static::getFieldColumnMap() ),
array( 'oaac_user_id' => $userId,
diff --git a/backend/MWOAuthDataStore.php b/backend/MWOAuthDataStore.php
index e513805..84bc924 100644
--- a/backend/MWOAuthDataStore.php
+++ b/backend/MWOAuthDataStore.php
@@ -36,7 +36,7 @@
public function lookup_token( $consumer, $token_type, $token ) {
wfDebugLog( 'OAuth', __METHOD__ . ": Looking up $token_type
token '$token'" );
- if ( $token_type == 'request' ) {
+ if ( $token_type === 'request' ) {
$returnToken = $this->cache->get(
MWOAuthUtils::getCacheKey(
'token',
$consumer->key,
@@ -46,19 +46,13 @@
if ( $token === null || !( $returnToken instanceof
MWOAuthToken ) ) {
throw new MWOAuthException(
'mwoauthdatastore-request-token-not-found' );
}
- } elseif ( $token_type == 'access' ) {
- $row = $this->centralDB->selectRow(
- 'oauth_accepted_consumer',
- '*',
- array( 'oaac_access_token' => $token ),
- __METHOD__
- );
-
- if ( !$row ) {
+ } elseif ( $token_type === 'access' ) {
+ $cmra = MWOAuthConsumerAcceptance::newFromToken(
$this->centralDB, $token );
+ if ( !$cmra ) {
throw new MWOAuthException(
'mwoauthdatastore-access-token-not-found' );
}
-
- $returnToken = new MWOAuthToken(
$row->oaac_access_token, $row->oaac_access_secret );
+ $secret = MWOAuthUtils::hmacDBSecret( $cmra->get(
'accessSecret' ) );
+ $returnToken = new MWOAuthToken( $cmra->get(
'accessToken' ), $secret );
} else {
throw new MWOAuthException(
'mwoauthdatastore-invalid-token-type' );
}
@@ -105,6 +99,7 @@
* Generate a new token (attached to this consumer), save it in the
cache, and return it
*
* @param MWOAuthConsumer|OAuthConsumer $consumer
+ * @return MWOAuthToken
*/
public function new_request_token( $consumer, $callback = null ) {
$token = $this->newToken();
diff --git a/backend/MWOAuthUtils.php b/backend/MWOAuthUtils.php
index 6cda593..e6f1d2d 100644
--- a/backend/MWOAuthUtils.php
+++ b/backend/MWOAuthUtils.php
@@ -256,4 +256,21 @@
return $id;
}
+
+ /**
+ * Get the effective secret key/token to use for OAuth purposes.
+ *
+ * For example, the "secret key" and "access secret" values that are
+ * used for authenticating request should be the result of applying this
+ * function to the respective values stored in the DB. This means that
+ * a leak of DB values is not enough to impersonate consumers.
+ *
+ * @param string $secret
+ * @return string
+ */
+ public static function hmacDBSecret( $secret ) {
+ global $wgSecretKey;
+
+ return $wgSecretKey ? hash_hmac( 'sha1', $secret, $wgSecretKey
) : $secret;
+ }
}
diff --git a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
index fbd1b88..b9589c5 100644
--- a/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
+++ b/frontend/specialpages/SpecialMWOAuthConsumerRegistration.php
@@ -145,7 +145,7 @@
if ( $status instanceof Status && $status->isOk() ) {
$this->getOutput()->addWikiMsg(
'mwoauthconsumerregistration-proposed',
$status->value['result']->get(
'consumerKey' ),
- $status->value['result']->get(
'secretKey' ) );
+ MWOAuthUtils::hmacDBSecret(
$status->value['result']->get( 'secretKey' ) ) );
$this->getOutput()->returnToMain();
}
break;
@@ -242,8 +242,8 @@
$this->getOutput()->addWikiMsg(
'mwoauthconsumerregistration-updated' );
$curSecretKey = $status->value['result']->get(
'secretKey' );
if ( $oldSecretKey !== $curSecretKey ) { //
token reset?
- $this->getOutput()->addWikiMsg(
-
'mwoauthconsumerregistration-secretreset', $curSecretKey );
+ $this->getOutput()->addWikiMsg(
'mwoauthconsumerregistration-secretreset',
+ MWOAuthUtils::hmacDBSecret(
$curSecretKey ) );
}
$this->getOutput()->returnToMain();
} else {
diff --git a/frontend/specialpages/SpecialMWOAuthManageConsumers.php
b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
index 2c00d82..5c70151 100644
--- a/frontend/specialpages/SpecialMWOAuthManageConsumers.php
+++ b/frontend/specialpages/SpecialMWOAuthManageConsumers.php
@@ -294,11 +294,6 @@
'default' => $cmr->get( 'restrictions',
array( 'FormatJSON', 'encode' ) ),
'rows' => 5
),
- 'secretKey' => array(
- 'type' => 'info',
- 'label-message' =>
'mwoauth-consumer-secretkey',
- 'default' => $cmr->get( 'secretKey' )
- ),
'rsaKey' => array(
'type' => 'info',
'label-message' =>
'mwoauth-consumer-rsakey',
--
To view, visit https://gerrit.wikimedia.org/r/75259
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iec9f0f77426d0c868a3d8725643b64cbc96bfa11
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/OAuth
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: CSteipp <cste...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
