Looks good to me, please merge. Daniel
On Wed, Apr 20, 2011 at 15:41, R.I.Pienaar <[email protected]> wrote: > Add an aes_key_size config option that defaults to 256 for backward > compatability and support setting custom key sizes either in config > or when creating the instance of the SSL class > > Signed-off-by: R.I.Pienaar <[email protected]> > --- > Local-branch: feature/master/7191 > lib/mcollective/config.rb | 8 ++++- > lib/mcollective/ssl.rb | 16 +++++++-- > spec/unit/ssl_spec.rb | 47 > +++++++++++++++++++++++++++++- > website/changelog.md | 1 + > website/reference/basic/configuration.md | 1 + > 5 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/lib/mcollective/config.rb b/lib/mcollective/config.rb > index ea84166..fc8bfaf 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, :aes_key_size > > def initialize > @configured = false > @@ -89,7 +89,10 @@ module MCollective > @logger_type = val > when "fact_cache_time" > @fact_cache_time = val.to_i > - > + when "aes_key_size" > + if [256, 192, 128].include?(val.to_i) > + @aes_key_size = val.to_i > + end > else > raise("Unknown config parameter #{key}") > end > @@ -143,6 +146,7 @@ module MCollective > @loglevel = "info" > @collectives = ["mcollective"] > @main_collective = @collectives.first > + @aes_key_size = 256 > end > > def read_plugin_config_dir(dir) > diff --git a/lib/mcollective/ssl.rb b/lib/mcollective/ssl.rb > index 3f31392..587a2f8 100644 > --- a/lib/mcollective/ssl.rb > +++ b/lib/mcollective/ssl.rb > @@ -31,14 +31,22 @@ 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, :aes_key_size, > :ssl_cipher > > - def initialize(pubkey=nil, privkey=nil, passphrase=nil) > + def initialize(pubkey=nil, privkey=nil, passphrase=nil, > aes_key_size=nil) > @public_key_file = pubkey > @private_key_file = privkey > > @public_key = read_key(:public, pubkey) > @private_key = read_key(:private, privkey, passphrase) > + > + @aes_key_size = 256 > + @aes_key_size = Config.instance.aes_key_size if > Config.instance.aes_key_size > + @aes_key_size = aes_key_size if aes_key_size > + > + raise "AES Key size can only be 128, 192 or 256 not > '#{@aes_key_size}'" unless [128, 192, 256].include?(@aes_key_size) > + > + @ssl_cipher = "aes-#{@aes_key_size}-cbc" > end > > # Encrypts supplied data using AES and then encrypts using RSA > @@ -135,7 +143,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 +157,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..1a81243 100644 > --- a/spec/unit/ssl_spec.rb > +++ b/spec/unit/ssl_spec.rb > @@ -44,6 +44,39 @@ module MCollective > @ssl.decrypt_with_public(@ssl.encrypt_with_private("foo", false), > false).should == "foo" > end > > + describe "#initialize" do > + it "should default to 256 bit AES" do > + @ssl.aes_key_size.should == 256 > + @ssl.ssl_cipher.should == "aes-256-cbc" > + end > + > + it "should take the configured value when present" do > + Config.any_instance.stubs("aes_key_size").returns(128) > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem") > + > + @ssl.aes_key_size.should == 128 > + @ssl.ssl_cipher.should == "aes-128-cbc" > + end > + > + it "should set the supplied AES key size" do > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, 128) > + @ssl.aes_key_size.should == 128 > + end > + > + it "should prefer the supplied key size over configured key > size" do > + Config.any_instance.stubs("aes_key_size").returns(193) > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, 128) > + > + @ssl.aes_key_size.should == 128 > + end > + > + it "should fail on invalid key sizes" do > + expect { > + @ssl = > SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, 1) > + }.to raise_error("AES Key size can only be 128, 192 or 256 > not '1'") > + end > + end > + > describe "#read_key" do > it "should fail on non exiting files" do > expect { > @@ -94,12 +127,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 256 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 128, should work > + @ssl = SSL.new("#{@rootdir}/../fixtures/test-public.pem", > "#{@rootdir}/../fixtures/test-private.pem", nil, 128) > + @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..d9f7db5 100644 > --- a/website/changelog.md > +++ b/website/changelog.md > @@ -11,6 +11,7 @@ title: Changelog > > |Date|Description|Ticket| > |----|-----------|------| > +|2011/04/20|Make the AES key size configurable using the aes_key_size 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..26e3b3b 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| > +|aes_key_size|256|Sets the key size in use for AES encryption, should be set > the same everywhere| > > ## 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.
