Skip to content

Commit

Permalink
fix Firefox for Android compatibility, close #62
Browse files Browse the repository at this point in the history
  • Loading branch information
Minishlink committed Nov 23, 2016
1 parent 152de42 commit 47d2849
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 18 deletions.
30 changes: 24 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,31 @@ $res = array(
);
```

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

However, when you encrypt a string of a certain length, the resulting string will always have the same length,
The library pads the payload by default. This is more secure but it decreases performance for both your server and your user's device.

#### Why is it more secure?
When you encrypt a string of a certain length, the resulting string will always have the same length,
no matter how many times you encrypt the initial string. This can make attackers guess the content of the payload.
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
In order to circumvent this, this library adds some null padding to the initial payload, so that all the input of the encryption process
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
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.
That's why the library provides the option to disable this security measure:
guess the content of your payload.

#### Why does it decrease performance?
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.
It's also not very friendly with users who have limited data plans.

#### How can I disable or customize automatic padding?
You can customize automatic padding in order to better fit your needs.

Here are some ideas of settings:
* (default) `Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH` (3052 bytes) for compatibility purposes with Firefox for Android
* `Encryption::MAX_PAYLOAD_LENGTH` (4078 bytes) for maximum security
* `false` for maximum performance
* If you know your payloads will not exceed `X` bytes, then set it to `X` for the best balance between security and performance.

```php
<?php
Expand All @@ -190,6 +206,8 @@ use Minishlink\WebPush\WebPush;

$webPush = new WebPush();
$webPush->setAutomaticPadding(false); // disable automatic padding
$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)
$webPush->setAutomaticPadding(true); // enable automatic padding to default maximum compatibility length
```

### Changing the browser client
Expand Down
7 changes: 4 additions & 3 deletions src/Encryption.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
final class Encryption
{
const MAX_PAYLOAD_LENGTH = 4078;
const MAX_COMPATIBILITY_PAYLOAD_LENGTH = 3052;

/**
* @param string $payload
* @param bool $automatic
* @param bool $maxLengthToPad
*
* @return string padded payload (plaintext)
*/
public static function padPayload($payload, $automatic)
public static function padPayload($payload, $maxLengthToPad)
{
$payloadLen = Utils::safeStrlen($payload);
$padLen = $automatic ? self::MAX_PAYLOAD_LENGTH - $payloadLen : 0;
$padLen = $maxLengthToPad ? $maxLengthToPad - $payloadLen : 0;

return pack('n*', $padLen).str_pad($payload, $padLen + $payloadLen, chr(0), STR_PAD_LEFT);
}
Expand Down
22 changes: 19 additions & 3 deletions src/WebPush.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class WebPush
private $defaultOptions;

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

/** @var bool */
private $nativePayloadEncryptionSupport;
Expand Down Expand Up @@ -295,18 +295,34 @@ public function setBrowser($browser)
* @return bool
*/
public function isAutomaticPadding()
{
return $this->automaticPadding != false;
}

/**
* @return int
*/
public function getAutomaticPadding()
{
return $this->automaticPadding;
}

/**
* @param bool $automaticPadding
* @param int $automaticPadding Max padding length
*
* @return WebPush
*/
public function setAutomaticPadding($automaticPadding)
{
$this->automaticPadding = $automaticPadding;
if ($automaticPadding > Encryption::MAX_PAYLOAD_LENGTH) {
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).');
} else if ($automaticPadding < 0) {
throw new \Exception('Padding length should be positive or zero.');
} else if ($automaticPadding === true) {
$this->automaticPadding = Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH;
} else {
$this->automaticPadding = $automaticPadding;
}

return $this;
}
Expand Down
18 changes: 12 additions & 6 deletions tests/EncryptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,27 @@ class EncryptionTest extends PHPUnit_Framework_TestCase
* @dataProvider payloadProvider
*
* @param string $payload
* @param integer $maxLengthToPad
* @param integer $expectedResLength
*/
public function testPadPayload($payload)
public function testPadPayload($payload, $maxLengthToPad, $expectedResLength)
{
$res = Encryption::padPayload($payload, true);
$res = Encryption::padPayload($payload, $maxLengthToPad);

$this->assertContains('test', $res);
$this->assertEquals(4080, Utils::safeStrlen($res));
$this->assertEquals($expectedResLength, Utils::safeStrlen($res));
}

public function payloadProvider()
{
return array(
array('testé'),
array(str_repeat('test', 1019)),
array(str_repeat('test', 1019).'te'),
array('testé', 0, 8),
array('testé', 1, 8),
array('testé', 6, 8),
array('testé', 7, 9),
array('testé', Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH, Encryption::MAX_COMPATIBILITY_PAYLOAD_LENGTH + 2),
array('testé', Encryption::MAX_PAYLOAD_LENGTH, Encryption::MAX_PAYLOAD_LENGTH + 2),
array(str_repeat('test', 1019).'te', Encryption::MAX_PAYLOAD_LENGTH, Encryption::MAX_PAYLOAD_LENGTH + 2),
);
}
}

0 comments on commit 47d2849

Please sign in to comment.