Skip to content

notifySubscriber() sometimes generates invalid signature (HMAC)

Mykyta Savchuk requested to merge invalid-signature into master

The notifySubscriber() method sometimes (about 5-10% of all notifications) generates an invalid HMAC signature, so pushEndpoint responds with 400. Through investigation and debugging I was able to reproduce this bug with a similar percentage of occurrence. This happens because multiple threads are simultaneously changing the secret of the same object. Lines 62–64 of the NotificationHandler class:

 SecretAuth secretAuth = authFactory.getSecretAuth(secretType);
 secretAuth.setSecret(secret);
 pushUrl = secretAuth.getPushUrl(endpoint);

authFactory returns a singleton SecretAuth (HmacAuth) bean, then the secret is set to that bean, and then that secret is used inside the getPushUrl() method, which is not thread-safe because if multiple notifications arrive at the same time, multiple threads can set different secrets to the same HmacAuth bean at the same time, which sometimes causes one or more threads to use the wrong secret in getPushUrl(). To fix this, I changed the HmacAuth scope to SimpleThreadScope so that each thread has its own instance.

Edited by Mykyta Savchuk

Merge request reports