Skip to content

Commit 47d2849

Browse files
committed
fix Firefox for Android compatibility, close #62
1 parent 152de42 commit 47d2849

File tree

4 files changed

+59
-18
lines changed

4 files changed

+59
-18
lines changed

README.md

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,15 +173,31 @@ $res = array(
173173
);
174174
```
175175

176-
### Payload length and security
177-
Payload will be encrypted by the library. The maximum payload length is 4078 bytes (or ASCII characters).
176+
### Payload length, security, and performance
177+
Payloads are encrypted by the library. The maximum payload length is theoretically 4078 bytes (or ASCII characters).
178+
For [compatibility reasons](mozilla-services/autopush/issues/748) though, your payload should be less than 3052 bytes long.
178179

179-
However, when you encrypt a string of a certain length, the resulting string will always have the same length,
180+
The library pads the payload by default. This is more secure but it decreases performance for both your server and your user's device.
181+
182+
#### Why is it more secure?
183+
When you encrypt a string of a certain length, the resulting string will always have the same length,
180184
no matter how many times you encrypt the initial string. This can make attackers guess the content of the payload.
181-
In order to circumvent this, this library can add some null padding to the initial payload, so that all the input of the encryption process
185+
In order to circumvent this, this library adds some null padding to the initial payload, so that all the input of the encryption process
182186
will have the same length. This way, all the output of the encryption process will also have the same length and attackers won't be able to
183-
guess the content of your payload. The downside of this approach is that you will use more bandwidth than if you didn't pad the string.
184-
That's why the library provides the option to disable this security measure:
187+
guess the content of your payload.
188+
189+
#### Why does it decrease performance?
190+
Encrypting more bytes takes more runtime on your server, and also slows down the user's device with decryption. Moreover, sending and receiving the packet will take more time.
191+
It's also not very friendly with users who have limited data plans.
192+
193+
#### How can I disable or customize automatic padding?
194+
You can customize automatic padding in order to better fit your needs.
195+
196+
Here are some ideas of settings:
197+
* (default) `Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH` (3052 bytes) for compatibility purposes with Firefox for Android
198+
* `Encryption::MAX_PAYLOAD_LENGTH` (4078 bytes) for maximum security
199+
* `false` for maximum performance
200+
* If you know your payloads will not exceed `X` bytes, then set it to `X` for the best balance between security and performance.
185201

186202
```php
187203
<?php
@@ -190,6 +206,8 @@ use Minishlink\WebPush\WebPush;
190206

191207
$webPush = new WebPush();
192208
$webPush->setAutomaticPadding(false); // disable automatic padding
209+
$webPush->setAutomaticPadding(512); // enable automatic padding to 512 bytes (you should make sure that your payload is less than 512 bytes, or else an attacker could guess the content)
210+
$webPush->setAutomaticPadding(true); // enable automatic padding to default maximum compatibility length
193211
```
194212

195213
### Changing the browser client

src/Encryption.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,18 @@
1818
final class Encryption
1919
{
2020
const MAX_PAYLOAD_LENGTH = 4078;
21+
const MAX_COMPATIBILITY_PAYLOAD_LENGTH = 3052;
2122

2223
/**
2324
* @param string $payload
24-
* @param bool $automatic
25+
* @param bool $maxLengthToPad
2526
*
2627
* @return string padded payload (plaintext)
2728
*/
28-
public static function padPayload($payload, $automatic)
29+
public static function padPayload($payload, $maxLengthToPad)
2930
{
3031
$payloadLen = Utils::safeStrlen($payload);
31-
$padLen = $automatic ? self::MAX_PAYLOAD_LENGTH - $payloadLen : 0;
32+
$padLen = $maxLengthToPad ? $maxLengthToPad - $payloadLen : 0;
3233

3334
return pack('n*', $padLen).str_pad($payload, $padLen + $payloadLen, chr(0), STR_PAD_LEFT);
3435
}

src/WebPush.php

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class WebPush
3434
private $defaultOptions;
3535

3636
/** @var bool Automatic padding of payloads, if disabled, trade security for bandwidth */
37-
private $automaticPadding = true;
37+
private $automaticPadding = Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH;
3838

3939
/** @var bool */
4040
private $nativePayloadEncryptionSupport;
@@ -295,18 +295,34 @@ public function setBrowser($browser)
295295
* @return bool
296296
*/
297297
public function isAutomaticPadding()
298+
{
299+
return $this->automaticPadding != false;
300+
}
301+
302+
/**
303+
* @return int
304+
*/
305+
public function getAutomaticPadding()
298306
{
299307
return $this->automaticPadding;
300308
}
301309

302310
/**
303-
* @param bool $automaticPadding
311+
* @param int $automaticPadding Max padding length
304312
*
305313
* @return WebPush
306314
*/
307315
public function setAutomaticPadding($automaticPadding)
308316
{
309-
$this->automaticPadding = $automaticPadding;
317+
if ($automaticPadding > Encryption::MAX_PAYLOAD_LENGTH) {
318+
throw new \Exception('Automatic padding is too large. Max is '.Encryption::MAX_PAYLOAD_LENGTH.'. Recommended max is '.Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH.' for compatibility reasons (see README).');
319+
} else if ($automaticPadding < 0) {
320+
throw new \Exception('Padding length should be positive or zero.');
321+
} else if ($automaticPadding === true) {
322+
$this->automaticPadding = Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH;
323+
} else {
324+
$this->automaticPadding = $automaticPadding;
325+
}
310326

311327
return $this;
312328
}

tests/EncryptionTest.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,27 @@ class EncryptionTest extends PHPUnit_Framework_TestCase
1818
* @dataProvider payloadProvider
1919
*
2020
* @param string $payload
21+
* @param integer $maxLengthToPad
22+
* @param integer $expectedResLength
2123
*/
22-
public function testPadPayload($payload)
24+
public function testPadPayload($payload, $maxLengthToPad, $expectedResLength)
2325
{
24-
$res = Encryption::padPayload($payload, true);
26+
$res = Encryption::padPayload($payload, $maxLengthToPad);
2527

2628
$this->assertContains('test', $res);
27-
$this->assertEquals(4080, Utils::safeStrlen($res));
29+
$this->assertEquals($expectedResLength, Utils::safeStrlen($res));
2830
}
2931

3032
public function payloadProvider()
3133
{
3234
return array(
33-
array('testé'),
34-
array(str_repeat('test', 1019)),
35-
array(str_repeat('test', 1019).'te'),
35+
array('testé', 0, 8),
36+
array('testé', 1, 8),
37+
array('testé', 6, 8),
38+
array('testé', 7, 9),
39+
array('testé', Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH, Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH + 2),
40+
array('testé', Encryption::MAX_PAYLOAD_LENGTH, Encryption::MAX_PAYLOAD_LENGTH + 2),
41+
array(str_repeat('test', 1019).'te', Encryption::MAX_PAYLOAD_LENGTH, Encryption::MAX_PAYLOAD_LENGTH + 2),
3642
);
3743
}
3844
}

0 commit comments

Comments
 (0)