Resist directory traversal attacks through indirections.

In various versions of Puppet it was possible to cause a directory traversal
attack through the SSLFile indirection base class.  This was variously
triggered through the user-supplied key, or the Subject of the certificate, in
the code.

Now, we detect bad patterns down in the base class for our indirections, and
fail hard on them.  This reduces the attack surface with as little disruption
to the overall codebase as possible, making it suitable to deploy as part of
older, stable versions of Puppet.

In the long term we will also address this higher up the stack, to prevent
these problems from reoccurring, but for now this will suffice.

Huge thanks to Kristian Erik Hermansen <kristian.hermansen@gmail.com> for the
responsible disclosure, and useful analysis, around this defect.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#9794) k5login can overwrite arbitrary files as root

The k5login type is typically used to manage a file in the home directory of a
user; the explicit purpose of the files is to allow access to other users.

It writes to the target file directly, as root, without doing anything to
secure the file. That would allow the owner of the home directory to symlink
to anything on the system, and have it replaced with the  correct  content of
the file. Which is a fairly obvious escalation to root the next time Puppet
runs.

Now, instead, fix that to securely write the target file in a predictable and
secure fashion, using the `secure_open` helper.

Fixes CVE-2011-3869

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
Drop privileges before creating and chmodding SSH keys.

Previously, potentially abusable chown and chmod calls were performed as
root.  This tries to moves as much as possible into code which is run
after privileges have been dropped.

Huge thanks to Ricky Zhou <ricky@fedoraproject.org> for discovering this and
supplying the security fix.  Awesome work.

Fixes CVE-2011-3870

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#9792) Predictable temporary filename in ralsh.

When ralsh is used in edit mode the temporary filename is in a shared
directory, and is absolutely predictable.  Worse, it won't be touched until
well after the startup of the command.

It can be tricked into writing through a symlink to edit any file on the
system, or to create through it, but worse - the file is reopened with the
same name later, so it can have the target replaced between edit and
operate...

The only possible mitigation comes from the system editor and the behaviour it
has around editing through symbolic links, which is very weak.

This improves this to prefer the current working directory for the temporary
file, and to be somewhat less predictable and more safe in conjuring it into
being.

Fixes CVE-2011-3871

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
Ensure that all tests pass before fixing CVE-2011-3872

When the head of 0.25.x was moved on a number of tests were failing.  They
made it significantly harder to tell if we were correctly merging back code,
and what broke as part of that.

This "fixes" those, cheaply: disabling most of them, and fixing the few that
had obvious or cheap resolutions to get back to passing.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#6928) backport Symbol#to_proc for Ruby < 1.8.7

We use the &:foo symbol-to-proc syntax in some of our code, so to avoid
problems on Ruby earlier than 1.8.7 we should backport the support in our
monkey-patch file.

Backported to 2.6.x to ease working there as part of #2848.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/util/monkey_patches.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#6928) Don't blow up when the method is undefined...

Use the same model for testing instance methods as the rest of the code.

Reviewed-By: Max Martin <max@puppetlabs.com>
(#2848) Set `certdnsnames` values into the CSR.

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>

Conflicts:

	lib/puppet/ssl/certificate_request.rb
	spec/unit/ssl/certificate_request_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) extract the subjectAltName value from the CSR.

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>

Conflicts:

	lib/puppet/ssl/certificate_request.rb
	spec/unit/ssl/certificate_request_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Reject unknown (== all) extensions on the CSR.

If we get a CSR with a request extension that we don't recognize, we used to
just ignore those.  This can lead to dangerous situations, or at least
surprising behaviour, since we wouldn't copy those extensions into the final
certificate.

This changes that, by establishing an internal policy that we will whitelist
acceptable request extensions, and will hard-reject anything that isn't on
that.

Once something passes the whitelist we will have further policy, which will
enforce sanity in the request => certificate path.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/ssl/certificate_authority.rb
	spec/unit/ssl/certificate_authority_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Rewrite SSL Certificate Factory, fixing `subjectAltName` leak.

This is a major rewrite of the SSL CertificateFactory, which transforms the
way that we build the certificate we are about to sign in response to a CSR.

The main body of rework is to clean up the code and make it easier to manage
and validate, but there are two essential changes:

1. We no longer inject `subjectAltName` from local `certdnsnames`
   configuration option into every certificate we generate.  This fixes
   CVE-2011-3872, and prevents issuing client certificates that can
   impersonate the master.

2. *All* request extensions from the CSR are transported into the final
   certificate.  This includes `basicConstraints`; we rely on other layers of
   the Puppet stack having validated the code to this point.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/ssl/certificate_authority.rb
	lib/puppet/ssl/certificate_factory.rb
	spec/unit/ssl/certificate_factory_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#7224) Add a helper to Puppet::SSL::Certificate to retrieve alternate names

