voxpupuli / puppet-openssl

Puppet OpenSSL module
Apache License 2.0
38 stars 84 forks source link

Dead code #173

Open rtib opened 10 months ago

rtib commented 10 months ago

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

https://github.com/voxpupuli/puppet-openssl/blob/8915708c0593831e3e4b8b7490e426fee4761e7c/lib/puppet/provider/x509_cert/openssl.rb#L45-L54

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.

ekohl commented 3 months ago

I agree that's surprising to say the least. Just because there's a SAN doesn't mean there is no CN.