Ticket UUID: | 60160205fe965d617482fd65dee13031fc16a1b7 | |||
Title: | Broken support for LDAPS | |||
Type: | Bug | Version: | 1.19 | |
Submitter: | pda | Created on: | 2018-05-11 12:37:13 | |
Subsystem: | ldap | Assigned To: | aku | |
Priority: | 8 | Severity: | Critical | |
Status: | Closed | Last Modified: | 2018-09-24 18:38:58 | |
Resolution: | Accepted | Closed By: | aku | |
Closed on: | 2018-09-24 18:38:58 | |||
Description: |
Support for LDAPS is broken with ldap.tcl (and ldapx.tcl too). This ticket details a problem and suggest improvements to the ldap/ldapx modules. 1) TLS handshake does not report an appropriate message I am trying to connect to our LDAPS server: Connection error: Protocol error: Error reading SEQUENCE response for handle ::ldap::ldapsock55682b6a9da0 : error : 562(long list of bytes) This message is not useful. While tracking the problem in ldap::secure_connect, I see that tls::handshake is called with async I/O enabled (fconfigure ... -blocking no). If I move this line after the while{1} loop, I get the more meaningful message: Connection error: handshake failed: certificate verify failed Conclusion 1 : TLS handshake should run on a blocking connection. 2) ldap::secure_connect does not have appropriate parameters ldap::secure_connect accepts only the following TLS related parameters "verify_cert" and "sni_servername". It does not accept the "cafile", "certfile", not "keyfile" parameters (as with ldap::starttls). In my problem described in 1), it means that I cannot specify some TLS-related parameters. Conclusion 2 : ldap::secure_connect should accept "cafile", "certfile", and "keyfile" parameters. 3) TLS parameters are not in sync with tcltls tls::import accepts the cadir/cipher/keyfile/password Conclusion 3 : ldap::secure_connect and ldap::starttls should accept these parameters too 4) The "ldap connect" method in ldapx.tcl should accept TLS-related parameters Since the "connect" method of ldap objects (in ldapx) calls secure_connect, it should accept the same parameters as ldap::secure_connect. Conclusion 4 : add these TLS-related parameters to ldapx This ticket details some proposals as a basis for discussion, before trying to implement these points. | |||
User Comments: |
aku added on 2018-09-24 18:38:58:
Patch containing new test reviewed and applied. Branch completed, merged to trunk. Merge commit [5e2cff31b4]. Closing the ticket. aku added on 2018-07-09 21:47:07: I have placed the patch into commit [337c7e5654] on branch `ldap-60160205fe-tls`. The only thing I see missing are basic tests for the new command (`::ldap::tlsoptions`), and if possible the extended syntax of the changed commands (`starttls`, `secure_connect`). I say `basic` because we currently do not have anything better than that for the changed commands. pda added on 2018-06-20 13:11:17: 1) this problem does not occur when using ldap::connect then ldap::starttls ... but I don't understand why... aku added on 2018-05-18 17:51:02: > Looking for patches. To clarify: Please provide patches, and/or a branch of Tcllib containing the changes. aku added on 2018-05-17 20:08:01: Agreed on (1), run connection setup non-async. Agreed on (2). Agreed on (3). Agreed on (4). Depending on how the arguments are passed / options are handled this might be automatic, i.e. with secure_connect accepting new options the connect will do as well. -- I haven't looked into the code yet. Looking for patches. |