Skip to content

fix(activesync): harden auth header, Autodiscover parsing/XML escaping, and ValidateCert #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TDannhauer merged 4 commits into from
Jul 2, 2026
Merged

fix(activesync): harden auth header, Autodiscover parsing/XML escaping, and ValidateCert #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc385d9
fix(activesync): harden HTTP Basic auth header parsing
TDannhauer Jul 2, 2026
bffd305
fix(activesync): harden Autodiscover parsing and escape XML responses
TDannhauer Jul 2, 2026
36ded68
fix(activesync): correct ValidateCert OpenSSL result handling
TDannhauer Jul 2, 2026
1e7b07c
docs(activesync): note ValidateCert validator extraction in H6 todo
TDannhauer Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/todo.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ when a migration plan is written.
- On a case-by-case basis, Horde\ActiveSync also owns default implementations as makes sense. These are marked final and reusable code is exposed as traits.
- SQL, Mongo or other backends are owned by Horde\Core
- Library owns a generic AuthBackendInterface, Core owns an implementation which ties into Horde Auth.
- Certificate validation becomes a validator interface owned by
Horde\ActiveSync; Core owns the implementation and its configuration.
Today `Request_ValidateCert` performs the OpenSSL purpose/trust checks
(and the CRL/chain TODOs) inline in the request handler, which is the
wrong layer for that concern (review note on
[horde/ActiveSync#74](https://github.com/horde/ActiveSync/pull/74),
2026-07-02).
- Interaction with orthogonal subsystems happens via PSR Events.

### Protocol and class layout
Expand Down
8 changes: 6 additions & 2 deletions lib/Horde/ActiveSync/Credentials.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,12 @@ protected function _getCredentials()
: (!empty($serverVars['REDIRECT_HTTP_AUTHORIZATION'])
? $serverVars['REDIRECT_HTTP_AUTHORIZATION']
: $serverVars['Authorization']);
$hash = base64_decode(str_replace('Basic ', '', $authorization));
if (strpos($hash, ':') !== false) {
// Strip only a leading, case-insensitive "Basic " scheme token
// (str_replace() would remove the token anywhere in the string),
// and decode strictly so malformed input yields no credentials
// rather than garbage.
$hash = base64_decode(preg_replace('/^\s*Basic\s+/i', '', $authorization), true);
if ($hash !== false && strpos($hash, ':') !== false) {
[$user, $pass] = explode(':', $hash, 2);
}
} else {
Expand Down
81 changes: 59 additions & 22 deletions lib/Horde/ActiveSync/Request/Autodiscover.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,27 @@ public function handle(?Horde_Controller_Request $request = null)
if (empty($values) && empty($username)) {
throw new Horde_Exception_AuthenticationFailure('No username provided.');
} elseif (!empty($values)) {
// Override the username; AUTODISCOVER MUST use email address.
$credentials->username = $values[2]['value'];
// Override the username; AUTODISCOVER MUST use the email address.
// Locate the EMailAddress element by tag name instead of relying on
// a fixed offset ($values[2]), which breaks (and can select the
// wrong node) if a client reorders or adds elements. The wire value
// is always an email address per the Autodiscover protocol; mapping
// it to the backend username (email, AD/LDAP, plain username, ...)
// is handled downstream by the driver's getUsernameFromEmail().
$email = null;
foreach ($values as $value) {
if (!empty($value['tag']) && $value['tag'] == 'EMAILADDRESS'
&& isset($value['value'])) {
$email = trim($value['value']);
break;
}
}
if ($email !== null && $email !== '') {
$credentials->username = $email;
} elseif (empty($username)) {
// No EMailAddress element and no username from the auth header.
throw new Horde_Exception_AuthenticationFailure('No username provided.');
}
}

if (!$this->_activeSync->authenticate($credentials)) {
Expand Down Expand Up @@ -107,6 +126,24 @@ public function handle(?Horde_Controller_Request $request = null)
*/
protected function _handle() {}

/**
* Escape a value for safe inclusion in the XML responses built below.
*
* The interpolated values (email, display name, schemas echoed back from
* the client request, backend host names, etc.) are otherwise concatenated
* straight into the response markup, which both corrupts the XML for values
* containing metacharacters and allows content injection for the
* client-supplied schema values.
*
* @param mixed $value The value to escape.
*
* @return string The XML-escaped value.
*/
protected function _xmlEscape($value)
{
return htmlspecialchars((string) $value, ENT_QUOTES | ENT_XML1, 'UTF-8');
}

/**
* Build the appropriate response string to send back to the client.
*
Expand Down Expand Up @@ -136,17 +173,17 @@ protected function _buildResponseString($properties)
return '<?xml version="1.0" encoding="utf-8"?>
<Autodiscover xmlns="http://schemas.microsoft.com/exchange/autodiscover/responseschema/2006">
<Response xmlns="http://schemas.microsoft.com/exchange/autodiscover/mobilesync/responseschema/2006">
<Culture>' . $properties['culture'] . '</Culture>
<Culture>' . $this->_xmlEscape($properties['culture']) . '</Culture>
<User>
<DisplayName>' . $properties['display_name'] . '</DisplayName>
<EMailAddress>' . $properties['email'] . '</EMailAddress>
<DisplayName>' . $this->_xmlEscape($properties['display_name']) . '</DisplayName>
<EMailAddress>' . $this->_xmlEscape($properties['email']) . '</EMailAddress>
</User>
<Action>
<Settings>
<Server>
<Type>MobileSync</Type>
<Url>' . $properties['url'] . '</Url>
<Name>' . $properties['url'] . '</Name>
<Url>' . $this->_xmlEscape($properties['url']) . '</Url>
<Name>' . $this->_xmlEscape($properties['url']) . '</Name>
</Server>
</Settings>
</Action>
Expand All @@ -159,9 +196,9 @@ protected function _buildResponseString($properties)
}

$xml = '<Autodiscover xmlns="http://schemas.microsoft.com/exchange/autodiscover/responseschema/2006">
<Response xmlns="' . $properties['response_schema'] . '">
<Response xmlns="' . $this->_xmlEscape($properties['response_schema']) . '">
<User>
<DisplayName>' . $properties['display_name'] . '</DisplayName>
<DisplayName>' . $this->_xmlEscape($properties['display_name']) . '</DisplayName>
</User>
<Account>
<AccountType>email</AccountType>
Expand All @@ -170,9 +207,9 @@ protected function _buildResponseString($properties)
if (!empty($properties['imap'])) {
$xml .= '<Protocol>
<Type>IMAP</Type>
<Server>' . $properties['imap']['host'] . '</Server>
<Port>' . $properties['imap']['port'] . '</Port>
<LoginName>' . $properties['username'] . '</LoginName>
<Server>' . $this->_xmlEscape($properties['imap']['host']) . '</Server>
<Port>' . $this->_xmlEscape($properties['imap']['port']) . '</Port>
<LoginName>' . $this->_xmlEscape($properties['username']) . '</LoginName>
<DomainRequired>off</DomainRequired>
<SPA>off</SPA>
' . $this->_getEncryptionValue('imap', $properties) . '
Expand All @@ -182,9 +219,9 @@ protected function _buildResponseString($properties)
if (!empty($properties['pop'])) {
$xml .= '<Protocol>
<Type>POP3</Type>
<Server>' . $properties['pop']['host'] . '</Server>
<Port>' . $properties['pop']['port'] . '</Port>
<LoginName>' . $properties['username'] . '</LoginName>
<Server>' . $this->_xmlEscape($properties['pop']['host']) . '</Server>
<Port>' . $this->_xmlEscape($properties['pop']['port']) . '</Port>
<LoginName>' . $this->_xmlEscape($properties['username']) . '</LoginName>
<DomainRequired>off</DomainRequired>
<SPA>off</SPA>
' . $this->_getEncryptionValue('pop', $properties) . '
Expand All @@ -194,9 +231,9 @@ protected function _buildResponseString($properties)
if (!empty($properties['smtp'])) {
$xml .= '<Protocol>
<Type>SMTP</Type>
<Server>' . $properties['smtp']['host'] . '</Server>
<Port>' . $properties['smtp']['port'] . '</Port>
<LoginName>' . $properties['username'] . '</LoginName>
<Server>' . $this->_xmlEscape($properties['smtp']['host']) . '</Server>
<Port>' . $this->_xmlEscape($properties['smtp']['port']) . '</Port>
<LoginName>' . $this->_xmlEscape($properties['username']) . '</LoginName>
<DomainRequired>off</DomainRequired>
<SPA>off</SPA>
' . $this->_getEncryptionValue('smtp', $properties) . '
Expand All @@ -218,7 +255,7 @@ protected function _buildResponseString($properties)
protected function _getEncryptionValue($type, $properties)
{
if (!empty($properties[$type]['encryption'])) {
return '<Encryption>' . $properties[$type]['encryption'] . '</Encryption>';
return '<Encryption>' . $this->_xmlEscape($properties[$type]['encryption']) . '</Encryption>';
}
// Older version of autodiscover.
if (!empty($properties[$type]['ssl'])) {
Expand All @@ -244,14 +281,14 @@ protected function _buildFailureResponse($email, $status, $response_schema)
{
return '<?xml version="1.0" encoding="utf-8"?>
<Autodiscover xmlns="http://schemas.microsoft.com/exchange/autodiscover/responseschema/2006">
<Response xmlns="' . $response_schema . '">
<Response xmlns="' . $this->_xmlEscape($response_schema) . '">
<Culture>en:us</Culture>
<User>
<EMailAddress>' . $email . '</EMailAddress>
<EMailAddress>' . $this->_xmlEscape($email) . '</EMailAddress>
</User>
<Action>
<Error>
<Status>' . $status . '</Status>
<Status>' . $this->_xmlEscape($status) . '</Status>
<Message>Unable to autoconfigure the supplied email address.</Message>
<DebugData>MailUser</DebugData>
</Error>
Expand Down
13 changes: 9 additions & 4 deletions lib/Horde/ActiveSync/Request/ValidateCert.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,21 @@ protected function _handle()

// Valid purpose/trusted?
// @TODO: CRL support, CHAIN support
// openssl_x509_checkpurpose() returns true (valid + trusted for the
// purpose), false (invalid purpose OR untrusted), or -1 on error.
// Check the -1 error case with a strict comparison first, since -1
// is loosely truthy and was previously masked by a typo ($results)
// that left this branch dead.
$result = openssl_x509_checkpurpose($cert_pem, X509_PURPOSE_SMIME_SIGN, [$this->_activeSync->certPath]);
if ($result === false) {
if ($result === -1) {
// Unspecified error.
$cert_status[$key] = self::STATUS_UNKNOWN;
} elseif ($result === false) {
// @TODO:
Comment thread
TDannhauer marked this conversation as resolved.
// checkpurpose returns false if either the purpose is invalid OR
// the certificate is untrusted, so we should validate the
// trust before we send back any errors.
$cert_status[$key] = self::STATUS_PURPOSE_INVALID;
} elseif ($results == -1) {
// Unspecified error.
$cert_status[$key] = self::STATUS_UNKNOWN;
} else {
// If checkpurpose passes, it's valid AND trusted.
$cert_status[$key] = self::STATUS_SUCCESS;
Expand Down
Loading