From: Brad Jorsch Date: Thu, 18 Aug 2016 17:37:05 +0000 (-0400) Subject: SECURITY: API: Don't log "sensitive" parameters X-Git-Tag: 1.31.0-rc.0~3578 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=4d38a489b075fbd0a4c9ec228f83295cf9b9c5fc SECURITY: API: Don't log "sensitive" parameters Stuff like passwords and CSRF tokens shouldn't be in the logs. The fact of being sensitive is intentionally separated from the need to be in the POST body because, for example, the wltoken parameter to ApiQueryWatchlist needs to be in the query string to serve its purpose but still shouldn't be logged. Bug: T125177 Change-Id: I1d61f4dcf792d77401ee2e2988b1afcb2a2ad58f --- diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29 index a2dbcd5838..94bdcf7038 100644 --- a/RELEASE-NOTES-1.29 +++ b/RELEASE-NOTES-1.29 @@ -90,6 +90,8 @@ production. to interwiki links. * (T144845) SECURITY: XSS in SearchHighlighter::highlightText() when $wgAdvancedSearchHighlighting is true. +* (T125177) SECURITY: API parameters may now be marked as "sensitive" to keep + their values out of the logs. === Action API changes in 1.29 === * Submitting sensitive authentication request parameters to action=login, @@ -150,6 +152,8 @@ production. various methods now take a module path rather than a module name. * ApiMessageTrait::getApiCode() now strips 'apierror-' and 'apiwarn-' prefixes from the message key, and maps some message keys for backwards compatibility. +* API parameters may now be marked as "sensitive" to keep their values out of + the logs. === Languages updated in 1.29 === diff --git a/includes/api/ApiAuthManagerHelper.php b/includes/api/ApiAuthManagerHelper.php index d037c365bd..8862cc7f9f 100644 --- a/includes/api/ApiAuthManagerHelper.php +++ b/includes/api/ApiAuthManagerHelper.php @@ -169,6 +169,7 @@ class ApiAuthManagerHelper { $this->module->getMain()->markParamsUsed( array_keys( $data ) ); if ( $sensitive ) { + $this->module->getMain()->markParamsSensitive( array_keys( $sensitive ) ); $this->module->requirePostedParameters( array_keys( $sensitive ), 'noprefix' ); } diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index fec4234cf0..b698ceffbc 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -188,6 +188,13 @@ abstract class ApiBase extends ContextSource { */ const PARAM_EXTRA_NAMESPACES = 18; + /* + * (boolean) Is the parameter sensitive? Note 'password'-type fields are + * always sensitive regardless of the value of this field. + * @since 1.29 + */ + const PARAM_SENSITIVE = 19; + /**@}*/ const ALL_DEFAULT_STRING = '*'; @@ -1025,6 +1032,10 @@ abstract class ApiBase extends ContextSource { } else { $type = 'NULL'; // allow everything } + + if ( $type == 'password' || !empty( $paramSettings[self::PARAM_SENSITIVE] ) ) { + $this->getMain()->markParamsSensitive( $encParamName ); + } } if ( $type == 'boolean' ) { @@ -2030,6 +2041,7 @@ abstract class ApiBase extends ContextSource { $params['token'] = [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => true, + ApiBase::PARAM_SENSITIVE => true, ApiBase::PARAM_HELP_MSG => [ 'api-help-param-token', $this->needsToken(), diff --git a/includes/api/ApiCheckToken.php b/includes/api/ApiCheckToken.php index 3cc7a8a058..480915e60c 100644 --- a/includes/api/ApiCheckToken.php +++ b/includes/api/ApiCheckToken.php @@ -73,6 +73,7 @@ class ApiCheckToken extends ApiBase { 'token' => [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => true, + ApiBase::PARAM_SENSITIVE => true, ], 'maxtokenage' => [ ApiBase::PARAM_TYPE => 'integer', diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php index e3513da80d..41bec355a7 100644 --- a/includes/api/ApiLogin.php +++ b/includes/api/ApiLogin.php @@ -250,6 +250,7 @@ class ApiLogin extends ApiBase { 'token' => [ ApiBase::PARAM_TYPE => 'string', ApiBase::PARAM_REQUIRED => false, // for BC + ApiBase::PARAM_SENSITIVE => true, ApiBase::PARAM_HELP_MSG => [ 'api-help-param-token', 'login' ], ], ]; diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index a1fac0cc3d..b5eff7056e 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -161,6 +161,7 @@ class ApiMain extends ApiBase { private $mCacheMode = 'private'; private $mCacheControl = []; private $mParamsUsed = []; + private $mParamsSensitive = []; /** @var bool|null Cached return value from self::lacksSameOriginSecurity() */ private $lacksSameOriginSecurity = null; @@ -1602,13 +1603,17 @@ class ApiMain extends ApiBase { " {$logCtx['ip']} " . "T={$logCtx['timeSpentBackend']}ms"; + $sensitive = array_flip( $this->getSensitiveParams() ); foreach ( $this->getParamsUsed() as $name ) { $value = $request->getVal( $name ); if ( $value === null ) { continue; } - if ( strlen( $value ) > 256 ) { + if ( isset( $sensitive[$name] ) ) { + $value = '[redacted]'; + $encValue = '[redacted]'; + } elseif ( strlen( $value ) > 256 ) { $value = substr( $value, 0, 256 ); $encValue = $this->encodeRequestLogValue( $value ) . '[...]'; } else { @@ -1658,6 +1663,24 @@ class ApiMain extends ApiBase { $this->mParamsUsed += array_fill_keys( (array)$params, true ); } + /** + * Get the request parameters that should be considered sensitive + * @since 1.29 + * @return array + */ + protected function getSensitiveParams() { + return array_keys( $this->mParamsSensitive ); + } + + /** + * Mark parameters as sensitive + * @since 1.29 + * @param string|string[] $params + */ + public function markParamsSensitive( $params ) { + $this->mParamsSensitive += array_fill_keys( (array)$params, true ); + } + /** * Get a request value, and register the fact that it was used, for logging. * @param string $name diff --git a/includes/api/ApiQueryWatchlist.php b/includes/api/ApiQueryWatchlist.php index fee0b78c29..f8f6e7d8a1 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -475,7 +475,8 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { ApiBase::PARAM_TYPE => 'user' ], 'token' => [ - ApiBase::PARAM_TYPE => 'string' + ApiBase::PARAM_TYPE => 'string', + ApiBase::PARAM_SENSITIVE => true, ], 'continue' => [ ApiBase::PARAM_HELP_MSG => 'api-help-param-continue', diff --git a/includes/api/ApiQueryWatchlistRaw.php b/includes/api/ApiQueryWatchlistRaw.php index 116f21928d..b0b1cde92b 100644 --- a/includes/api/ApiQueryWatchlistRaw.php +++ b/includes/api/ApiQueryWatchlistRaw.php @@ -170,7 +170,8 @@ class ApiQueryWatchlistRaw extends ApiQueryGeneratorBase { ApiBase::PARAM_TYPE => 'user' ], 'token' => [ - ApiBase::PARAM_TYPE => 'string' + ApiBase::PARAM_TYPE => 'string', + ApiBase::PARAM_SENSITIVE => true, ], 'dir' => [ ApiBase::PARAM_DFLT => 'ascending',