Skip to content

Commit 52dbc0e

Browse files
fabiangfabiang
committed
Cleanup + fixed tests of PR #19 
1 parent e6d6b0c commit 52dbc0e

File tree

11 files changed

+148
-36
lines changed

11 files changed

+148
-36
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,16 @@ jobs:
6464
- name: Wait for XMPP to become available
6565
uses: iFaxity/wait-on-action@v1
6666
with:
67-
resource: tcp:localhost:5222
67+
resource: tcp:localhost:15222
68+
timeout: 1800000
69+
interval: 10000
70+
delay: 60000
71+
log: true
72+
73+
- name: Wait for second XMPP to become available
74+
uses: iFaxity/wait-on-action@v1
75+
with:
76+
resource: tcp:localhost:25222
6877
timeout: 1800000
6978
interval: 10000
7079
delay: 60000

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
build/
44
phpunit.xml
55
.phpunit.result.cache
6+
.phpunit.cache/
67
.phpcs-cache
78
*.log
89
codeclimate.json

‎behat.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ default:
55
contexts:
66
- Fabiang\Sasl\Behat\XMPPContext:
77
- localhost
8-
- 5222
8+
- 15222
9+
# Extra test server for https://dyn.eightysoft.de/xeps/xep-0474.html
10+
- 25222
911
- localhost
1012
- testuser
1113
- testpass

‎docker-compose.yml‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,23 @@ services:
66
- "./tests/config/ejabberd/ejabberd.yml:/home/ejabberd/conf/ejabberd.yml"
77
- "./tests/config/ejabberd/ejabberd.db:/home/ejabberd/database/ejabberd.db"
88
ports:
9-
- "5222:5222"
10-
- "5223:5223"
9+
- "15222:5222"
10+
- "15223:5223"
11+
environment:
12+
- CTL_ON_CREATE=register testuser localhost testpass
13+
14+
# Extra test server for https://dyn.eightysoft.de/xeps/xep-0474.html
15+
xmpp_source:
16+
build:
17+
context: https://github.com/processone/docker-ejabberd.git#master:ecs
18+
args:
19+
- VERSION=ceee3d3be1fc1053e61bbc81881bd40dbbbc1e89
20+
volumes:
21+
- "./tests/config/ejabberd/ejabberd.yml:/home/ejabberd/conf/ejabberd.yml"
22+
- "./tests/config/ejabberd/ejabberd.db:/home/ejabberd/database/ejabberd.db"
23+
ports:
24+
- "25222:5222"
25+
- "25223:5223"
1126
environment:
1227
- CTL_ON_CREATE=register testuser localhost testpass
1328

‎src/Authentication/AbstractAuthentication.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,11 @@ protected function generateCnonce()
103103
/**
104104
* Generate downgrade protection string
105105
*
106+
* @param string $groupDelimiter
107+
* @param string $delimiter
106108
* @return string
107109
*/
108-
protected function generateDowngradeProtectionVerification()
110+
protected function generateDowngradeProtectionVerification($groupDelimiter, $delimiter)
109111
{
110112
$downgradeProtectionOptions = $this->options->getDowngradeProtection();
111113

@@ -119,9 +121,9 @@ protected function generateDowngradeProtectionVerification()
119121
usort($allowedMechanisms, array($this, 'sortOctetCollation'));
120122
usort($allowedChannelBindings, array($this, 'sortOctetCollation'));
121123

122-
$protect = implode(',', $allowedMechanisms);
124+
$protect = implode($delimiter, $allowedMechanisms);
123125
if (count($allowedChannelBindings) > 0) {
124-
$protect .= '|' . implode(',', $allowedChannelBindings);
126+
$protect .= $groupDelimiter. implode($delimiter, $allowedChannelBindings);
125127
}
126128
return $protect;
127129
}

‎src/Authentication/SCRAM.php‎

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,18 @@ private function generateResponse($challenge, $password)
165165
$serverMessageRegexp = "#^r=(?<nonce>[\x21-\x2B\x2D-\x7E/]+)"
166166
. ",s=(?<salt>(?:[A-Za-z0-9/+]{4})*(?:[A-Za-z0-9/+]{3}=|[A-Za-z0-9/+]{2}==)?)"
167167
. ",i=(?<iteration>[0-9]*)"
168-
. "(,(?<additionalAttributes>.*))?$#";
168+
. "(?<additionalAttr>(?:,[A-Za-z]=[^,]+)*)$#";
169169

