From 5d3b53e1073970bd68f9232e476f6c3996c49623 Mon Sep 17 00:00:00 2001 From: Dan Jensen Date: Mon, 23 Aug 2021 22:20:51 -0500 Subject: [PATCH] Replace MAX_BYTE_SIZE constant with setting The MAX_BYTE_SIZE constant did not allow for customization, which is necessary for cases where legitimate SAML responses are larger than 250,000 bytes. This replaces the constant with a setting, which has a default value of 250,000 bytes, but can be customized like any other setting. --- README.md | 22 +++++++++++++++++++++ lib/onelogin/ruby-saml/logoutresponse.rb | 4 ++-- lib/onelogin/ruby-saml/response.rb | 12 +++-------- lib/onelogin/ruby-saml/saml_message.rb | 15 +++++++++----- lib/onelogin/ruby-saml/settings.rb | 2 ++ lib/onelogin/ruby-saml/slo_logoutrequest.rb | 11 +++-------- test/logoutresponse_test.rb | 5 +++-- test/saml_message_test.rb | 6 +++--- test/settings_test.rb | 3 ++- test/xml_security_test.rb | 1 + 10 files changed, 51 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 5ae09c714..19c9532e7 100644 --- a/README.md +++ b/README.md @@ -804,6 +804,28 @@ response = OneLogin::RubySaml::Response.new(params[:SAMLResponse], :allowed_cloc Make sure to keep the value as comfortably small as possible to keep security risks to a minimum. +## Deflation Limit + +To protect against decompression bombs (a form of DoS attack), SAML messages are limited to 250,000 bytes by default. +Sometimes legitimate SAML messages will exceed this limit, +for example due to custom claims like including groups a user is a member of. +If you want to customize this limit, you need to provide a different setting. +Example: + +```ruby +def consume + response = OneLogin::RubySaml::Response.new(params[:SAMLResponse]) + response.settings = saml_settings + ... +end + +private + +def saml_settings + OneLogin::RubySaml::Settings.new(message_max_bytesize: 500000) +end +``` + ## Attribute Service To request attributes from the IdP the SP needs to provide an attribute service within it's metadata and reference the index in the assertion. diff --git a/lib/onelogin/ruby-saml/logoutresponse.rb b/lib/onelogin/ruby-saml/logoutresponse.rb index 47b496e86..b19ab927b 100644 --- a/lib/onelogin/ruby-saml/logoutresponse.rb +++ b/lib/onelogin/ruby-saml/logoutresponse.rb @@ -34,7 +34,7 @@ class Logoutresponse < SamlMessage def initialize(response, settings = nil, options = {}) @errors = [] raise ArgumentError.new("Logoutresponse cannot be nil") if response.nil? - @settings = settings + @settings = settings || default_settings if settings.nil? || settings.soft.nil? @soft = true @@ -43,7 +43,7 @@ def initialize(response, settings = nil, options = {}) end @options = options - @response = decode_raw_saml(response) + @response = decode_raw_saml(response, settings) @document = XMLSecurity::SignedDocument.new(@response) end diff --git a/lib/onelogin/ruby-saml/response.rb b/lib/onelogin/ruby-saml/response.rb index 9d1dba4d6..0183734cf 100644 --- a/lib/onelogin/ruby-saml/response.rb +++ b/lib/onelogin/ruby-saml/response.rb @@ -53,17 +53,11 @@ def initialize(response, options = {}) raise ArgumentError.new("Response cannot be nil") if response.nil? @errors = [] - @options = options - @soft = true - unless options[:settings].nil? - @settings = options[:settings] - unless @settings.soft.nil? - @soft = @settings.soft - end - end + @settings = options[:settings] || default_settings + @soft = @settings.soft || true - @response = decode_raw_saml(response) + @response = decode_raw_saml(response, settings) @document = XMLSecurity::SignedDocument.new(@response, @errors) if assertion_encrypted? diff --git a/lib/onelogin/ruby-saml/saml_message.rb b/lib/onelogin/ruby-saml/saml_message.rb index d184200a1..9a590e4f9 100644 --- a/lib/onelogin/ruby-saml/saml_message.rb +++ b/lib/onelogin/ruby-saml/saml_message.rb @@ -22,8 +22,6 @@ class SamlMessage BASE64_FORMAT = %r(\A([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?\Z) @@mutex = Mutex.new - MAX_BYTE_SIZE = 250000 - # @return [Nokogiri::XML::Schema] Gets the schema object of the SAML 2.0 Protocol schema # def self.schema @@ -88,11 +86,11 @@ def valid_saml?(document, soft = true) # @param saml [String] The deflated and encoded SAML Message # @return [String] The plain SAML Message # - def decode_raw_saml(saml) + def decode_raw_saml(saml, settings) return saml unless base64_encoded?(saml) - if saml.bytesize > MAX_BYTE_SIZE - raise ValidationError.new("Encoded SAML Message exceeds " + MAX_BYTE_SIZE.to_s + " bytes, so was rejected") + if saml.bytesize > settings.message_max_bytesize + raise ValidationError.new("Encoded SAML Message exceeds " + settings.message_max_bytesize.to_s + " bytes, so was rejected") end decoded = decode(saml) @@ -157,6 +155,13 @@ def inflate(deflated) def deflate(inflated) Zlib::Deflate.deflate(inflated, 9)[2..-5] end + + # Default settings + # @return [OneLogin::RubySaml::Settings] A settings object + # + def default_settings + OneLogin::RubySaml::Settings.new + end end end end diff --git a/lib/onelogin/ruby-saml/settings.rb b/lib/onelogin/ruby-saml/settings.rb index 616a2e165..c96ab90cc 100644 --- a/lib/onelogin/ruby-saml/settings.rb +++ b/lib/onelogin/ruby-saml/settings.rb @@ -53,6 +53,7 @@ def initialize(overrides = {}, keep_security_attributes = false) attr_accessor :compress_request attr_accessor :compress_response attr_accessor :double_quote_xml_attribute_values + attr_accessor :message_max_bytesize attr_accessor :passive attr_accessor :protocol_binding attr_accessor :attributes_index @@ -258,6 +259,7 @@ def get_sp_key :idp_cert_fingerprint_algorithm => XMLSecurity::Document::SHA1, :compress_request => true, :compress_response => true, + :message_max_bytesize => 250000, :soft => true, :double_quote_xml_attribute_values => false, :security => { diff --git a/lib/onelogin/ruby-saml/slo_logoutrequest.rb b/lib/onelogin/ruby-saml/slo_logoutrequest.rb index 22efce984..fa322a9f2 100644 --- a/lib/onelogin/ruby-saml/slo_logoutrequest.rb +++ b/lib/onelogin/ruby-saml/slo_logoutrequest.rb @@ -35,15 +35,10 @@ def initialize(request, options = {}) @errors = [] @options = options - @soft = true - unless options[:settings].nil? - @settings = options[:settings] - unless @settings.soft.nil? - @soft = @settings.soft - end - end + @settings = options[:settings] || default_settings + @soft = @settings.soft || true - @request = decode_raw_saml(request) + @request = decode_raw_saml(request, settings) @document = REXML::Document.new(@request) end diff --git a/test/logoutresponse_test.rb b/test/logoutresponse_test.rb index 67eb6a084..7d50f5336 100644 --- a/test/logoutresponse_test.rb +++ b/test/logoutresponse_test.rb @@ -7,6 +7,7 @@ class RubySamlTest < Minitest::Test describe "Logoutresponse" do + let(:default_settings) { OneLogin::RubySaml::Settings.new } let(:valid_logout_response_without_settings) { OneLogin::RubySaml::Logoutresponse.new(valid_logout_response_document) } let(:valid_logout_response) { OneLogin::RubySaml::Logoutresponse.new(valid_logout_response_document, settings) } @@ -14,8 +15,8 @@ class RubySamlTest < Minitest::Test it "raise an exception when response is initialized with nil" do assert_raises(ArgumentError) { OneLogin::RubySaml::Logoutresponse.new(nil) } end - it "default to empty settings" do - assert_nil valid_logout_response_without_settings.settings + it "default to default settings" do + assert valid_logout_response_without_settings.settings.kind_of?(OneLogin::RubySaml::Settings) end it "accept constructor-injected settings" do refute_nil valid_logout_response.settings diff --git a/test/saml_message_test.rb b/test/saml_message_test.rb index 7aa494d38..a841e416c 100644 --- a/test/saml_message_test.rb +++ b/test/saml_message_test.rb @@ -10,7 +10,7 @@ class RubySamlTest < Minitest::Test let(:response_document_xml) { read_response("adfs_response_xmlns.xml") } it "return decoded raw saml" do - decoded_raw = saml_message.send(:decode_raw_saml, logout_request_deflated_base64) + decoded_raw = saml_message.send(:decode_raw_saml, logout_request_deflated_base64, settings) assert logout_request_document, decoded_raw end @@ -64,9 +64,9 @@ class RubySamlTest < Minitest::Test data = prefix + "A" * (200000 * 1024) + suffix bomb = Base64.encode64(Zlib::Deflate.deflate(data, 9)[2..-5]) - assert_raises(OneLogin::RubySaml::ValidationError, "Encoded SAML Message exceeds " + OneLogin::RubySaml::SamlMessage::MAX_BYTE_SIZE.to_s + " bytes, so was rejected") do + assert_raises(OneLogin::RubySaml::ValidationError, "Encoded SAML Message exceeds " + OneLogin::RubySaml::Settings::DEFAULTS[:message_max_bytesize].to_s + " bytes, so was rejected") do saml_message = OneLogin::RubySaml::SamlMessage.new - saml_message.send(:decode_raw_saml, bomb) + saml_message.send(:decode_raw_saml, bomb, settings) end end end diff --git a/test/settings_test.rb b/test/settings_test.rb index e9b0a8dbe..cebc27294 100644 --- a/test/settings_test.rb +++ b/test/settings_test.rb @@ -18,7 +18,7 @@ class SettingsTest < Minitest::Test :single_logout_service_url, :single_logout_service_binding, :sp_name_qualifier, :name_identifier_format, :name_identifier_value, :name_identifier_value_requested, :sessionindex, :attributes_index, :passive, :force_authn, - :compress_request, :double_quote_xml_attribute_values, :protocol_binding, + :compress_request, :double_quote_xml_attribute_values, :message_max_bytesize, :protocol_binding, :security, :certificate, :private_key, :authn_context, :authn_context_comparison, :authn_context_decl_ref, :assertion_consumer_logout_service_url, @@ -41,6 +41,7 @@ class SettingsTest < Minitest::Test :idp_sso_service_url => "http://sso.muda.no/sso", :idp_slo_service_url => "http://sso.muda.no/slo", :idp_cert_fingerprint => "00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00", + :message_max_bytesize => 750000, :valid_until => '2029-04-16T03:35:08.277Z', :name_identifier_format => "urn:oasis:names:tc:SAML:2.0:nameid-format:transient", :attributes_index => 30, diff --git a/test/xml_security_test.rb b/test/xml_security_test.rb index 67c61d22c..5e82d41c1 100644 --- a/test/xml_security_test.rb +++ b/test/xml_security_test.rb @@ -243,6 +243,7 @@ class XmlSecurityTest < Minitest::Test settings.assertion_consumer_service_url = "https://sp.example.com/acs" settings.single_logout_service_url = "https://sp.example.com/sls" end + let(:settings) { OneLogin::RubySaml::Settings.new } it "sign an AuthNRequest" do request = OneLogin::RubySaml::Authrequest.new.create_authentication_xml_doc(settings)