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.

Sign up for GitHub

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

Jump to bottom

(SERVER-763) Read client auth info from tk-authz when configured #709

Merged
merged 4 commits into from Sep 30, 2015
Merged

(SERVER-763) Read client auth info from tk-authz when configured #709

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f9fc3f
(SERVER-763) Read client auth info from tk-authz when configured
camlow325 Sep 24, 2015
1a732ce
(SERVER-763) tk-authz doc updates to config and external SSL termination
camlow325 Sep 28, 2015
d12449b
(SERVER-763) Update tk-authz dependency to 0.1.4 and use tk-auth Ring…
camlow325 Sep 29, 2015
9f79699
(SERVER-763) Fixed a couple of typos and inlined a when in a doseq
camlow325 Sep 30, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions dev/puppet-server.conf.sample
View file
Open in desktop
Expand Up @@ -2,10 +2,6 @@ global: {
logging-config: ./dev/logback-dev.xml
}

master: {
allow-header-cert-info: false
}

product: {
update-server-url: "http://localhost/"
name: {group-id: puppetlabs.dev
Expand Down
262 changes: 227 additions & 35 deletions documentation/configuration.markdown
View file
Open in desktop

Large diffs are not rendered by default.

45 changes: 40 additions & 5 deletions documentation/external_ssl_termination.markdown
View file
Open in desktop
Expand Up @@ -15,11 +15,33 @@ You'll need to turn off SSL and have Puppet Server use the HTTP protocol instead

When using external SSL termination, Puppet Server expects to receive client certificate information via some HTTP headers.

By default, reading this data from headers is disabled. To allow Puppet Server to recognize it, edit (or create) `config.d/master.conf` and add `allow-header-cert-info: true` to the `master` config section. See [Puppet Server Configuration](./configuration.markdown) for more information on the `master.conf` file.
By default, reading this data from headers is disabled. To allow Puppet Server
to recognize it, you'll need to set the `allow-header-cert-info` setting to `true`.

Prior to the inclusion of `trapperkeeper-authorization` and an `auth.conf` file
specific to Puppet Server, you would need to edit (or create) `conf.d/master.conf`
and add `allow-header-cert-info: true` to the `master` config section. See
[Puppet Server Configuration](./configuration.markdown) for more information on
the `master.conf` file. This approach, however, is now deprecated.

Instead, it is preferred to enable `trapperkeeper-authorization` and
set the `allow-header-cert-info` setting via the `authorization` config
section. This involves the following steps:

* Migrate any custom authorization rule definitions that you may have made to core Puppet's
`/etc/puppetlabs/puppet/auth.conf` file over to the
`/etc/puppetlabs/puppetserver/conf.d/auth.conf` file.
* Set the `jruby-puppet.use-legacy-auth-conf` setting in the
`conf.d/puppetserver.conf` file to `false`.
* Add `allow-header-cert-info: true` to the `authorization` config section in
the `/etc/puppetlabs/puppetserver/conf.d/auth.conf` file.

See [Puppet Server Configuration](./configuration.markdown) for more information
on the `puppetserver.conf` and `auth.conf` files.

> **WARNING**: Setting `allow-header-cert-info` to 'true' puts Puppet Server in an incredibly vulnerable state. Take extra caution to ensure it is **absolutely not reachable** by an untrusted network.
>
> With `allow-header-cert-info` set to 'true', core Ruby Puppet application code will use only the client HTTP header values---not an SSL-layer client certificate---to determine the client subject name, authentication status, and trusted facts. This is true even if the web server is hosting an HTTPS connection. This applies to validation of the client via rules in the [auth.conf](https://docs.puppetlabs.com/guides/rest_auth_conf.html) file and any [trusted facts][trusted] extracted from certificate extensions.
> With `allow-header-cert-info` set to 'true', authorization code will use only the client HTTP header values---not an SSL-layer client certificate---to determine the client subject name, authentication status, and trusted facts. This is true even if the web server is hosting an HTTPS connection. This applies to validation of the client via rules in the [auth.conf](https://docs.puppetlabs.com/guides/rest_auth_conf.html) file and any [trusted facts][trusted] extracted from certificate extensions.
>
> If the `client-auth` setting in the `webserver` config block is set to `need` or `want`, the Jetty web server will still validate the client certificate against a certificate authority store, but it will only verify the SSL-layer client certificate---not a certificate in an `X-Client-Cert` header.

Expand All @@ -38,19 +60,32 @@ The headers you'll need to set are `X-Client-Verify`, `X-Client-DN`, and `X-Clie

Mandatory. Must be either `SUCCESS` if the certificate was validated, or something else if not. (The convention seems to be to use `NONE` for when a certificate wasn't presented, and `FAILED:reason` for other validation failures.) Puppet Server uses this to authorize requests; only requests with a value of `SUCCESS` will be considered authenticated.

You can change this header name with [the `ssl_client_verify_header` setting.](https://docs.puppetlabs.com/references/latest/configuration.html#sslclientverifyheader)
When using the `master.allow-header-cert-info: true` setting, you can change this header name with [the `ssl_client_verify_header` setting.](https://docs.puppetlabs.com/references/latest/configuration.html#sslclientverifyheader)

This setting (and its twin, `ssl_client_header`) is a bit odd: its value should be the result of transforming the desired HTTP header name into a CGI-style environment variable name. That is, to change the HTTP header to `X-SSL-Client-Verify`, you would set the setting to `HTTP_X_SSL_CLIENT_VERIFY`. (Add `HTTP_` to the front, change hyphens to underscores, and uppercase everything.)

(Puppet Server will eventually UN-munge the CGI variable name to get a valid HTTP header name, and use that name to interact directly with an HTTP request. This is a legacy quirk to ensure that the setting works the same for both Puppet Server and a Rack Puppet master; note that Rack actually does use CGI environment variables.)

Note that if you are using the `authorization.allow-header-cert-info: true`
setting, the name of the client verify header in the request must be
`X-Client-Verify`; the `ssl_client_verify_header` setting in the `puppet.conf`
file has no effect on how the client verify header is processed.

### `X-Client-DN`

Mandatory. Must be the [Subject DN][] of the agent's certificate, if a certificate was presented. Puppet Server uses this to authorize requests.

> **Note:** Currently, the DN must be in RFC-2253 format (comma-delimited). Due to a bug ([SERVER-213](https://tickets.puppetlabs.com/browse/SERVER-213)), Puppet Server can't decode OpenSSL-style DNs (slash-delimited). Note that Apache's `mod_ssl` `SSL_CLIENT_S_DN` variable uses OpenSSL-style DNs.
> **Note:** Currently, when using `master.allow-header-cert-info: true`, the DN must be in RFC-2253 format (comma-delimited). Due to a bug ([SERVER-213](https://tickets.puppetlabs.com/browse/SERVER-213)), Puppet Server can't decode OpenSSL-style DNs (slash-delimited). Note that Apache's `mod_ssl` `SSL_CLIENT_S_DN` variable uses OpenSSL-style DNs. Note
that the bug does not apply when `authorization.allow-header-cert-info` is set
to true. `trapperkeeper-authorization` supports decoding both the RFC-2253
and OpenSSL "slash-delimited" DN formats.

When using the `master.allow-header-cert-info: true` setting, you can change this header name with [the `ssl_client_header` setting.](https://docs.puppetlabs.com/references/latest/configuration.html#sslclientheader) See the note above for more info about this setting's expected values.

You can change this header name with [the `ssl_client_header` setting.](https://docs.puppetlabs.com/references/latest/configuration.html#sslclientheader) See the note above for more info about this setting's expected values.
Note that if you are using the `authorization.allow-header-cert-info: true`
setting, the name of the client DN header in the request must be
`X-Client-DN`; the `ssl_client_header` setting in the `puppet.conf` file has no
effect on how the client DN header is processed.

[subject dn]: https://docs.puppetlabs.com/background/ssl/cert_anatomy.html#the-subject-dn-cn-certname-etc

Expand Down
2 changes: 1 addition & 1 deletion project.clj
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@
;; end version conflict resolution dependencies

[puppetlabs/trapperkeeper ~tk-version]
[puppetlabs/trapperkeeper-authorization "0.1.3"]
[puppetlabs/trapperkeeper-authorization "0.1.4"]
[puppetlabs/kitchensink ~ks-version]
[puppetlabs/ssl-utils "0.8.1"]
[puppetlabs/dujour-version-check "0.1.2" :exclusions [org.clojure/tools.logging]]
Expand Down
10 changes: 5 additions & 5 deletions src/clj/puppetlabs/services/ca/certificate_authority_service.clj
View file
Open in desktop
Expand Up @@ -18,11 +18,11 @@
puppet-version (get-in-config [:puppet-server :puppet-version])]
(if (not-empty (:access-control settings))
(log/warn
(str "The 'client-whitelist' and 'authorization-required' settings in "
"the 'certificate-authority.certificate-status' section are "
"deprecated and will be removed in a future release. Remove these "
"settings and instead create an appropriate authorization rule in "
"the /etc/puppetlabs/puppetserver/conf.d/auth.conf file.")))
"The 'client-whitelist' and 'authorization-required' settings in the"
"'certificate-authority.certificate-status' section are deprecated and"
"will be removed in a future release. Remove these settings and create"
"an appropriate authorization rule in the"
"/etc/puppetlabs/puppetserver/conf.d/auth.conf file."))
(ca/initialize! settings)
(log/info "CA Service adding a ring handler")
(add-ring-handler
Expand Down
12 changes: 6 additions & 6 deletions src/clj/puppetlabs/services/jruby/jruby_puppet_service.clj
View file
Open in desktop
Expand Up @@ -29,12 +29,12 @@
(core/verify-config-found! config)
(log/info "Initializing the JRuby service")
(if (:use-legacy-auth-conf config)
(log/warn
(str "The 'use-legacy-auth-conf' setting in the 'jruby-puppet' section "
"is deprecated and will be removed in a future release. Change "
"the value of this setting to 'false' and migrate your authorization "
"rule definitions in the /etc/puppetlabs/puppet/auth.conf file to "
"the /etc/puppetlabs/puppetserver/conf.d/auth.conf file.")))
(log/warn "The 'jruby-puppet.use-legacy-auth-conf' setting is set to"
"'true'. Support for the legacy Puppet auth.conf file is"
"deprecated and will be removed in a future release. Change"
"this setting to 'false' and migrate your authorization rule"
"definitions in the /etc/puppetlabs/puppet/auth.conf file to"
"the /etc/puppetlabs/puppetserver/conf.d/auth.conf file."))
(core/add-facter-jar-to-system-classloader (:ruby-load-path config))
(let [pool-context (core/create-pool-context config profiler agent-shutdown-fn)]
(jruby-agents/send-prime-pool! pool-context)
Expand Down
10 changes: 5 additions & 5 deletions src/clj/puppetlabs/services/puppet_admin/puppet_admin_service.clj
View file
Open in desktop
Expand Up @@ -19,11 +19,11 @@
settings)]
(if (not-empty whitelist-settings)
(log/warn
(str "The 'client-whitelist' and 'authorization-required' settings in "
"the 'puppet-admin' section are deprecated and will be removed "
"in a future release. Remove these settings and instead create an "
"appropriate authorization rule in the "
"/etc/puppetlabs/puppetserver/conf.d file.")))
"The 'client-whitelist' and 'authorization-required' settings in"
"the 'puppet-admin' section are deprecated and will be removed"
"in a future release. Remove these settings and create an"
"appropriate authorization rule in the"
"/etc/puppetlabs/puppetserver/conf.d/auth.conf file."))
(add-ring-handler
this
(core/build-ring-handler route
Expand Down
155 changes: 91 additions & 64 deletions src/clj/puppetlabs/services/request_handler/request_handler_core.clj
View file
Open in desktop
Expand Up @@ -4,11 +4,11 @@
(com.puppetlabs.puppetserver JRubyPuppetResponse))
(:require [clojure.tools.logging :as log]
[clojure.string :as string]
[puppetlabs.kitchensink.core :as ks]
[puppetlabs.ssl-utils.core :as ssl-utils]
[ring.util.codec :as ring-codec]
[ring.util.response :as ring-response]
[slingshot.slingshot :as sling]
[puppetlabs.trapperkeeper.authorization.ring :as ring-auth]
[puppetlabs.puppetserver.ring.middleware.params :as pl-ring-params]
[puppetlabs.services.jruby.jruby-puppet-service :as jruby]))

Expand Down Expand Up @@ -39,17 +39,6 @@
:ssl-client-header (unmunge-http-header-name
(:ssl-client-header puppet-server))})

(defn get-cert-common-name
"Given a request, return the Common Name from the client certificate subject."
[ssl-client-cert]
(if-let [cert ssl-client-cert]
(if-let [cert-dn (-> cert .getSubjectX500Principal .getName)]
(if-let [cert-cn (ks/cn-for-dn cert-dn)]
cert-cn
(log/errorf "cn not found in client certificate dn: %s"
cert-dn))
(log/error "dn not found for client certificate subject"))))

(defn response->map
"Converts a JRubyPuppetResponse instance to a map."
[response]
Expand Down Expand Up @@ -113,13 +102,27 @@

(defn header-auth-info
"Return a map with authentication info based on header content"
[header-dn-name header-dn-val header-auth-val]
[header-dn-name header-dn-val header-auth-name header-auth-val]
(if (ssl-utils/valid-x500-name? header-dn-val)
{:client-cert-cn (ssl-utils/x500-name->CN header-dn-val)
:authenticated (= "SUCCESS" header-auth-val)}
(do
(if-not (nil? header-dn-val)
(log/errorf "The DN '%s' provided by the HTTP header '%s' is malformed."
(let [cn (ssl-utils/x500-name->CN header-dn-val)
authenticated (= "SUCCESS" header-auth-val)]
(log/debugf "CN '%s' provided by HTTP header '%s'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging++

cn header-dn-val)
(log/debugf (str "Verification of client '%s' provided by HTTP "
"header '%s': '%s'. Authenticated: %s.")
cn
header-auth-name
header-auth-val
authenticated)
{:client-cert-cn cn
:authenticated authenticated}))
(do
(if (nil? header-dn-val)
(log/debugf (str "No DN provided by the HTTP header '%s'. Treating "
"client as unauthenticated.") header-dn-name)
(log/errorf (str "DN '%s' provided by the HTTP header '%s' is "
"malformed. Treating client as unauthenticated.")
header-dn-val header-dn-name))
unauthenticated-client-info)))

Expand Down Expand Up @@ -173,40 +176,68 @@
cert-count
" found"))))))

(defn auth-maybe-with-client-header-info
"Return authentication info based on client headers"
[config headers]
(let [header-dn-name (:ssl-client-header config)
header-dn-val (get headers header-dn-name)
header-auth-name (:ssl-client-verify-header config)
header-auth-val (get headers header-auth-name)
header-cert-val (get headers header-client-cert-name)]
(if (:allow-header-cert-info config)
(-> (header-auth-info header-dn-name
header-dn-val
header-auth-val)
(assoc :client-cert (header-cert header-cert-val)))
(do
(doseq [[header-name header-val] {header-dn-name header-dn-val
header-auth-name header-auth-val
header-client-cert-name header-cert-val}]
(if header-val
(log/warn "The HTTP header" header-name "was specified,"
"but the master config option allow-header-cert-info"
"was either not set, or was set to false."
"This header will be ignored.")))
unauthenticated-client-info))))
(defn ssl-auth-info
"Get map of client authentication info from the supplied
`java.security.cert.X509Certificate` object. If the supplied object is nil,
the information returned would represent an 'unauthenticated' client."
[ssl-client-cert]
(if ssl-client-cert
(let [cn (ssl-utils/get-cn-from-x509-certificate ssl-client-cert)
authenticated (not (empty? cn))]
(log/debugf "CN '%s' provided by SSL certificate. Authenticated: %s."
cn authenticated)
{:client-cert-cn cn
:authenticated authenticated})
(do
(log/debugf "No SSL client certificate provided. "
"Treating client as unauthenticated.")
unauthenticated-client-info)))

(defn client-auth-info
"Get map of client authentication info for the client. Map has the following
keys:

* :client-cert - A `java.security.cert.X509Certificate` object or nil
* :client-cert-cn - The CN (Common Name) of the client, typically associated
with the CN attribute from the Distinguished Name
in an X.509 certificate's Subject.
* :authenticated - A boolean representing whether or not the client is
considered to have been successfully authenticated.

(defn auth-maybe-with-ssl-info
"Merge information from the SSL client cert into the jruby request if
available and information was not expected to be provided via client headers"
[config ssl-client-cert request]
(if (:allow-header-cert-info config)
request
(let [cn (get-cert-common-name ssl-client-cert)]
(merge request {:client-cert ssl-client-cert
:client-cert-cn cn
:authenticated (not (nil? cn))}))))
Parameters:

* config - Map of configuration data
* request - Ring request containing client data"
[config request]
(if-let [authorization (:authorization request)]
{:client-cert (ring-auth/authorized-certificate request)
:client-cert-cn (ring-auth/authorized-name request)
:authenticated (true? (ring-auth/authorized-authentic? request))}
(let [headers (:headers request)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ring actually had a get-header function which allows you to grab headers by name directly from the request object, but this is totally fine with me as it is.

header-dn-name (:ssl-client-header config)
header-dn-val (get headers header-dn-name)
header-auth-name (:ssl-client-verify-header config)
header-auth-val (get headers header-auth-name)
header-cert-val (get headers header-client-cert-name)]
(if (:allow-header-cert-info config)
(-> (header-auth-info header-dn-name
header-dn-val
header-auth-name
header-auth-val)
(assoc :client-cert (header-cert header-cert-val)))
(do
(doseq [[header-name header-val]
{header-dn-name header-dn-val
header-auth-name header-auth-val
header-client-cert-name header-cert-val}]
(if header-val
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the comprehension forms have special support for the common conditionals that follow, so you could collapse this if into the doseq like so:

(doseq [[header-name header-val] {...}
        :when header-val]
...)

Might be worthwhile in this particular function with as many nesting levels as it has.

(log/warn "The HTTP header" header-name "was specified,"
"but the master config option allow-header-cert-info"
"was either not set, or was set to false."
"This header will be ignored.")))
(let [ssl-cert (:ssl-client-cert request)]
(-> (ssl-auth-info ssl-cert)
(assoc :client-cert ssl-cert))))))))

(defn as-jruby-request
"Given a ring HTTP request, return a new map that contains all of the data
Expand All @@ -221,20 +252,16 @@
Server. If so, then the DN, authentication status, and, optionally, the
certificate will be provided by HTTP headers."
[config request]
(let [headers (:headers request)
jruby-req {:uri (:uri request)
:params (:params request)
:remote-addr (:remote-addr request)
:headers headers
:body (:body request)
:request-method (-> (:request-method request)
name
string/upper-case)}]
(merge jruby-req
(->> (auth-maybe-with-client-header-info config
headers)
(auth-maybe-with-ssl-info config
(:ssl-client-cert request))))))
(merge
{:uri (:uri request)
:params (:params request)
:remote-addr (:remote-addr request)
:headers (:headers request)
:body (:body request)
:request-method (-> (:request-method request)
name
string/upper-case)}
(client-auth-info config request)))

(defn make-request-mutable
[request]
Expand Down