Skip to content

Commit

Permalink
Fixes #37614 - Use SHA512 for password hashing when no OS is set
Browse files Browse the repository at this point in the history
In e2dee7d the default for new
operating systems was changed to SHA512, but there's an edge case where
no is know for a host that makes it use SHA256.

SHA512 has been the recommended value for hashing since EL5 and at least
in EL6 it was also the default (didn't have EL5 on hand to check
/etc/login.defs).
  • Loading branch information
ekohl authored and ares committed Jul 11, 2024
1 parent a39297e commit e78bd15
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 4 deletions.
2 changes: 1 addition & 1 deletion app/models/concerns/host_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def crypt_pass(unencrypted_pass, pass_kind)

case pass_kind
when :root
operatingsystem.nil? ? PasswordCrypt.passw_crypt(unencrypted_pass) : PasswordCrypt.passw_crypt(unencrypted_pass, operatingsystem.password_hash)
PasswordCrypt.passw_crypt(unencrypted_pass, operatingsystem&.password_hash)
when :grub
PasswordCrypt.grub2_passw_crypt(unencrypted_pass)
else
Expand Down
7 changes: 6 additions & 1 deletion app/services/password_crypt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

class PasswordCrypt
ALGORITHMS = {'SHA512' => '$6$', 'SHA256' => '$5$', 'Base64' => '', 'Base64-Windows' => ''}
# Matches ENCRYPT_METHOD in /etc/login.defs on EL6+
# When changing this, be sure to add a migration for operatingsystems'
# default value
DEFAULT_HASH_ALGORHITHM = 'SHA512'

if Foreman::Fips.md5_available?
ALGORITHMS['MD5'] = '$1$'
Expand All @@ -12,7 +16,8 @@ def self.generate_linux_salt
SecureRandom.alphanumeric(16)
end

def self.passw_crypt(passwd, hash_alg = 'SHA256')
def self.passw_crypt(passwd, hash_alg = nil)
hash_alg ||= DEFAULT_HASH_ALGORHITHM
raise Foreman::Exception.new(N_("Unsupported password hash function '%s'"), hash_alg) unless ALGORITHMS.has_key?(hash_alg)

case hash_alg
Expand Down
4 changes: 2 additions & 2 deletions test/models/hostgroup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class HostgroupTest < ActiveSupport::TestCase
test "root_pass inherited from settings if blank" do
Setting[:root_pass] = '12345678'
PasswordCrypt.expects(:crypt_gnu_compatible?).at_least_once.returns(true)
PasswordCrypt.expects(:passw_crypt).with(Setting[:root_pass]).at_least_once.returns(Setting[:root_pass])
PasswordCrypt.expects(:passw_crypt).with(Setting[:root_pass], nil).at_least_once.returns(Setting[:root_pass])
hostgroup = FactoryBot.build(:hostgroup, :root_pass => '')
assert_equal '12345678', hostgroup.root_pass
hostgroup.save!
Expand All @@ -234,7 +234,7 @@ class HostgroupTest < ActiveSupport::TestCase
test "root_pass inherited from settings if group and parent are blank" do
Setting[:root_pass] = '12345678'
PasswordCrypt.expects(:crypt_gnu_compatible?).at_least_once.returns(true)
PasswordCrypt.expects(:passw_crypt).with(Setting[:root_pass]).at_least_once.returns(Setting[:root_pass])
PasswordCrypt.expects(:passw_crypt).with(Setting[:root_pass], nil).at_least_once.returns(Setting[:root_pass])
parent = FactoryBot.create(:hostgroup, :root_pass => '')
hostgroup = FactoryBot.build(:hostgroup, :parent => parent, :root_pass => '')
assert_equal '12345678', hostgroup.root_pass
Expand Down

0 comments on commit e78bd15

Please sign in to comment.