Age | Commit message (Collapse) | Author | Files | Lines |
|
When a CSR custom attribute or extension request was passed to a CSR for
inclusion, the resulting error would be raised up from the openssl error
and would not be handled by anything, resulting in error messages like
'first num too long'. Since this is all but useless from the perspective
of a user, this commit adds handling to CSR creation. Errors will be
wrapped to indicate which OID was causing the failure.
|
|
RFC 2985 indicates that CSR attributes may contain arbitrarily complex
data, but the `openssl req` command can only handle attributes with
string values. This commit changes the csr attribute structure to simple
key/value pairs so that the attributes are properly displayed by
OpenSSL. If richer data types are need then data can be serialized as
JSON or YAML and added as a CSR value.
|
|
The `Puppet::SSL::CertificateRquest#subject_alt_names` method was trying to
select all of the subjectAltName extensions in the extension requests,
but was accidentally assigning that OID to all extensions instead of
comparing them.
Before 63cecc56 subjectAltName was the only possible extension request so
this was innocuous, but following that commit this error was destructive.
This commit resolves the issue by using the comparison operator instead
of assignment.
|
|
|
|
When a CSR is generated, additional information may need to be attached
to the CSR for use when the CSR is signed. This commit allows an
additional hash of OID/value pairs to be included as CSR attributes when
the CSR is generated.
To ensure that user specified attributes cannot overwrite attributes
added by Puppet itself, adding attributes that deal with certificate extension
requests will fail the CSR generation process.
Original patch by Patrick Hemmer <patrick.hemmer@gmail.com>
|
|
Without this patch applied, the x.509 CN extraction improvements cause
the following spec failure in the certificate_request_spec.rb file
1) Puppet::SSL::CertificateRequest when converting from a string should create a CSR ...
Failure/Error: described_class.from_s("my csr")
NoMethodError:
undefined method `to_a' for "/CN=Foo.madstop.com":String
# ./lib/puppet/util/ssl.rb:23:in `cn_from_subject'
# ./lib/puppet/ssl/base.rb:54:in `name_from_subject'
# ./lib/puppet/ssl/base.rb:62:in `from_instance'
# ./lib/puppet/ssl/base.rb:71:in `from_s'
# ./spec/unit/ssl/certificate_request_spec.rb:42:in `block (3 levels) in <top (required)>'
Poor stubbing is the root cause of this problem. The mock certificate
object returns a String from `#subject`, however the system expects a
kind of `OpenSSL::X509::Name`.
This patch addresses the problem by changing the mock object to return
an instance of `OpenSSL::X509::Name` when the mocked `#subject` instance
is called.
|
|
This adds a general validation method to check that only valid instances can
be passed into the indirector. Since access control is based on the URI but
many operations directly use the serialized instance passed in, it was
possible to bypass restrictions by passing in a custom object. Specifically it
was possible to cause the puppet kick indirection to execute arbitrary code by
passing in an instance of the wrong class. This validates that the instance is
of the correct type and that the name matches the key that was used to
authorize the request.
|
|
Without this patch applied puppet fails to create a CSR
with the following message -
Error: Could not request certificate: uninitialized constant OpenSSL::Digest::SHA256
This patch adds a class Puppet::SSL::CertificateSigner that
takes care of signing certificates in CSR creation and via
the CA. This class assumes a default hash algorithm of
SHA256 and if not available falls back to SHA1. For the
sake of FIPS 140-2 compliance it does not try MD5 or any
earlier hash algorithms.
|
|
Without this patch Ruby 1.9 is still complaining loudly about trying to
parse the spec files. The previous attempt to clean up this problem in
edc3ddf works for Ruby 1.8 but not 1.9.
I'd prefer to remove the shebang lines entirely, but doing so will cause
encoding errors in Ruby 1.9. This patch strives for a happy middle
ground of convincing Ruby it is actually working with Ruby while not
confusing it to think it should exec() to rspec.
This patch is the result of the following command run against the source
tree:
find spec -type f -print0 | \
xargs -0 perl -pl -i -e 's,^\#\!\s?/(.*)rspec,\#! /usr/bin/env ruby,'
|
|
This uses the Puppet::SSL::Digest class to show the digest and digest
algorithm used when generating a CSR. This also refactors the
CertificateRequest specs to significantly reduce stubbing.
|
|
Instead of creating certificates by turning OpenSSL::X509::Certificate
instances into strings then back into a new instance, just reuse them.
This method is on the base SSL class, so it can be used for keys,
certificate requests, etc.
The side effect of this change is that extracting the certificate 'name'
out of its subject is abstracted (`name_from_subject()`), and it no
longer calls downcase on the name. This is safe since it's downcased in
the base class constructor, and also provides a prettier 'name' if
needed.
|
|
Without this patch some spec files are using `ruby -S rspec` and others
are using `rspec`.
We should standardize on a single form of the interpreter used for spec
files.
`ruby -S rspec` is the best choice because it correctly informs editors
such as Vim with Syntastic that the file is a Ruby file rather than an
Rspec file.
|
|
Change the default digest algorithm away from MD5
This commit is one step along the way to FIPS 140-2 compliance (#8120).
In a FIPS 140-2 environment, MD5 is not available. Older versions of
Ruby (1.8.7, 1.9.2) will SIGABRT when trying to use MD5 because they
don't properly check the return code from openssl.
Because the fingerprints between agent and master aren't
machine-verified and `puppet cert list --digest <digest>` supports any
of the digests, this commit is backwards and forwards compatibile.
Later portions of #8120 will make the default digest algorithm
configurable.
|
|
Now that `dns_alt_names` applies to local CSR generation, there is no need
for a special configuration option only applied to a master certificate.
Eliminating that option allows us to simplify the overall model, and provide
more uniform access to the `subjectAltName` setting when required.
Documentation about the option is also updated.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
The various implementations of the `subject_alt_names` accessor on SSL objects
had different return types when the records were not present: nil, or an empty
array.
This unifies them to consistently return an empty array, which makes it easier
to iterate across the code. It also cleans up some logic around handling
policy on those, to make it cleaner and quicker by not duplicating the same
"are there any subjectAltName values?" test across every policy check.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
The name of the option for adding subjectAltName fields to the request didn't
really indicate what it actually did: we forced the names into the DNS label
space, and generally imposed a bunch of additional behaviours.
This renames the option to better match the intended use.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
As part of the update to SSL the behaviour of the `certdnsnames` options
changed sufficiently that it would be terribly confusing to retain it.
Instead, modify the setting to warn that it is ignored, and add a new setting
to set the default subjectAltName value for bootstrapping a master
certificate.
This retains the one really useful part of the feature, without the risk that
someone will accidentally use the old name and receive a nasty surprise.
It should also draw more attention to formerly insecure configurations.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
The puppet cert application now supports the --allow-subject-alt-names command
line flag to specify that the certificate to sign should contain a
subjectAltName extension request.
If the option is not specified, an attempt to sign a certificate with alt
names will fail. Similarly, if the option is specified, an attempt to sign a
certificate without alt names will fail.
The latter behavior is deliberate, and is intended to help stop users from
developing a habit of always supplying the option, and thus accidentally
signing a dangerous certificate without realizing it when one comes along.
It also adds the `--subject-alt-name` option that will add a `subjectAltName`
extension request to a CSR when it is generated. This allows an additional
service to be bootstrapped with those names.
Generally only one of the two options should be required, as the CSR and the
signing process are distinct.
Mostly by Nick Lewis, additional code by Daniel Pittman
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
When a CSR is submitted, we need to inspect it and determine if there are any
subjectAltNames present in the request, and if those are legal.
The logic to validate the content of an extension is complex, and decomposing
the content requires digging into various X509 and ASN.1 related classes.
This commit adds a `subject_alt_names` method that will extract the extensions
containing subjectAltName values from the CSR, and reformat them into a useful
set of content suitable for use in Ruby.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
We were incorrectly applying the `certdnsnames` setting when a certificate was
signed, not when a CSR was generated. This would misapply the server setting
to all nodes in the infrastructure, rather than applying the client-supplied
version.
This commit adds `certdnsnames` to the CSR, allowing the client to pass their
desired set of names to the CA to be propagated into the final certificate.
It does not alter the behaviour when the certificate is generated.
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
|
|
We now use a shebang of: #!/usr/bin/env rspec
This enables the direct execution of spec tests again, which was lost earlier
during the transition to more directly using the rspec2 runtime environment.
|
|
rspec2 automatically sets a bunch of load-path stuff we were by hand, so we
can just stop. As a side-effect we can now avoid a whole pile of stupid things
to try and include the spec_helper.rb file...
...and then we can stop protecting spec_helper from evaluating twice, since we
now require it with a consistent name. Yay.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
|
Doing a require to a relative path can cause files to be required more
than once when they're required from different relative paths. If you
expand the path fully, this won't happen. Ruby 1.9 also requires that
you use expand_path when doing these requires.
Paired-with: Jesse Wolfe
|
|
In commit b67f4c68503ca3abf0a55857da452e46fa75abd5 I fixed one place
that was causing order dependent test failures in
spec/unit/ssl/certificate_request_spec.rb, but missed another case that
was causing a similar failure. To find all the order dependent failures
with that file I ran the following bash script from the spec directory:
for TEST in `find . -name "*.rb" -type f`; do
spec $TEST unit/ssl/certificate_request_spec.rb > /dev/null 2>&1
if [[ $? != 0 ]]; then
echo $TEST
fi
done
Paired-with: Jesse Wolfe
|
|
This change replaces calls to <model object>.save with calls to <model
class>.indirection.save(<model object>). This makes the use of the
indirector explicit rather than implicit so that it will be easier to
search for all indirector call sites using grep. This is an
intermediate refactor on the way towards allowing indirector calls to
be explicitly routed to multiple termini.
This patch affects tests only; the next patch will make the
corresponding change to the code.
|
|
Autosigning was previously accomplished by overriding
CertificateRequest#save. This meant that it wouldn't work if
certificate requests were saved via a direct call to Indirection#save.
Changed it to use the indirector :extend mechanism, which works no
matter how the save is invoked.
|
|
The first argument was often nil, and the second was mandatory.
|
|
Replaced 106806 occurances of ^( +)(.*$) with
The ruby community almost universally (i.e. everyone but Luke, Markus, and the other eleven people
who learned ruby in the 1900s) uses two-space indentation.
3 Examples:
The code:
end
# Tell getopt which arguments are valid
def test_get_getopt_args
element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new
assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
becomes:
end
# Tell getopt which arguments are valid
def test_get_getopt_args
element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new
assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
The code:
assert_equal(str, val)
assert_instance_of(Float, result)
end
# Now test it with a passed object
becomes:
assert_equal(str, val)
assert_instance_of(Float, result)
end
# Now test it with a passed object
The code:
end
assert_nothing_raised do
klass[:Yay] = "boo"
klass["Cool"] = :yayness
end
becomes:
end
assert_nothing_raised do
klass[:Yay] = "boo"
klass["Cool"] = :yayness
end
|
|
Part 2 re-did the change on the spec files, which it shouldn't have.
|
|
Some spec files like active_record.rb had names that would confuse the
load path and get loaded instead of the intended implentation when the
spec was run from the same directory as the file.
Author: Matt Robinson <matt@puppetlabs.com>
Date: Fri Jun 11 15:29:33 2010 -0700
|
|
Some spec files like active_record.rb had names that would confuse the
load path and get loaded instead of the intended implentation when the
spec was run from the same directory as the file.
Author: Matt Robinson <matt@puppetlabs.com>
Date: Fri Jun 11 15:29:33 2010 -0700
|