AdjustClockSkew logs a "time skew / signature check failed" WARN on every non-2xx outcome (incl. 404/412), before the error-type / ShouldRetry check

[filimonov]

Describe the bug

In the retry loop, AWSClient::AdjustClockSkew() is called on every non-successful HTTP outcome — before RetryStrategy::ShouldRetry() and without any error-type gating — and it emits, unconditionally at the top of the function:

If the signature check failed. This could be because of a time skew. Attempting to adjust the signer.

before it has determined whether the error is actually clock-skew / signature related. So ordinary, expected non-2xx responses that have nothing to do with signatures or clocks — 404 NoSuchKey (HeadObject on a missing key), 412 PreconditionFailed (conditional PutObject If-None-Match), etc. — each emit this alarming WARN and do a wasted Date header parse. Workloads that use conditional requests / existence checks as normal control flow produce huge volumes of this misleading log and it falsely suggests a signature/clock defect where none exists.

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

The "…time skew… adjust the signer" WARN should be emitted only when a clock skew is actually detected (i.e. inside the branch where diff exceeds the threshold and the signer is adjusted). Non-skew errors (404, 412, …) should not produce a signature/clock-skew warning and ideally should not enter the skew-detection / Date-parsing path at all.

Current Behavior

src/aws-cpp-sdk-core/source/client/AWSClient.cpp (identical in 1.11.771 and main):

// retry loop — called on any error outcome, BEFORE ShouldRetry:
bool shouldSleep = !AdjustClockSkew(outcome, signerName) && !retryWithCorrectRegion;
if (!retryWithCorrectRegion && !m_retryStrategy->ShouldRetry(outcome.GetError(), retries))
    break;

bool AWSClient::AdjustClockSkew(HttpResponseOutcome& outcome, const char* signerName) const {
    if (m_enableClockSkewAdjustment) {
        auto signer = GetSignerByName(signerName);
        AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG,
            "If the signature check failed. This could be because of a time skew. Attempting to adjust the signer."); // <-- unconditional, before any skew check
        DateTime serverTime = GetServerTimeFromError(outcome.GetError());
        ...
        if (diff >= TIME_DIFF_MAX || diff <= TIME_DIFF_MIN) { /* real adjust + retry */ return true; }
    }
    return false;
}

With clocks in sync, a 404/412 logs the WARN, parses the Date header, computes diff ≈ 0, and returns false (no adjustment, no retry). Net effect: one misleading WARN + a wasted Date parse per non-2xx outcome.

Reproduction Steps

Point an S3Client at any S3-compatible endpoint whose clock matches the client, enable logging at Warn, and HEAD a non-existent key (→404). The WARN is logged even though there is no skew.

#include <aws/core/Aws.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/model/HeadObjectRequest.h>

int main() {
    Aws::SDKOptions options;
    options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Warn; // logs to aws_sdk_*.log
    Aws::InitAPI(options);
    {
        Aws::Client::ClientConfiguration cfg;
        cfg.region = "us-east-1";
        // cfg.endpointOverride = "http://127.0.0.1:9000"; // any reachable S3 / S3-compatible endpoint
        Aws::S3::S3Client s3(cfg);
        Aws::S3::Model::HeadObjectRequest req;
        req.SetBucket("an-existing-bucket");
        req.SetKey("a-definitely-missing-key");   // -> 404 NoSuchKey
        (void) s3.HeadObject(req);                // logs "...time skew... Attempting to adjust the signer."
    }
    Aws::ShutdownAPI(options);
    return 0;
}

A conditional PutObject with If-None-Match: * returning 412 reproduces it the same way.

Possible Solution

Only log / enter the skew path when a skew is plausible or confirmed. Minimal fix — move the WARN into the branch that actually detects skew:

if (diff >= TIME_DIFF_MAX || diff <= TIME_DIFF_MIN) {
    AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG,
        "Signature check may have failed due to a clock skew of " << diff.count() << " ms; adjusting the signer.");
    ... // adjust
}

Better: gate AdjustClockSkew (or its body) on clock-skew-related conditions (e.g. RequestTimeTooSkewed / RequestExpired / signature errors) so unrelated errors (404/412/…) never parse the Date header or log.

Additional Information/Context

Found in a high-conditional-request S3 workload (content-addressed storage: HEAD-before-PUT existence checks, PUT If-None-Match dedup → 412, DELETE If-Match). Every such non-2xx emitted the WARN; at debug/trace log level this was ~184k lines / ~98 MB in a single test run and initially looked like a signature/clock defect (it is not — the requests succeed, non-retryable errors break out without a retry). The impact is misleading logs + wasted Date parsing, not extra network retries.

AWS CPP SDK version used

1.11.771 (also reproduces on main)

Compiler and Version used

gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0

Operating System and version

Ubuntu 25.04 (x86_64, kernel 6.14.0-37-generic)