Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dead code #173

Open
rtib opened this issue Dec 18, 2023 · 0 comments
Open

Dead code #173

rtib opened this issue Dec 18, 2023 · 0 comments

Comments

@rtib
Copy link
Contributor

rtib commented Dec 18, 2023

I was reviewing the below code, because of suspected failing its purpose:

ini_file = Puppet::Util::IniConfig::PhysicalFile.new(resource[:template])
if (req_ext = ini_file.get_section('req_ext'))
if (value = req_ext['subjectAltName']) && (value.delete(' ').gsub(%r{^"|"$}, '') != alt_name.delete(' ').gsub(%r{^"|"$}, '').gsub('IPAddress', 'IP'))
return false
end
elsif (req_dn = ini_file.get_section('req_distinguished_name'))
if (value = req_dn['commonName']) && (value != cdata['CN'])
return false
end
end

My analysis unveiled, that neither the if nor the elsif branches are ever executed. This is because the ini_file's initialize method does not read and parse the file itself, thus each call to its get_section method is returning nil.

An instance of Puppet::Util::IniConfig::PhysicalFile would need the invocation of its read method to read and parse the file, but that's never happen. Using this class to parse an OpenSSL config file constructed by this module's templates/cert.cnf.epp would, however, end up raising IniParseError with the message Property with key HOME outside of a section from puppet/util/inifile.rb#L185. AFAIK, there is no specification for the format of ini files, however, Microsoft and OpenSSL seem to treat them differently. Nevertheless, Puppet::Util::IniConfig::PhysicalFile.parse method does not support keys before the first section header, thus it seems not suitable for parsing OpenSSL style config files.

In that sense the old_cert_is_equal method of the x509_cert openssl provider would need to be reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant