Skip to content
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.

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

Easier tls #512

Merged
merged 4 commits into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion js/wizard/wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ OCA = OCA || {};
detectorQueue.init();

var detectors = [];
detectors.push(new OCA.LDAP.Wizard.WizardDetectorPort());
detectors.push(new OCA.LDAP.Wizard.WizardDetectorBaseDN());
detectors.push(new OCA.LDAP.Wizard.WizardDetectorEmailAttribute());
detectors.push(new OCA.LDAP.Wizard.WizardDetectorUserDisplayNameAttribute());
Expand Down
44 changes: 0 additions & 44 deletions js/wizard/wizardDetectorPort.js

This file was deleted.

34 changes: 13 additions & 21 deletions js/wizard/wizardTabElementary.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@ OCA = OCA || {};
},
ldap_port: {
$element: $('#ldap_port'),
setMethod: 'setPort',
$relatedElements: $('.ldapDetectPort')
setMethod: 'setPort'
},
ldap_tls: {
$element: $('#ldap_tls'),
setMethod: 'setTLS'
},
ldap_dn: {
$element: $('#ldap_dn'),
Expand All @@ -65,8 +68,7 @@ OCA = OCA || {};
}
};
this.setManagedItems(items);
_.bindAll(this, 'onPortButtonClick', 'onBaseDNButtonClick', 'onBaseDNTestButtonClick');
this.managedItems.ldap_port.$relatedElements.click(this.onPortButtonClick);
_.bindAll(this, 'onBaseDNButtonClick', 'onBaseDNTestButtonClick');
this.managedItems.ldap_base.$detectButton.click(this.onBaseDNButtonClick);
this.managedItems.ldap_base.$testButton.click(this.onBaseDNTestButtonClick);
},
Expand Down Expand Up @@ -103,11 +105,6 @@ OCA = OCA || {};
*/
setHost: function(host) {
this.setElementValue(this.managedItems.ldap_host.$element, host);
if(host) {
this.enableElement(this.managedItems.ldap_port.$relatedElements);
} else {
this.disableElement(this.managedItems.ldap_port.$relatedElements);
}
},

/**
Expand All @@ -119,6 +116,13 @@ OCA = OCA || {};
this.setElementValue(this.managedItems.ldap_port.$element, port);
},

/**
* updates the TLS configuration
*/
setTLS: function(tls) {
this.setElementValue(this.managedItems.ldap_tls.$element, tls);
},

/**
* updates the user (agent) DN text field
*
Expand Down Expand Up @@ -205,7 +209,6 @@ OCA = OCA || {};
*/
onConfigSwitch: function(view, configuration) {
this.baseDNTestTriggered = false;
view.disableElement(view.managedItems.ldap_port.$relatedElements);
view.onConfigLoaded(view, configuration);
},

Expand Down Expand Up @@ -290,17 +293,6 @@ OCA = OCA || {};
}
},

/**
* request to count the users with the current filter
*
* @param {Event} event
*/
onPortButtonClick: function(event) {
event.preventDefault();
this.configModel.executeAfterSet(this.configModel.requestWizard, this.configModel, 'ldap_port');
// this.configModel.requestWizard('ldap_port');
},

/**
* request to count the users with the current filter
*
Expand Down
1 change: 1 addition & 0 deletions lib/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ private function doConnect($host, $port) {
}
// Set network timeout threshold to avoid long delays when ldap server cannot be resolved
$this->getLDAP()->setOption($this->ldapConnectionRes, LDAP_OPT_NETWORK_TIMEOUT, \intval($this->configuration->ldapNetworkTimeout));
$this->getLDAP()->setOption($this->ldapConnectionRes, LDAP_OPT_TIMEOUT, \intval($this->configuration->ldapNetworkTimeout));
if (!$this->getLDAP()->isResource($this->ldapConnectionRes)) {
$this->ldapConnectionRes = null; // to indicate it really is not set, connect() might have set it to false
throw new ServerNotAvailableException("Connect to $host:$port failed");
Expand Down
1 change: 0 additions & 1 deletion lib/Controller/WizardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ public function cast($ldap_serverconfig_chooser, $action, $cfgkey = null, $cfgva
$config = new Configuration($this->config, $prefix);

switch ($action) {
case 'guessPortAndTLS':
case 'guessBaseDN':
case 'detectEmailAttribute':
case 'detectUserDisplayNameAttribute':
Expand Down
175 changes: 2 additions & 173 deletions lib/Wizard.php
Original file line number Diff line number Diff line change
Expand Up @@ -652,60 +652,6 @@ public function testLoginName($loginName) {
return $this->result;
}

/**
* Tries to determine the port, requires given Host, User DN and Password
* @return WizardResult|false WizardResult on success, false otherwise
* @throws \Exception
*/
public function guessPortAndTLS() {
if (!$this->checkRequirements(['ldapHost',
])) {
return false;
}
$this->checkHost();
$portSettings = $this->getPortSettingsToTry();

if (!\is_array($portSettings)) {
throw new \Exception(\print_r($portSettings, true));
}

//proceed from the best configuration and return on first success
foreach ($portSettings as $setting) {
$p = $setting['port'];
$t = $setting['tls'];
\OCP\Util::writeLog('user_ldap', 'Wiz: trying port '. $p . ', TLS '. $t, \OCP\Util::DEBUG);
//connectAndBind may throw Exception, it needs to be catched by the
//callee of this method

try {
$settingsFound = $this->connectAndBind($p, $t);
} catch (\Exception $e) {
// any reply other than -1 (= cannot connect) is already okay,
// because then we found the server
// unavailable startTLS returns -11
if ($e->getCode() > 0) {
$settingsFound = true;
} else {
throw $e;
}
}

if ($settingsFound === true) {
$config = [
'ldapPort' => $p,
'ldapTLS' => (int)$t
];
$this->configuration->setConfiguration($config);
\OCP\Util::writeLog('user_ldap', 'Wiz: detected Port ' . $p, \OCP\Util::DEBUG);
$this->result->addChange('ldap_port', $p);
return $this->result;
}
}

//custom port, undetected (we do not brute force)
return false;
}

/**
* tries to determine a base dn from User DN or LDAP Host
* @return WizardResult|false WizardResult on success, false otherwise
Expand Down Expand Up @@ -1023,80 +969,6 @@ private function composeLdapFilter($filterType) {
return $filter;
}

/**
* Connects and Binds to an LDAP Server
* @param int $port the port to connect with
* @param bool $tls whether startTLS is to be used
* @param bool $ncc
* @return bool
* @throws \Exception
*/
private function connectAndBind($port = 389, $tls = false, $ncc = false) {
if ($ncc) {
//No certificate check
//FIXME: undo afterwards
\putenv('LDAPTLS_REQCERT=never');
}

//connect, does not really trigger any server communication
\OCP\Util::writeLog('user_ldap', 'Wiz: Checking Host Info ', \OCP\Util::DEBUG);
$host = $this->configuration->ldapHost;
$hostInfo = \parse_url($host);
if (!$hostInfo) {
throw new \Exception($this->l->t('Invalid Host'));
}
\OCP\Util::writeLog('user_ldap', 'Wiz: Attempting to connect ', \OCP\Util::DEBUG);
$cr = $this->getLDAP()->connect($host, $port);
if (!\is_resource($cr)) {
throw new \Exception($this->l->t('Invalid Host'));
}

\OCP\Util::writeLog('user_ldap', 'Wiz: Setting LDAP Options ', \OCP\Util::DEBUG);
//set LDAP options
$this->getLDAP()->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3);
$this->getLDAP()->setOption($cr, LDAP_OPT_REFERRALS, 0);
$this->getLDAP()->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT);

try {
if ($tls) {
$isTlsWorking = @$this->getLDAP()->startTls($cr);
if (!$isTlsWorking) {
return false;
}
}

\OCP\Util::writeLog('user_ldap', 'Wiz: Attemping to Bind ', \OCP\Util::DEBUG);
//interesting part: do the bind!
$login = $this->getLDAP()->bind($cr,
$this->configuration->ldapAgentName,
$this->configuration->ldapAgentPassword
);
$errNo = $this->getLDAP()->errno($cr);
$error = \ldap_error($cr);
$this->getLDAP()->unbind($cr);
} catch (ServerNotAvailableException $e) {
return false;
}

if ($login === true) {
$this->getLDAP()->unbind($cr);
if ($ncc) {
throw new \Exception('Certificate cannot be validated.');
}
\OCP\Util::writeLog('user_ldap', 'Wiz: Bind successful to Port '. $port . ' TLS ' . \intval($tls), \OCP\Util::DEBUG);
return true;
}

