Open ozgurhangisi opened 5 years ago
this is interesting
I don't really think this is a problem of the library, as the security of your application a 100% your own concern (no offence). Neither is the way you store the data. What we have as the process for our subscriptions is the following:
in your case, I guess, you're missing the CSRF-token validation on subscription endpoint, to ensure that the request is valid and should be accepted to store data.
Hope that helps.
Hi,
Thanks for the comment. The real problem is if you are a push provider you generally add some code to your customer's website and run some script to get the token info. Even if you create some CSRF-token, hash, etc... a hacker can create exactly the same token like you do because your javascript is open and a hacker can easily follow the functions that you call and simulate what you do. They can also send a valid User-Agent string. I mean a hacker can easily send everything to your server endpoint.
Lets say you have 499 valid data and 1 invalid token data. Even in this case you can not send any webpush notification to your subscribers. Because the library doesn't fail only for invalid token it fails for all 500 webpushes and you can not send any webpushes to your subscribers.
On the third topic you mentioned above, I couldn't see any validation function in the library. If so I would be happy if you share with me.
Thanks, Ozgur.
On the third topic you mentioned above, I couldn't see any validation function in the library. If so I would be happy if you share with me.
For sure, because I didn't write it :) What you could also do, for example, is sending a test or welcome message straight on subscription. If that fails, just don't save the subscription info. Would that work for you?
:) If our customer doesn't want to show anything on registration we can not send welcome message to the users. We are not the owner of the subscribers. We are a service provider.
Actually we solve the problem in a different way. We add a few try catch block to the prepare and encrypt method and if error occurs we send empty headers to push servers and push servers gives Invalid Token as a result. So we act them like expired token. It's a quick and dirty fix :) But I think sending invalid tokens to the push servers is not the best way.
We add a few try catch block to the prepare and encrypt method and if error occurs we send empty headers to push servers and push servers gives Invalid Token as a result
Could you please post a code sample here, just so that anyone else who encounters the same issue could use it? I mean while there is no better solution.
But I think sending invalid tokens to the push servers is not the best way.
Well, feel free to submit a pull-request then, I guess :)
So actually we changed the code so heavily. But I will try to add the code blocks we change :) I hope it helps.
WebPush.php
line 303 we don't throw an exception if GCM API Key is empty. // if GCM if (substr($endpoint, 0, strlen(self::GCM_URL)) === self::GCM_URL) { if (array_key_exists('GCM', $auth)) { $headers['Authorization'] = 'key='.$auth['GCM']; } else { $headers['Authorization'] = 'key='.''; // throw new \ErrorException('No GCM API Key specified.'); } } line 317 we don't throw exception if parse_url fails.
if (!$audience_arr || !isset($audience_arr['scheme']) || !isset($audience_arr['host'])) {
// throw new \ErrorException('Audience could not be generated.');
$audience = ''.'://'.'';
} else {
$audience = $audience_arr['scheme'].'://'.$audience_arr['host'];
}
VAPID.php line 140 : return empty if VAPID Keys are wrong
try {
$jsonConverter = new StandardConverter();
$jwsCompactSerializer = new CompactSerializer($jsonConverter);
$jwsBuilder = new JWSBuilder($jsonConverter, AlgorithmManager::create([new ES256()]));
$jws = $jwsBuilder
->create()
->withPayload($jwtPayload)
->addSignature($jwk, $header)
->build();
$jwt = $jwsCompactSerializer->serialize($jws, 0);
$encodedPublicKey = Base64Url::encode($publicKey);
} catch(\Exception $e) {
return [
'Authorization' => 'vapid t=, k=',
'Crypto-Key' => 'p256ecdsa='
];
}
Encryption.php our file is completely different bu we added try catch block for creation of sharedSecret.
try {
$sharedSecret = self::getSharedSecret($localKeyObject, Base64Url::decode($userPublicKey));
} catch (\Exception $e) {
return [
'PublicKey'=>null,
'sharedSecret'=>null
];
}
This would likely be an issue with the Guzzle client not being configured with the option:
['http_errors' => false]
This can be configured by passing the above array as the fourth parameter to Minishlink\WebPush\Webpush::__construct()
.
You can read more here.
Hi @ryancco I don't think it's related with http_errors parameter because the problem occurs before http request. If endpoint or push keys are not correct library fails on encryption or prepare method just before sending WebPushes to push provider. The problem must be solved before http request also.
BTW We sent billions of web pushes and we haven't seen any problem on http request. Of course if there is a parameter to suppress http errors you can set it by default but the main problem is in encrypt and prepare methods.
@ozgurhangisi to send large amounts of pushes (more than 1 million) what value do you use for flush? do you use multi curl? how much RAM does your sever have? I'm trying to solve issues with sending bulk pushes to large amount of subs and at the moment able to send to ~500 subs in one call which takes about 12-15 seconds to complete, so sending to 1 million of subs takes at least 8 hours and that's pretty much. With FCM I was able to send to 1M in less than 1 minute (without delivery reports, but that is not what I actually care about at the moment) using 10 worker processes with multi curl, but with web push I'm constantly running in different issues: trying to allocate too much memory, too many files open, to long execution... I'm trying to find optimal values.
@ozgurhangisi to send large amounts of pushes (more than 1 million) what value do you use for flush? do you use multi curl? how much RAM does your sever have? I'm trying to solve issues with sending bulk pushes to large amount of subs and at the moment able to send to ~500 subs in one call which takes about 12-15 seconds to complete, so sending to 1 million of subs takes at least 8 hours and that's pretty much. With FCM I was able to send to 1M in less than 1 minute (without delivery reports, but that is not what I actually care about at the moment) using 10 worker processes with multi curl, but with web push I'm constantly running in different issues: trying to allocate too much memory, too many files open, to long execution... I'm trying to find optimal values.
what do you do for "With FCM I was able to send to 1M in less than 1 minute" , I have same issue with "sending to 1 million of subs takes at least 8 hours", thanks
<?php
use Minishlink\WebPush\Encryption;
foreach ($deliveries as $delivery) {
$subscription = Subscription::create([
'endpoint' => $delivery[0]->endpoint,
"keys" => [
"auth" => $delivery[0]->key_auth,
"p256dh" => $delivery[0]->key_p256dh
]
]);
$payload = json_encode([
'title' => $this->title,
'body' => $this->body,
'icon' => $this->icon_url,
'data' => [
'vibrate' => $vibrate->toArray(),
'additionalData' => [
'message_id' => $this->id,
'delivery_id' => $delivery_id,
],
'url' => $this->urlWithKey($delivery_id),
],
]);
//validate keys
try {
$contentEncoding = $subscription->getContentEncoding();
Encryption::encrypt($payload, $delivery[0]->key_p256dh, $delivery[0]->key_auth, $contentEncoding);
} catch (\Exception $e) {
continue;
}
$pushes[] = [
'subscription' => $subscription,
'payload' => $payload,
];
}
Hi,
Thanks for the comment. The real problem is if you are a push provider you generally add some code to your customer's website and run some script to get the token info. Even if you create some CSRF-token, hash, etc... a hacker can create exactly the same token like you do because your javascript is open and a hacker can easily follow the functions that you call and simulate what you do. They can also send a valid User-Agent string. I mean a hacker can easily send everything to your server endpoint.
Lets say you have 499 valid data and 1 invalid token data. Even in this case you can not send any webpush notification to your subscribers. Because the library doesn't fail only for invalid token it fails for all 500 webpushes and you can not send any webpushes to your subscribers.
On the third topic you mentioned above, I couldn't see any validation function in the library. If so I would be happy if you share with me.
Thanks, Ozgur.
I am doing the same thing as a push provider. My solution is to double confirm the subscription. Here is what I do. As I have service worker codes which get the notification data and show notifications , and I also have client javascript codes which subscribe the user. When the user subscription data posts to my backend server, I send a push notification request to test wether the subscription is valid or not. If it's not, I could simply mark it as invalid subscription. If it is valid, cos I have add a test tag to my backend server push request payload, so the service worker codes know that this is just a test notification, it can simply make a fetch request to tell me that this is a valid and alive subscriber without showing notification which my clients don't like. And I also do subscription status checking regularly, mark the unsubscribers timely.
NOTE: Please test in a least two browsers (i.e. Chrome and Firefox). This helps with diagnosing problems quicker.
Setup
When we send bulk webpushes (lets say 500 webpushes in 1 flush call) if any of the publickey or token is wrong it fails for all webpushes. When you get endpoint info from the browser some people can see the URL that you use to save the data on server side and they can add whatever they want.
Lets say a hacker added millions of fake endpoint or wrong public key or tokens to your system. So you also have many valid subscription data. It's impossible to send WebPushes even to valid subscriber in this case. You must find wrong public keys or auth keys manually and you must delete them manually from your db. It's impossible if you have millions of subscribers data.
Expected
I think WebPush library shouldn't throw any exception on prepare or encrypt method. It can silently add some info to the result and when we get result we can see the error and eliminate these fake data by adding some code like we do for expired endpoints.
Another option is you can add some methods like validate_endpoint_data and we can control the data using this method before adding it to db.
Features Used
Example / Reproduce Case
Other
Thanks :)