Heh. Also looks good to me. :) Daniel
On Wed, Apr 20, 2011 at 16:53, R.I.Pienaar <[email protected]> wrote: > This creates a new config option - ssl_cipher - that can be used > to set the Cipher used by the SSL system and so also the AES+RSA > security plugin which now technically should also be renamed to > something else, but that's probably a bad idea. > > Valid ciphers are checked against OpenSSL::Cipher.ciphers > > Signed-off-by: R.I.Pienaar <[email protected]> > --- > Local-branch: feature/master/7191 > lib/mcollective/config.rb | 6 ++- > lib/mcollective/ssl.rb | 14 ++++++-- > spec/unit/ssl_spec.rb | 45 > +++++++++++++++++++++++++++++- > website/changelog.md | 1 + > website/reference/basic/configuration.md | 1 + > 5 files changed, 60 insertions(+), 7 deletions(-) > > diff --git a/lib/mcollective/config.rb b/lib/mcollective/config.rb > index ea84166..8574472 100644 > --- a/lib/mcollective/config.rb > +++ b/lib/mcollective/config.rb > @@ -8,7 +8,7 @@ module MCollective > :securityprovider, :factsource, :registration, > :registerinterval, :topicsep, > :classesfile, :rpcauditprovider, :rpcaudit, :configdir, > :rpcauthprovider, > :rpcauthorization, :color, :configfile, :rpchelptemplate, > :rpclimitmethod, > - :logger_type, :fact_cache_time, :collectives, > :main_collective > + :logger_type, :fact_cache_time, :collectives, > :main_collective, :ssl_cipher > > def initialize > @configured = false > @@ -89,7 +89,8 @@ module MCollective > @logger_type = val > when "fact_cache_time" > @fact_cache_time = val.to_i > - > + when "ssl_cipher" > + @ssl_cipher = val > else > raise("Unknown config parameter #{key}") > end > @@ -143,6 +144,7 @@ module MCollective > @loglevel = "info" > @collectives = ["mcollective"] > @main_collective = @collectives.first > + @ssl_cipher = "aes-256-cbc" > end > > def read_plugin_config_dir(dir) > diff --git a/lib/mcollective/ssl.rb b/lib/mcollective/ssl.rb > index 3f31392..bb22871 100644 > --- a/lib/mcollective/ssl.rb > +++ b/lib/mcollective/ssl.rb > @@ -31,14 +31,20 @@ module MCollective > # There are matching methods for using a public key to encrypt > # data to be decrypted using a private key > class SSL > - attr_reader :public_key_file, :private_key_file > + attr_reader :public_key_file, :private_key_file, :ssl_cipher > > - def initialize(pubkey=nil, privkey=nil, passphrase=nil) > + def initialize(pubkey=nil, privkey=nil, passphrase=nil, cipher=nil) > @public_key_file = pubkey > @private_key_file = privkey > > @public_key = read_key(:public, pubkey) > @private_key = read_key(:private, privkey, passphrase) > + > + @ssl_cipher = "aes-256-cbc" > + @ssl_cipher = Config.instance.ssl_cipher if > Config.instance.ssl_cipher > + @ssl_cipher = cipher if cipher > + > + raise "The supplied cipher '#{@ssl_cipher}' is not supported" > unless OpenSSL::Cipher.ciphers.include?(@ssl_cipher) > end > > # Encrypts supplied data using AES and then encrypts using RSA > @@ -135,7 +141,7 @@ module MCollective > > # encrypts a string, returns a hash of key, iv and data > def aes_encrypt(plain_string) > - cipher = OpenSSL::Cipher::Cipher.new('aes-256-cbc') > + cipher = OpenSSL::Cipher::Cipher.new(ssl_cipher) > cipher.encrypt > > key = cipher.random_key > @@ -149,7 +155,7 @@ module MCollective > > # decrypts a string given key, iv and data > def aes_decrypt(key, crypt_string) > - cipher = OpenSSL::Cipher::Cipher.new('aes-256-cbc') > + cipher = OpenSSL::Cipher::Cipher.new(ssl_cipher) > > cipher.decrypt > cipher.key = key > diff --git a/spec/unit/ssl_spec.rb b/spec/unit/ssl_spec.rb > index 8b7b38e..cc1c6e5 100644 > --- a/spec/unit/ssl_spec.rb > +++ b/spec/unit/ssl_spec.rb > @@ -44,6 +44,37 @@ module MCollective > @ssl.decrypt_with_public(@ssl.encrypt_with_private("foo", false), > false).should == "foo" > end > > + describe "#initialize" do > + it "should default to aes-256-cbc" do > + @ssl.ssl_cipher.should == "aes-256-cbc" > + end > + > + it "should take the configured value when present" do > + > Config.any_instance.stubs("ssl_cipher").returns("aes-128-cbc") > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem") > + > + @ssl.ssl_cipher.should == "aes-128-cbc" > + end > + > + it "should set the supplied ssl cipher" do > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, "aes-128-cbc") > + @ssl.ssl_cipher.should == "aes-128-cbc" > + end > + > + it "should prefer the supplied key size over configured key > size" do > + > Config.any_instance.stubs("aes_key_size").returns("foo-foo-foo") > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, "aes-128-cbc") > + > + @ssl.ssl_cipher.should == "aes-128-cbc" > + end > + > + it "should fail on invalid key sizes" do > + expect { > + @ssl = > SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, "foo-foo-foo") > + }.to raise_error("The supplied cipher 'foo-foo-foo' is not > supported") > + end > + end > + > describe "#read_key" do > it "should fail on non exiting files" do > expect { > @@ -94,12 +125,24 @@ module MCollective > end > > describe "#aes_decrypt" do > - it "should decrypted correctly given key and data" do > + it "should decrypt correctly given key and data" do > key = > @ssl.base64_decode("rAaCyW6qB0XqZNa9hji0qHwrI3P47t8diLNXoemW9ss=") > data = @ssl.base64_decode("mSthvO/wSl0ArNOcgysTVw==") > > @ssl.aes_decrypt(key, data).should == "foo" > end > + > + it "should decrypt correctly given key, data and key size" do > + key = @ssl.base64_decode("VEma3a/R7fjw2M4d0NIctA==") > + data = @ssl.base64_decode("FkH6qLvKTn7a+uNPe8ciHA==") > + > + # the default aes-256-cbc should fail here, the key above is > 128 bit > + expect { @ssl.aes_decrypt(key, data) }.to raise_error(/key > length too short: no start line/) > + > + # new ssl instance configured for aes-128-cbc, should work > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, "aes-128-cbc") > + @ssl.aes_decrypt(key, data).should == "foo" > + end > end > > describe "#decrypt_with_public" do > diff --git a/website/changelog.md b/website/changelog.md > index 365cde7..fb48042 100644 > --- a/website/changelog.md > +++ b/website/changelog.md > @@ -11,6 +11,7 @@ title: Changelog > > |Date|Description|Ticket| > |----|-----------|------| > +|2011/04/20|Make the SSL Cipher used a config option|7191| > |2011/04/20|Add a clear method to the PluginManager that deletes all > plugins, improve test isolation|7176| > |2011/04/19|Abstract the creation of request and reply hashes to simplify > connector plugin development|5701| > |2011/04/15|Improve the shellsafe validator and add a Util method to do > shell escaping|7066| > diff --git a/website/reference/basic/configuration.md > b/website/reference/basic/configuration.md > index 1598a27..37f7307 100644 > --- a/website/reference/basic/configuration.md > +++ b/website/reference/basic/configuration.md > @@ -42,6 +42,7 @@ Configuration is a simple *key = val* style configuration > file. > |securityprovider|Psk|Which security model to use, see [SSL Security > Plugin][SSLSecurity] and [AES Security Plugin][AESSecurity] for details on > others| > |rpchelptemplate|/etc/mcollective/rpc-help.erb|The path to the erb template > used for generating help| > |loggertype|file|Valid logger types, currently file, syslog or console| > +|ssl_cipher|aes-256-cbc|This sets the cipher in use by the SSL code, see > _man enc_ for a list supported by OpenSSL| > > ## Server Configuration > The server configuration file should be root only readable > -- > 1.7.1 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- ⎋ Puppet Labs Developer – http://puppetlabs.com ✉ Daniel Pittman <[email protected]> ✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775 ♲ Made with 100 percent post-consumer electrons -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
