diff --git a/CHANGELOG.md b/CHANGELOG.md index f6de1dbe930..392a1a9c1cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New features * [#7735](https://github.com/rubocop-hq/rubocop/issues/7735): `NodePattern` and `AST` classes have been moved to the [`rubocop-ast` gem](https://github.com/rubocop-hq/rubocop-ast). ([@marcandre][]) +* [#7950](https://github.com/rubocop-hq/rubocop/pull/7950): Add new `Lint/DeprecatedOpenSSLConstant` cop. ([@bdewater][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 15a2796d399..cfc3bb8a194 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1370,6 +1370,11 @@ Lint/DeprecatedClassMethods: Enabled: true VersionAdded: '0.19' +Lint/DeprecatedOpenSSLConstant: + Description: "Don't use algorithm constants for `OpenSSL::Cipher` and `OpenSSL::Digest`." + Enabled: pending + VersionAdded: '0.84' + Lint/DisjunctiveAssignmentInConstructor: Description: 'In constructor, plain assignment is preferred over disjunctive.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 99f723a8c14..4ca1bcad195 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -237,6 +237,7 @@ require_relative 'rubocop/cop/lint/circular_argument_reference' require_relative 'rubocop/cop/lint/debugger' require_relative 'rubocop/cop/lint/deprecated_class_methods' +require_relative 'rubocop/cop/lint/deprecated_open_ssl_constant' require_relative 'rubocop/cop/lint/disjunctive_assignment_in_constructor' require_relative 'rubocop/cop/lint/duplicate_case_condition' require_relative 'rubocop/cop/lint/duplicate_hash_key' diff --git a/lib/rubocop/cop/lint/deprecated_open_ssl_constant.rb b/lib/rubocop/cop/lint/deprecated_open_ssl_constant.rb new file mode 100644 index 00000000000..61fc1dd9101 --- /dev/null +++ b/lib/rubocop/cop/lint/deprecated_open_ssl_constant.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # Algorithmic constants for `OpenSSL::Cipher` and `OpenSSL::Digest` + # deprecated since OpenSSL version 2.2.0. Prefer passing a string + # instead. + # + # @example + # + # # Example for OpenSSL::Cipher instantiation. + # + # # bad + # OpenSSL::Cipher::AES.new(128, :GCM) + # + # # good + # OpenSSL::Cipher.new('AES-128-GCM') + # + # @example + # + # # Example for OpenSSL::Digest instantiation. + # + # # bad + # OpenSSL::Digest::SHA256.new + # + # # good + # OpenSSL::Digest.new('SHA256') + # + # @example + # + # # Example for ::Digest inherited class methods. + # + # # bad + # OpenSSL::Digest::SHA256.digest('foo') + # + # # good + # OpenSSL::Digest.digest('SHA256', 'foo') + # + class DeprecatedOpenSSLConstant < Cop + include RangeHelp + + MSG = 'Use `%s.%s(%s)`' \ + ' instead of `%s`.' + + def_node_matcher :algorithm_const, <<~PATTERN + (send + $(const + (const + (const {nil? cbase} :OpenSSL) {:Cipher :Digest}) + _) + ...) + PATTERN + + def on_send(node) + add_offense(node) if algorithm_const(node) + end + + def autocorrect(node) + algorithm_constant, = algorithm_const(node) + + lambda do |corrector| + corrector.remove(algorithm_constant.loc.double_colon) + corrector.remove(algorithm_constant.loc.name) + + corrector.replace( + correction_range(node), + "#{node.loc.selector.source}(#{replacement_args(node)})" + ) + end + end + + private + + def message(node) + algorithm_constant, = algorithm_const(node) + parent_constant = openssl_class(algorithm_constant) + replacement_args = replacement_args(node) + method = node.loc.selector.source + + format( + MSG, + constant: parent_constant, + method: method, + replacement_args: replacement_args, + original: node.source + ) + end + + def correction_range(node) + begin_pos = node.loc.selector.column + end_pos = node.loc.expression.last_column + range_between(begin_pos, end_pos) + end + + def openssl_class(node) + node.children.first.source + end + + def algorithm_name(node) + name = node.loc.name.source + + if openssl_class(node) == 'OpenSSL::Cipher' + name.scan(/.{3}/).join('-') + else + name + end + end + + def sanitize_arguments(arguments) + arguments.flat_map { |arg| arg.source.tr(":'", '').split('-') } + end + + def replacement_args(node) + algorithm_constant, = algorithm_const(node) + algorithm_name = algorithm_name(algorithm_constant) + + if openssl_class(algorithm_constant) == 'OpenSSL::Cipher' + build_cipher_arguments(node, algorithm_name) + else + (["'#{algorithm_name}'"] + node.arguments.map(&:source)).join(', ') + end + end + + def build_cipher_arguments(node, algorithm_name) + algorithm_parts = algorithm_name.split('-') + size_and_mode = sanitize_arguments(node.arguments) + "'#{(algorithm_parts + size_and_mode + ['CBC']).take(3).join('-')}'" + end + end + end + end +end diff --git a/manual/cops.md b/manual/cops.md index 0d3ef451173..c6088078a14 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -187,6 +187,7 @@ In the following section you find all available cops: * [Lint/CircularArgumentReference](cops_lint.md#lintcircularargumentreference) * [Lint/Debugger](cops_lint.md#lintdebugger) * [Lint/DeprecatedClassMethods](cops_lint.md#lintdeprecatedclassmethods) +* [Lint/DeprecatedOpenSSLConstant](cops_lint.md#lintdeprecatedopensslconstant) * [Lint/DisjunctiveAssignmentInConstructor](cops_lint.md#lintdisjunctiveassignmentinconstructor) * [Lint/DuplicateCaseCondition](cops_lint.md#lintduplicatecasecondition) * [Lint/DuplicateHashKey](cops_lint.md#lintduplicatehashkey) diff --git a/manual/cops_lint.md b/manual/cops_lint.md index 749ec4952dc..06c30b72fe4 100644 --- a/manual/cops_lint.md +++ b/manual/cops_lint.md @@ -298,6 +298,46 @@ Dir.exist?(some_path) block_given? ``` +## Lint/DeprecatedOpenSSLConstant + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Pending | Yes | Yes | 0.84 | - + +Algorithmic constants for `OpenSSL::Cipher` and `OpenSSL::Digest` +deprecated since OpenSSL version 2.2.0. Prefer passing a string +instead. + +### Examples + +```ruby +# Example for OpenSSL::Cipher instantiation. + +# bad +OpenSSL::Cipher::AES.new(128, :GCM) + +# good +OpenSSL::Cipher.new('AES-128-GCM') +``` +```ruby +# Example for OpenSSL::Digest instantiation. + +# bad +OpenSSL::Digest::SHA256.new + +# good +OpenSSL::Digest.new('SHA256') +``` +```ruby +# Example for ::Digest inherited class methods. + +# bad +OpenSSL::Digest::SHA256.digest('foo') + +# good +OpenSSL::Digest.digest('SHA256', 'foo') +``` + ## Lint/DisjunctiveAssignmentInConstructor Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/lint/deprecated_open_ssl_constant_spec.rb b/spec/rubocop/cop/lint/deprecated_open_ssl_constant_spec.rb new file mode 100644 index 00000000000..111f12583ec --- /dev/null +++ b/spec/rubocop/cop/lint/deprecated_open_ssl_constant_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::DeprecatedOpenSSLConstant do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + it 'registers an offense with cipher constant and two arguments and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Cipher::AES.new(128, :GCM) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Cipher.new('AES-128-GCM')` instead of `OpenSSL::Cipher::AES.new(128, :GCM)`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Cipher.new('AES-128-GCM') + RUBY + end + + it 'registers an offense with cipher constant and one argument and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Cipher::AES.new('128-GCM') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Cipher.new('AES-128-GCM')` instead of `OpenSSL::Cipher::AES.new('128-GCM')`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Cipher.new('AES-128-GCM') + RUBY + end + + it 'registers an offense with AES + blocksize constant and mode argument and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Cipher::AES128.new(:GCM) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Cipher.new('AES-128-GCM')` instead of `OpenSSL::Cipher::AES128.new(:GCM)`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Cipher.new('AES-128-GCM') + RUBY + end + + it 'registers an offense with AES + blocksize constant and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Cipher::AES128.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Cipher.new('AES-128-CBC')` instead of `OpenSSL::Cipher::AES128.new`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Cipher.new('AES-128-CBC') + RUBY + end + + it 'does not register an offense when using cipher with a string' do + expect_no_offenses(<<~RUBY) + OpenSSL::Cipher.new('AES-128-GCM') + RUBY + end + + it 'registers an offense when building an instance using an digest constant and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Digest::SHA256.new + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Digest.new('SHA256')` instead of `OpenSSL::Digest::SHA256.new`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Digest.new('SHA256') + RUBY + end + + it 'registers an offense when using ::Digest class methods on an algorithm constant and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Digest::SHA256.digest('foo') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Digest.digest('SHA256', 'foo')` instead of `OpenSSL::Digest::SHA256.digest('foo')`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Digest.digest('SHA256', 'foo') + RUBY + end + + it 'registers an offense when using an digest constant with chained methods and corrects' do + expect_offense(<<~RUBY) + OpenSSL::Digest::SHA256.new.digest('foo') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `OpenSSL::Digest.new('SHA256')` instead of `OpenSSL::Digest::SHA256.new`. + RUBY + + expect_correction(<<~RUBY) + OpenSSL::Digest.new('SHA256').digest('foo') + RUBY + end + + it 'does not register an offense when building digest using an algorithm string' do + expect_no_offenses(<<~RUBY) + OpenSSL::Digest.new('SHA256') + RUBY + end + + it 'does not register an offense when using ::Digest class methods with an algorithm string and value' do + expect_no_offenses(<<~RUBY) + OpenSSL::Digest.digest('SHA256', 'foo') + RUBY + end +end