-
Notifications
You must be signed in to change notification settings - Fork 849
Description
The S3 plugin currently only strips leading and trailing whitespace from the entire configuration line before determining which field is currently being parsed. Once the line is stripped, a string comparison is performed to identify keys, which includes the equals symbol, and if that matches, we advance the pointer based on the key's length and set the value for the key with the value immediately following the equals symbol.
Leading and trailing whitespace around the equals symbol is not handled, for example, if someone provides secret_key = foo or access_key= bar.
Because we are already stripping the entire string, it would be desirable if the value was checked for leading whitespace before its setter is called. Leading whitespace could be stripped, or logged as a warning if found, but as written, any value set could contain leading whitespace, which may lead to unexpected and difficult to diagnose failures, in particular around the secret and access keys, and session token.
Similarly, a space after the key name but before the equals will cause all of the string matches to fail, and the else case is not implemented, and instead contains: // ToDo: warnings?. Ideally a minimal fix for this issue would implement a warning if the key in the line being processed did not match any of the string comparisons, or a more robust solution could be a mechanism to strip inner spaces on either side of the equals symbol.
Area in question: https://github.com/apache/trafficserver/blob/master/plugins/s3_auth/s3_auth.cc#L557