if ($errNo === -1 || ($errNo === 2 && $ncc)) {
//host, port or TLS wrong
return false;
}
if ($errNo === 2) {
return $this->connectAndBind($port, $tls, true);
}
throw new \Exception($error, $errNo);
}

/**
* checks whether a valid combination of agent and password has been
* provided (either two values or nothing for anonymous connect)
Expand Down Expand Up @@ -1304,7 +1176,8 @@ private function getConnection() {
$this->getLDAP()->setOption($cr, LDAP_OPT_PROTOCOL_VERSION, 3);
$this->getLDAP()->setOption($cr, LDAP_OPT_REFERRALS, 0);
$this->getLDAP()->setOption($cr, LDAP_OPT_NETWORK_TIMEOUT, self::LDAP_NW_TIMEOUT);
if ($this->configuration->ldapTLS === 1) {
$this->getLDAP()->setOption($cr, LDAP_OPT_TIMEOUT, self::LDAP_NW_TIMEOUT);
if ($this->configuration->ldapTLS) {
$this->getLDAP()->startTls($cr);
}

Expand All @@ -1318,48 +1191,4 @@ private function getConnection() {

return false;
}

/**
* @return array
*/
private function getDefaultLdapPortSettings() {
static $settings = [
['port' => 7636, 'tls' => false],
['port' => 636, 'tls' => false],
['port' => 7389, 'tls' => true],
['port' => 389, 'tls' => true],
['port' => 7389, 'tls' => false],
['port' => 389, 'tls' => false],
];
return $settings;
}

/**
* @return array
*/
private function getPortSettingsToTry() {
//389 ← LDAP / Unencrypted or StartTLS
//636 ← LDAPS / SSL
//7xxx ← UCS. need to be checked first, because both ports may be open
$host = $this->configuration->ldapHost;
$port = (int)$this->configuration->ldapPort;
$portSettings = [];

//In case the port is already provided, we will check this first
if ($port > 0) {
$hostInfo = \parse_url($host);
if (!(\is_array($hostInfo)
&& isset($hostInfo['scheme'])
&& \stripos($hostInfo['scheme'], 'ldaps') !== false)) {
$portSettings[] = ['port' => $port, 'tls' => true];
}
$portSettings[] =['port' => $port, 'tls' => false];
}

//default ports
$portSettings = \array_merge($portSettings,
$this->getDefaultLdapPortSettings());

return $portSettings;
}
}
9 changes: 6 additions & 3 deletions templates/part.wizard-server.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ class="ldapIconCopy icon-default-style"
<span class="hostPortCombinatorSpan">
<input type="number" id="ldap_port" name="ldap_port"
placeholder="<?php p($l->t('Port'));?>" />
<button class="ldapDetectPort" name="ldapDetectPort" type="button">
<?php p($l->t('Detect Port'));?>
</button>
</span>
</div>
</div>
</div>

<div class="tablerow">
<input type="checkbox" id="ldap_tls" name="ldap_tls" title="<?php p($l->t('Enable StartTLS support (also known as LDAP over TLS) for the connection. Note that this is different than LDAPS (LDAP over SSL) which doesn\'t need this checkbox checked. You\'ll need to import the LDAP server\'s certificate in your %s server.', $theme->getName()));?>" value="1">
<label for="ldap_tls"><?php p($l->t('Use StartTLS support'));?></label>
</div>

<div class="tablerow">
<input type="text" id="ldap_dn" name="ldap_dn"
class="tablecell"
Expand Down
1 change: 0 additions & 1 deletion templates/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
'wizard/wizardTabExpert',
'wizard/wizardDetectorQueue',
'wizard/wizardDetectorGeneric',
'wizard/wizardDetectorPort',
'wizard/wizardDetectorBaseDN',
'wizard/wizardDetectorFeatureAbstract',
'wizard/wizardDetectorUserObjectClasses',
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/WizardTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ private function prepareLdapWrapperForConnections() {
//dummy value, usually invalid
->will($this->returnValue(true));

$this->ldap->expects($this->exactly(3))
->method('setOption')
$this->ldap->method('setOption')
->will($this->returnValue(true));

$this->ldap->expects($this->once())
Expand Down