170170
if (!isset($this->cnonce, $this->gs2Header) || !preg_match($serverMessageRegexp, $challenge, $matches)) {
171171
return false;
172172
}
173173

174-
$additionalAttributes = $this->parseAdditionalAttributes($matches);
174+
$additionalAttributes = $this->parseAdditionalAttributes($matches['additionalAttr']);
175175

176176
//forbidden by RFC 5802
177-
if(isset($additionalAttributes['m']))
177+
if (isset($additionalAttributes['m'])) {
178178
return false;
179+
}
179180

180181
$nonce = $matches['nonce'];
181182
$salt = base64_decode($matches['salt']);
@@ -191,9 +192,14 @@ private function generateResponse($challenge, $password)
191192
return false;
192193
}
193194

194-
//SSDP hash
195-
if (!empty($additionalAttributes['d'])) {
196-
if (!$this->downgradeProtection($additionalAttributes['d'])) {
195+
if (! empty($additionalAttributes['h'])) {
196+
if (! $this->downgradeProtection($additionalAttributes['h'], "\x1f", "\x1e")) {
197+
return false;
198+
}
199+
}
200+
201+
if (! empty($additionalAttributes['d'])) {
202+
if (! $this->downgradeProtection($additionalAttributes['d'], '|', ',')) {
197203
return false;
198204
}
199205
}
@@ -215,15 +221,22 @@ private function generateResponse($challenge, $password)
215221

216222
/**
217223
* @param string $expectedDowngradeProtectionHash
224+
* @param string $groupDelimiter
225+
* @param string $delimiter
218226
* @return bool
219227
*/
220-
private function downgradeProtection($expectedDowngradeProtectionHash)
228+
private function downgradeProtection($expectedDowngradeProtectionHash, $groupDelimiter, $delimiter)
221229
{
222230
if ($this->options->getDowngradeProtection() === null) {
223231
return true;
224232
}
225233

226-
$actualDgPHash = base64_encode(call_user_func($this->hash, $this->generateDowngradeProtectionVerification()));
234+
$actualDgPHash = base64_encode(
235+
call_user_func(
236+
$this->hash,
237+
$this->generateDowngradeProtectionVerification($groupDelimiter, $delimiter)
238+
)
239+
);
227240
return $expectedDowngradeProtectionHash === $actualDgPHash;
228241
}
229242

@@ -248,19 +261,26 @@ private function hi($str, $salt, $i)
248261

249262
/**
250263
* This will parse all non-fixed-position additional SCRAM attributes (the optional ones and the m-attribute)
251-
* @param array $matches The array returned by our regex match, MUST contain an 'additionalAttributes' key
264+
*
265+
* @param string $additionalAttributes 'additionalAttributes' string
252266
* @return array
253267
*/
254-
private function parseAdditionalAttributes($matches)
268+
private function parseAdditionalAttributes($additionalAttributes)
255269
{
256-
$additionalAttributes=array();
257-
$tail=explode(',', $matches['additionalAttributes']);
258-
foreach($tail as $entry)
259-
{
260-
$entry=explode("=", $entry, 2);
261-
$additionalAttributes[$entry[0]] = $entry[1];
270+
if ($additionalAttributes == "") {
271+
return array();
262272
}
263-
return $additionalAttributes;
273+
274+
$return = array();
275+
$tail = explode(',', $additionalAttributes);
276+
277+
foreach ($tail as $entry) {
278+
$entry = explode("=", $entry, 2);
279+
if (count($entry) > 1) {
280+
$return[$entry[0]] = $entry[1];
281+
}
282+
}
283+
return $return;
264284
}
265285

266286
/**
@@ -274,21 +294,22 @@ private function parseAdditionalAttributes($matches)
274294
*/
275295
public function verify($data)
276296
{
277-
$verifierRegexp = '#^v=((?:[A-Za-z0-9/+]{4})*(?:[A-Za-z0-9/+]{3}=|[A-Za-z0-9/+]{2}==)?)(,(?<additionalAttributes>.*))?$#';
297+
$verifierRegexp = '#^v=(?<verifier>(?:[A-Za-z0-9/+]{4})*(?:[A-Za-z0-9/+]{3}=|[A-Za-z0-9/+]{2}==)?)'
298+
. '(?<additionalAttr>(?:,[A-Za-z]=[^,]+)*)$#';
278299

279300
$matches = array();
280301
if (!isset($this->saltedPassword, $this->authMessage) || !preg_match($verifierRegexp, $data, $matches)) {
281302
// This cannot be an outcome, you never sent the challenge's response.
282303
return false;
283304
}
284305

285-
$additionalAttributes = $this->parseAdditionalAttributes($matches);
306+
$additionalAttribute = $this->parseAdditionalAttributes($matches['additionalAttr']);
286307

287-
//forbidden by RFC 5802
288-
if(isset($additionalAttributes['m']))
308+
if (isset($additionalAttribute['m'])) {
289309
return false;
310+
}
290311

291-
$verifier = $matches[1];
312+
$verifier = $matches['verifier'];
292313
$proposedServerSignature = base64_decode($verifier);
293314
$serverKey = call_user_func($this->hmac, $this->saltedPassword, "Server Key", true);
294315
$serverSignature = call_user_func($this->hmac, $serverKey, $this->authMessage, true);

‎tests/features/bootstrap/AbstractContext.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@ abstract class AbstractContext
5656
protected $logdir;
5757
protected $logfile;
5858

59-
protected function connect()
59+
protected function connect($port)
6060
{
6161
$errno = null;
6262
$errstr = null;
6363

64-
$connectionString = "tcp://{$this->hostname}:{$this->port}";
64+
$connectionString = "tcp://{$this->hostname}:{$port}";
6565

6666
$context = stream_context_create(array(
6767
'ssl' => array(

‎tests/features/bootstrap/AbstractXMPPContext.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ abstract class AbstractXMPPContext extends AbstractContext implements Context, S
5555
*/
5656
protected $domain;
5757

58+
protected $port1;
59+
protected $port2;
60+
5861
/**
5962
* @var string
6063
*/
@@ -81,7 +84,8 @@ abstract class AbstractXMPPContext extends AbstractContext implements Context, S
8184
* context constructor through behat.yml.
8285
*
8386
* @param string $hostname Hostname for connection
84-
* @param integer $port
87+
* @param integer $port1
88+
* @param integer $port2
8589
* @param string $domain
8690
* @param string $username Domain name of server (important for connecting)
8791
* @param string $password
@@ -90,15 +94,17 @@ abstract class AbstractXMPPContext extends AbstractContext implements Context, S
9094
*/
9195
public function __construct(
9296
$hostname,
93-
$port,
97+
$port1,
98+
$port2,
9499
$domain,
95100
$username,
96101
$password,
97102
$logdir,
98103
$tlsversion = 'tlsv1.2'
99104
) {
100105
$this->hostname = $hostname;
101-
$this->port = (int) $port;
106+
$this->port1 = (int) $port1;
107+
$this->port2 = (int) $port2;
102108
$this->domain = $domain;
103109
$this->username = $username;
104110
$this->password = $password;

‎tests/features/bootstrap/POP3Context.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public function __construct($hostname, $port, $username, $password, $logdir)
9595
*/
9696
public function connectionToPopServer()
9797
{
98-
$this->connect();
98+
$this->connect($this->port);
9999
Assert::assertSame("+OK Dovecot ready.\r\n", $this->read());
100100
}
101101

‎tests/features/bootstrap/XMPPContext.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,16 @@ private function sendStreamStart()
112112
*/
113113
public function connectionToXmppServer()
114114
{
115-
$this->connect();
115+
$this->connect($this->port1);
116+
$this->sendStreamStart();
117+
}
118+
119+
/**
120+
* @Given Connection to second XMPP server
121+
*/
122+
public function connectionToSecondXmppServer(): void
123+
{
124+
$this->connect($this->port2);
116125
$this->sendStreamStart();
117126
}
118127

0 commit comments

Comments
 (0)