Alternate names, if present, are specified in the subjectAltName extension of
the certificate. The values are in the form:

"DNS:alternate_name1, DNS:alternate_name2"

This helper will retrieve the value of the subjectAltName extension and extract
the alternate names, returning and empty list if the extension is absent. This
will make it easier to access the entire list of possible names for a
certificate, rather than just the common name; this is helpful for generating
more detailed SSL error messages.

Paired-With: Jacob Helwig <jacob@puppetlabs.com>

Conflicts:

	lib/puppet/ssl/certificate.rb
	spec/unit/ssl/certificate_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) List subject alt names in output of puppet cert --list

Given that it's potentially risky to sign a certificate which has alt
names, we now surface that information so the user can make an informed
decision when signing. This also includes some changes to the format of
the output, grouping certs by status to make it more easily scannable.

Conflicts:

	lib/puppet/ssl/certificate_authority/interface.rb
	spec/unit/ssl/certificate_authority/interface_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) CSR subjectAltNames handling while signing.

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>

Conflicts:

	lib/puppet/application/cert.rb
	lib/puppet/defaults.rb
	lib/puppet/ssl/certificate_authority.rb
	lib/puppet/ssl/certificate_authority/interface.rb
	lib/puppet/ssl/certificate_request.rb
	lib/puppet/ssl/host.rb
	lib/puppet/util/command_line/puppetca
	spec/unit/ssl/certificate_authority/interface_spec.rb
	spec/unit/ssl/host_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Use `certdnsnames` when bootstrapping a local master.

We don't have any formal way to determine if a node is a master or not, which
makes it hard to authoritatively answer the question of "should `certdnsnames`
apply to this node".

However, we can assume that if you are the CA then you are also a master node.
You can't be one without the other.

We can also assume that if you are running as master, but are not a CA, you
intend to continue in that fashion.  This is a weaker heuristic, but should be
effective enough for folks that start a master at the right time...

This means that the basic bootstrapping case uses that setting as it is
designed, and since we just generated the CA certificate we can fairly
confidently trust that we are doing the right thing with the request.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/ssl/certificate_authority.rb
	lib/puppet/ssl/host.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Rename `certdnsnames` to match new behaviour.

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>

Conflicts:

	acceptance/pending/ticket_3360_allow_duplicate_csr_with_option_set.rb
	acceptance/tests/ticket_5477_master_not_dectect_sitepp.rb
	lib/puppet/defaults.rb
	lib/puppet/ssl/host.rb
	spec/integration/defaults_spec.rb
	spec/unit/ssl/host_spec.rb
	test/lib/puppettest/exetest.rb
	test/network/server/webrick.rb
	test/ral/type/filesources.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) rename subject-alt-name option to dns-alt-names

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>

Conflicts:

	lib/puppet/application/cert.rb
	lib/puppet/ssl/certificate_authority.rb
	lib/puppet/ssl/certificate_authority/interface.rb
	lib/puppet/ssl/certificate_request.rb
	lib/puppet/ssl/host.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
Wire up the `setbycli` slot in Puppet settings.

There was an existing slot on the setting object that tracked if the value had
been set on the command line or not, but it was not wired up to the rest of
the system.

This wires that up, allowing us to use that to determine if the
`dns-alt-names` setting should apply just to the local node, or to all nodes,
safely from the `puppet cert` application.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/util/settings.rb
	spec/unit/util/settings_spec.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Migrate `dns-alt-names` back to settings.

Now that all the other changes have been made, the `dns-alt-names` can migrate
back to the settings where they apply only for generating the CSR for the
local machine.

The one exception to that rule is the `puppet cert generate` application, in
which we want to pass any *command line* value of `dns-alt-names` into the CSR
for a foreign node, but not the value from anywhere but the command line.

Since the settings now track the CLI origin of a setting, we can use that to
derive the data and pass it around appropriately to make that so.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/application/cert.rb
	lib/puppet/defaults.rb
	lib/puppet/ssl/host.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
(#2848) Only mark `subjectAltName` critical if `subject` is empty.

From X509v3, the subjectAltName extension should only be marked critical if
the subject DN is empty:

"If the subject field contains an empty sequence, then the issuing CA MUST
 include a subjectAltName extension that is marked as critical. When including
 the subjectAltName extension in a certificate that has a non-empty subject
 distinguished name, conforming CAs SHOULD mark the subjectAltName extension
 as non-critical."

This applies that rule, by always marking the `subjectAltName` as
non-critical, because we never have an empty name in our system.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>

Conflicts:

	lib/puppet/ssl/certificate_request.rb

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>