From 005b15f32176a6d8f0d0911ccbc0691e0e9c5c43 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 14:48:29 -0700 Subject: [PATCH 01/13] first pass at URI support --- lib/net/ldap.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index f7a98ef5..263d79a7 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -1,5 +1,6 @@ # -*- ruby encoding: utf-8 -*- require 'ostruct' +require 'uri' module Net # :nodoc: class LDAP @@ -544,18 +545,28 @@ def self.result2string(code) #:nodoc: # parameters in the object. That's why Net::LDAP.new doesn't throw # cert validation errors itself; #bind does instead. def initialize(args = {}) - @host = args[:host] || DefaultHost - @port = args[:port] || DefaultPort + @uri = URI.parse(args[:uri] || '') + + @host = args[:host] || @uri.host || DefaultHost + @port = args[:port] || @uri.port || DefaultPort @hosts = args[:hosts] @verbose = false # Make this configurable with a switch on the class. @auth = args[:auth] || DefaultAuth @base = args[:base] || DefaultTreebase @force_no_page = args[:force_no_page] || DefaultForceNoPage @encryption = normalize_encryption(args[:encryption]) # may be nil + if @uri.scheme == 'ldaps' + @encryption ||= {} + @encryption[:method] = :simple_tls + end @connect_timeout = args[:connect_timeout] if pr = @auth[:password] and pr.respond_to?(:call) @auth[:password] = pr.call + elsif @uri.user || @uri.password + @auth[:method] = :simple + @auth[:username] ||= @uri.user if @uri.user + @auth[:password] ||= @uri.password if @uri.password end @instrumentation_service = args[:instrumentation_service] From b72c4a8495fa94d153bf2f4b4a2548216cf9ccfd Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 14:57:01 -0700 Subject: [PATCH 02/13] fix doc syntax --- lib/net/ldap.rb | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 263d79a7..955eff30 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -507,13 +507,13 @@ def self.result2string(code) #:nodoc: # talking over the public internet), you need to set :tls_options # something like this... # - # Net::LDAP.new( - # # ... set host, bind dn, etc ... - # encryption: { - # method: :simple_tls, - # tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS, - # } - # ) + # Net::LDAP.new( + # # ... set host, bind dn, etc ... + # encryption: { + # method: :simple_tls, + # tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS, + # } + # ) # # The above will use the operating system-provided store of CA # certificates to validate your LDAP server's cert. @@ -528,17 +528,16 @@ def self.result2string(code) #:nodoc: # `update-ca-certificates`), then the cert should pass validation. # To ignore the OS's CA store, put your CA in a PEM-encoded file and... # - # encryption: { - # method: :simple_tls, - # tls_options: { ca_file: '/path/to/my-little-ca.pem', - # ssl_version: 'TLSv1_1' }, - # } + # encryption: { + # method: :simple_tls, + # tls_options: { ca_file: '/path/to/my-little-ca.pem', + # ssl_version: 'TLSv1_1' }, + # } # # As you might guess, the above example also fails the connection # if the client can't negotiate TLS v1.1. - # tls_options is ultimately passed to OpenSSL::SSL::SSLContext#set_params - # For more details, see - # http://ruby-doc.org/stdlib-2.0.0/libdoc/openssl/rdoc/OpenSSL/SSL/SSLContext.html + # tls_options is ultimately passed to + # +OpenSSL::SSL::SSLContext#set_params+, For more details, see http://ruby-doc.org/stdlib-2.0.0/libdoc/openssl/rdoc/OpenSSL/SSL/SSLContext.html # # Instantiating a Net::LDAP object does not result in network # traffic to the LDAP server. It simply stores the connection and binding From 4898d81462165cdb619a97e7a1224d58c01b3096 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 16:13:29 -0700 Subject: [PATCH 03/13] URI may contain a base DN --- lib/net/ldap.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 955eff30..fb355d76 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -551,7 +551,11 @@ def initialize(args = {}) @hosts = args[:hosts] @verbose = false # Make this configurable with a switch on the class. @auth = args[:auth] || DefaultAuth - @base = args[:base] || DefaultTreebase + @base = args[:base] || if @uri.path && @uri.path.length > 1 + URI.decode(@uri.path[1..-1]) + else + DefaultTreebase + end @force_no_page = args[:force_no_page] || DefaultForceNoPage @encryption = normalize_encryption(args[:encryption]) # may be nil if @uri.scheme == 'ldaps' From 5fbe3ede6db49b4b7b05c1072bcbe04a0376b18f Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 16:16:18 -0700 Subject: [PATCH 04/13] RFC 4516 doesn't allow username/pw in the URL --- lib/net/ldap.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index fb355d76..599968f7 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -566,10 +566,6 @@ def initialize(args = {}) if pr = @auth[:password] and pr.respond_to?(:call) @auth[:password] = pr.call - elsif @uri.user || @uri.password - @auth[:method] = :simple - @auth[:username] ||= @uri.user if @uri.user - @auth[:password] ||= @uri.password if @uri.password end @instrumentation_service = args[:instrumentation_service] From 48610aec35a8ef085af2202f6cc1ab8fe93d7694 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 16:55:44 -0700 Subject: [PATCH 05/13] URI support: check for supported protocols --- lib/net/ldap.rb | 4 ++++ lib/net/ldap/error.rb | 1 + 2 files changed, 5 insertions(+) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 599968f7..c3401cd0 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -546,6 +546,10 @@ def self.result2string(code) #:nodoc: def initialize(args = {}) @uri = URI.parse(args[:uri] || '') + unless ['ldaps', 'ldap'].include? @uri.scheme + raise ProtocolNotSupported, + "scheme '#{@uri.scheme}' unsupported, use 'ldap' or 'ldaps'" + end @host = args[:host] || @uri.host || DefaultHost @port = args[:port] || @uri.port || DefaultPort @hosts = args[:hosts] diff --git a/lib/net/ldap/error.rb b/lib/net/ldap/error.rb index 50442d06..10ccd42f 100644 --- a/lib/net/ldap/error.rb +++ b/lib/net/ldap/error.rb @@ -70,4 +70,5 @@ class BadAttributeError < Error; end class FilterTypeUnknownError < Error; end class FilterSyntaxInvalidError < Error; end class EntryOverflowError < Error; end + class ProtocolNotSupported < Error; end end From 389fcc3fa678fa6b1b0ca9bf714a85c9333d47b9 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 16:56:03 -0700 Subject: [PATCH 06/13] clarifying comment on use of URI.parse() --- lib/net/ldap.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index c3401cd0..10cd214d 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -544,6 +544,8 @@ def self.result2string(code) #:nodoc: # parameters in the object. That's why Net::LDAP.new doesn't throw # cert validation errors itself; #bind does instead. def initialize(args = {}) + # URI.parse('') returns a valid URI object, but with all its + # attributes set to nil. This is convenient for chainging the '||' @uri = URI.parse(args[:uri] || '') unless ['ldaps', 'ldap'].include? @uri.scheme From 5484c5caebd17b214aabb136d9528f75c021850c Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 16:56:23 -0700 Subject: [PATCH 07/13] typo --- lib/net/ldap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 10cd214d..bd315a14 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -545,7 +545,7 @@ def self.result2string(code) #:nodoc: # cert validation errors itself; #bind does instead. def initialize(args = {}) # URI.parse('') returns a valid URI object, but with all its - # attributes set to nil. This is convenient for chainging the '||' + # attributes set to nil. This is convenient for chained '||' @uri = URI.parse(args[:uri] || '') unless ['ldaps', 'ldap'].include? @uri.scheme From a72d7bf86f3fa1891a8fbe1db5de8fd2c7703d81 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 17:43:38 -0700 Subject: [PATCH 08/13] move encryption param mangling to helper function --- lib/net/ldap.rb | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index bd315a14..555ed87f 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -336,6 +336,7 @@ class Net::LDAP DefaultHost = "127.0.0.1" DefaultPort = 389 + DefaultTlsPort = 636 DefaultAuth = { :method => :anonymous } DefaultTreebase = "dc=com" DefaultForceNoPage = false @@ -564,10 +565,6 @@ def initialize(args = {}) end @force_no_page = args[:force_no_page] || DefaultForceNoPage @encryption = normalize_encryption(args[:encryption]) # may be nil - if @uri.scheme == 'ldaps' - @encryption ||= {} - @encryption[:method] = :simple_tls - end @connect_timeout = args[:connect_timeout] if pr = @auth[:password] and pr.respond_to?(:call) @@ -1347,12 +1344,31 @@ def new_connection # Normalize encryption parameter the constructor accepts, expands a few # convenience symbols into recognizable hashes def normalize_encryption(args) - return if args.nil? - return args if args.is_a? Hash + valid_args = + "encryption may be hash, nil, or one of [:simple_tls, :start_tls, true]" - case method = args.to_sym - when :simple_tls, :start_tls - { :method => method, :tls_options => {} } + if args.nil? + return nil unless @uri.scheme == 'ldaps' + { method: :simple_tls, + tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS } + elsif args.is_a? Hash + args + else + case method = args.to_s.to_sym + when :simple_tls, :start_tls + { method: method, + tls_options: {} } + when :true + scheme = if @uri.scheme == 'ldaps' || @port == DefaultTlsPort + :simple_tls + else + :start_tls + end + { method: scheme, + tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS } + else + raise ArgumentError, valid_args + end end end From 6c9ce5019fa0dc4f43764fcca22c9b9d78346f55 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 17:45:31 -0700 Subject: [PATCH 09/13] only check for bad protocols if we got a uri string --- lib/net/ldap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 555ed87f..771b346a 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -549,7 +549,7 @@ def initialize(args = {}) # attributes set to nil. This is convenient for chained '||' @uri = URI.parse(args[:uri] || '') - unless ['ldaps', 'ldap'].include? @uri.scheme + unless [nil, 'ldaps', 'ldap'].include? @uri.scheme raise ProtocolNotSupported, "scheme '#{@uri.scheme}' unsupported, use 'ldap' or 'ldaps'" end From 7c61cefc7d1f918e972f472489e69470925b2289 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 17:48:54 -0700 Subject: [PATCH 10/13] s/@uri/@url/g because RFCs 2255 and 4516 say "URL" exclusively --- lib/net/ldap.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 771b346a..d6ffc003 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -547,19 +547,19 @@ def self.result2string(code) #:nodoc: def initialize(args = {}) # URI.parse('') returns a valid URI object, but with all its # attributes set to nil. This is convenient for chained '||' - @uri = URI.parse(args[:uri] || '') + @url = URI.parse(args[:uri] || '') - unless [nil, 'ldaps', 'ldap'].include? @uri.scheme + unless [nil, 'ldaps', 'ldap'].include? @url.scheme raise ProtocolNotSupported, - "scheme '#{@uri.scheme}' unsupported, use 'ldap' or 'ldaps'" + "scheme '#{@url.scheme}' unsupported, use 'ldap' or 'ldaps'" end - @host = args[:host] || @uri.host || DefaultHost - @port = args[:port] || @uri.port || DefaultPort + @host = args[:host] || @url.host || DefaultHost + @port = args[:port] || @url.port || DefaultPort @hosts = args[:hosts] @verbose = false # Make this configurable with a switch on the class. @auth = args[:auth] || DefaultAuth - @base = args[:base] || if @uri.path && @uri.path.length > 1 - URI.decode(@uri.path[1..-1]) + @base = args[:base] || if @url.path && @url.path.length > 1 + URI.decode(@url.path[1..-1]) else DefaultTreebase end @@ -1348,7 +1348,7 @@ def normalize_encryption(args) "encryption may be hash, nil, or one of [:simple_tls, :start_tls, true]" if args.nil? - return nil unless @uri.scheme == 'ldaps' + return nil unless @url.scheme == 'ldaps' { method: :simple_tls, tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS } elsif args.is_a? Hash @@ -1359,7 +1359,7 @@ def normalize_encryption(args) { method: method, tls_options: {} } when :true - scheme = if @uri.scheme == 'ldaps' || @port == DefaultTlsPort + scheme = if @url.scheme == 'ldaps' || @port == DefaultTlsPort :simple_tls else :start_tls From a4ffa7a53a22d706eea57267b2fab050818c6e19 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 17:56:30 -0700 Subject: [PATCH 11/13] let net/ldap be too long --- .rubocop_todo.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 50c86e74..cc79de15 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -82,6 +82,8 @@ Metrics/BlockNesting: # Configuration parameters: CountComments. Metrics/ClassLength: Max: 431 + Exclude: + - 'lib/net/ldap.rb' # Offense count: 22 Metrics/CyclomaticComplexity: From d3887ad2ebe92588bf50add50cb19e78a9cb5645 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 18:45:34 -0700 Subject: [PATCH 12/13] add :default for tls_options --- lib/net/ldap.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index d6ffc003..7ee155c7 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -1352,7 +1352,11 @@ def normalize_encryption(args) { method: :simple_tls, tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS } elsif args.is_a? Hash - args + if args[:tls_options].to_sym == :default + args.merge(tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS) + else + args + end else case method = args.to_s.to_sym when :simple_tls, :start_tls From ebcf83e35633f4e82de453945758108feb6f8109 Mon Sep 17 00:00:00 2001 From: Tom Maher Date: Thu, 25 Aug 2016 19:08:22 -0700 Subject: [PATCH 13/13] typecasting is hard, how do those casting directors do it??? --- lib/net/ldap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/ldap.rb b/lib/net/ldap.rb index 7ee155c7..1a5da1e4 100644 --- a/lib/net/ldap.rb +++ b/lib/net/ldap.rb @@ -1352,7 +1352,7 @@ def normalize_encryption(args) { method: :simple_tls, tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS } elsif args.is_a? Hash - if args[:tls_options].to_sym == :default + if !args[:tls_options].nil? && args[:tls_options].to_s.to_sym == :default args.merge(tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS) else args