Hi Josh,

On Tuesday, January 15, 2013 8:50:53 AM UTC+11, Josh Cooper wrote: 

>
> I'm concerned about overloading the same flag to specify different 
> types of algorithms. To generate a CSR, we need to know what signature 
> algorithm to use, e.g. sha256WithRSAEncryption. To generate a 
> fingerprint, we only need to know what hash algorithm to apply to the 
> public key. 
>
> If we only allow the user to specify the hash portion of the signature 
> algorithm, then the code has to make hard-coded assumptions about the 
> type of encryption algorithm to use, and therefore, the types of keys 
> in use (RSA vs DSA). Sadly, puppet already does this, but I'd prefer 
> to not make it worse. 
>

Obviously it's almost the same from a coding point of view whether I extend 
--digest or create a new option, e.g.

# puppet agent -t [ --signature AGORITHM ]

That said I am still not understanding why extending --digest would not be 
the least surprising solution, i.e. allow --digest to affect the 'digest 
algorithm' a.k.a. hashing algorithm in any situation where a hashing 
algorithm is required.  This seems more consistent with familiar system 
commands like openssl where passing -sha1 in any context would select the 
hash algorithm used - whether in the context of generating a fingerprint 
(openssl dgst), or a CSR (openssl req).

Okay - perhaps after I have digested the code it will become clearer.  

I would recommend leaving the `--digest` flag specific to specifying 
> the hash algorithm when generating a fingerprint. 
>
> Then add a different flag for specifying the signature algorithm. 
> Since this infrequently changes, I'd recommend making a global Puppet 
> setting (but not ca_md). 
>
> Ideally all of our ssl and certificate handling code should be 
> agnostic with respect to the underlying algorithms. This would make it 
> possible to easily change the algorithms in the future, similar to how 
> openssl provides EVP functions: 
> http://www.openssl.org/docs/crypto/evp.html. For example, 
> Puppet::Host::SSL#digest_algorithm is an example of what not to do, 
> because it makes assumptions about the syntax of the signature 
> algorithm.
>

Is that method or the surrounding code actually being used though?

We've got in
lib/puppet/ssl/base.rb

 89   def fingerprint(md = :SHA256)
 90     mds = md.to_s.upcase
 91     digest(mds).to_hex
 92   end
 93
 94   def digest(algorithm=nil)
 95     unless algorithm
 96       algorithm = digest_algorithm
 97     end
 98
 99     Puppet::SSL::Digest.new(algorithm, content.to_der)
100   end
101
102   def digest_algorithm
103     # The signature_algorithm on the X509 cert is a combination of the 
digest
104     # algorithm and the encryption algorithm
105     # e.g. md5WithRSAEncryption, sha256WithRSAEncryption
106     # Unfortunately there isn't a consistent pattern
107     # See RFCs 3279, 5758
108     digest_re = Regexp.union(
109       /ripemd160/i,
110       /md[245]/i,
111       /sha\d*/i
112     )
113     ln = content.signature_algorithm
114     if match = digest_re.match(ln)
115       match[0].downcase
116     else
117       raise Puppet::Error, "Unknown signature algorithm '#{ln}'"
118     end
119   end
120

Maybe I'll go through the whole of lib/puppet/ssl and stick print 
statements everywhere to figure out what's actually be used and when.  But 
at the moment I don't think this code is used.

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/puppet-dev/-/6PBU58_YdsMJ.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to 
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to