Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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: 1 addition & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Update password symbol for user creation #262
* Add support for td-client-ruby 2.x.x #267
* Add SSL certificate options (--insecure, --ssl-ca-file) for proxy environments
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is for current release, please remove. We will add the changelog during release process


== 2023-03-18 version 0.17.1

Expand Down
26 changes: 24 additions & 2 deletions README.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,32 @@ These are the available hooks:

$ TD_TOOLBELT_UPDATE_ROOT="http://toolbelt.treasuredata.com"

* Specify an alternative endpoint to use updating the JAR file (default: https://repo1.maven.org):
=== SSL Options

$ TD_TOOLBELT_JARUPDATE_ROOT="https://repo1.maven.org"
Comment on lines -147 to -149
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to remove these lines?

The CLI supports SSL certificate verification options to help with proxy environments:

* Disable SSL certificate verification:

$ td --insecure ...

* Use a custom CA certificate file:

$ td --ssl-ca-file /path/to/ca.crt ...

* Configure in ~/.td/td.conf:

[ssl]
verify = false
ca_file = /path/to/ca.crt

* Set environment variables:

$ TD_SSL_VERIFY=false td ...
$ TD_SSL_CA_FILE=/path/to/ca.crt td ...

The priority order is: CLI options > environment variables > config file > default.

Note: These options do not apply to the 'td workflow' command, which uses its own HTTP client.

= Copyright

Expand Down
7 changes: 7 additions & 0 deletions lib/td/command/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ def get_client(opts={})
opts[:retry_post_requests] = Config.retry_post_requests
end

# SSL verification options
if Config.ssl_verify == false
opts[:verify] = false
elsif Config.ssl_ca_file
opts[:verify] = Config.ssl_ca_file
end
Comment on lines +44 to +50
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we unify ssl_verify and ssl_ca_file into a single ssl_verify option similar to td-client-ruby's verify option?

After all, if ssl_verify is false, ssl_ca_file value does not apply anyway.


# apikey is mandatory
apikey = Config.apikey
raise ConfigError, "Account is not configured." unless apikey
Expand Down
12 changes: 3 additions & 9 deletions lib/td/command/import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,15 +306,9 @@ def set_sysprops_endpoint(sysprops)

# generic URI
host, port = endpoint.split(':', 2)
port = port.to_i
# Config.secure = false is the --insecure option was used
if Config.secure
port = 443 if port == 0
ssl = true
else
port = 80 if port == 0
ssl = false
end
port = port.to_i == 0 ? 443 : port.to_i
ssl = true

end

sysprops << "-Dtd.api.server.scheme=#{ssl ? 'https' : 'http'}://"
Expand Down
21 changes: 18 additions & 3 deletions lib/td/command/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def run(argv=ARGV)
endpoint = @endpoint
import_endpoint = @import_endpoint || @endpoint
insecure = nil
ssl_ca_file = nil
$verbose = false
#$debug = false
retry_post_requests = false
Expand All @@ -104,16 +105,24 @@ def run(argv=ARGV)
import_endpoint = e
}

op.on('--insecure', "Insecure access: disable SSL (enabled by default)") {|b|
op.on('--insecure', "Insecure access: disable SSL certificate verification (enabled by default)") {|b|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's a good idea to reuse --insecure here. As it just means enable/disable ssl, not certification verification. Changing semantic for this --insecure option can cause confusion to both users and maintainers. We can consider deprecate --insecure option and remove it in the near future

IMO, it's better to introduce new --ssl-verify option that handles both ssl-verify and ssl-ca-file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--insecure itself was causing a lot of confusion for our customers in the history.
Some customers try to use insecure option to disable ssl verification and it didn't work as expected as it just change the protocol.
So for me, it is natural to use --insecure for this option. but not that strong desire.
And --insecure is already meaning less as TD enforces https communication in the most case.

it's better to introduce new --ssl-verify option that handles both ssl-verify and ssl-ca-file

I think this would cause some confusion to merge options into one options as they need to understand one option has 2 meanings.

insecure = true
}

op.on('--ssl-ca-file PATH', "Path to the CA certification file for SSL") {|s|
require 'td/command/common'
unless File.exist?(s)
raise ParameterConfigurationError, "CA certification file not found: #{s}"
end
ssl_ca_file = s
}

op.on('-v', '--verbose', "verbose mode", TrueClass) {|b|
$verbose = b
}

#op.on('-d', '--debug', "debug mode", TrueClass) {|b|
# $debug = b
# $debug = b
#}

op.on('-h', '--help', "show help") {
Expand Down Expand Up @@ -155,7 +164,13 @@ def run(argv=ARGV)
Config.cl_import_endpoint = true
end
if insecure
Config.secure = false
Config.ssl_verify = false
Config.cl_ssl_verify = true
$stderr.puts "Warning: --insecure option disables SSL certificate verification, which is not recommended for production use."
end
if ssl_ca_file
Config.ssl_ca_file = ssl_ca_file
Config.cl_ssl_ca_file = true
end
if retry_post_requests
Config.retry_post_requests = true
Expand Down
65 changes: 65 additions & 0 deletions lib/td/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ class Config
@@cl_import_endpoint = false # flag to indicate whether an endpoint has been provided through the command-line option
@@secure = true
@@retry_post_requests = false
@@ssl_verify = ENV['TD_SSL_VERIFY'].nil? ? true : (ENV['TD_SSL_VERIFY'].downcase == 'false' ? false : true)
@@ssl_ca_file = ENV['TD_SSL_CA_FILE']
@@ssl_ca_file = nil if @@ssl_ca_file == ""
@@cl_ssl_verify = false # flag to indicate whether ssl verify has been provided through the command-line
@@cl_ssl_ca_file = false # flag to indicate whether ssl ca file has been provided through the command-line

def initialize
@path = nil
Expand Down Expand Up @@ -194,12 +199,72 @@ def self.workflow_endpoint
end
end

def self.ssl_verify
if @@cl_ssl_verify
return @@ssl_verify
end

begin
conf = read
if conf['ssl.verify']
return conf['ssl.verify'].downcase == 'false' ? false : true
end
rescue ConfigNotFoundError
end

return @@ssl_verify
end

def self.ssl_verify=(verify)
@@ssl_verify = verify
end

def self.cl_ssl_verify
@@cl_ssl_verify
end

def self.cl_ssl_verify=(flag)
@@cl_ssl_verify = flag
end

def self.ssl_ca_file
if @@cl_ssl_ca_file
return @@ssl_ca_file
end

begin
conf = read
if conf['ssl.ca_file']
return conf['ssl.ca_file']
end
rescue ConfigNotFoundError
end

return @@ssl_ca_file
end

def self.ssl_ca_file=(ca_file)
@@ssl_ca_file = ca_file
end

def self.cl_ssl_ca_file
@@cl_ssl_ca_file
end

def self.cl_ssl_ca_file=(flag)
@@cl_ssl_ca_file = flag
end

# renders the apikey and endpoint options as a string for the helper commands
def self.cl_options_string
require 'shellwords'

string = ""
string += "-k #{@@apikey} " if @@cl_apikey
string += "-e #{@@endpoint} " if @@cl_endpoint
string += "--import-endpoint #{@@import_endpoint} " if @@cl_import_endpoint
string += "--insecure " if @@cl_ssl_verify && !@@ssl_verify
string += "--ssl-ca-file #{Shellwords.escape(@@ssl_ca_file)} " if @@cl_ssl_ca_file
string
end

Expand Down
Loading