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

Kerberos/GSSAPI backend feature request #3005

Closed
wants to merge 2 commits into from
Closed

Kerberos/GSSAPI backend feature request #3005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14f9b7e
initial GSSAPI feature request
nrhall Jul 13, 2017
98b3e35
remove SRV record patch
nrhall Jul 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions api/logical.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,25 +79,27 @@ func (c *Logical) List(path string) (*Secret, error) {
return ParseSecret(resp.Body)
}

func (c *Logical) Write(path string, data map[string]interface{}) (*Secret, error) {
func (c *Logical) WriteRaw(path string, data map[string]interface{}) (*Response, *Secret, error) {
r := c.c.NewRequest("PUT", "/v1/"+path)
if err := r.SetJSONBody(data); err != nil {
return nil, err
return nil, nil, err
}

var secret *Secret = nil
resp, err := c.c.RawRequest(r)
if resp != nil {
defer resp.Body.Close()
}
if err != nil {
return nil, err
if err == nil && resp.StatusCode == 200 {
secret, err = ParseSecret(resp.Body)
}
}

if resp.StatusCode == 200 {
return ParseSecret(resp.Body)
}
return resp, secret, err
}

return nil, nil
func (c *Logical) Write(path string, data map[string]interface{}) (*Secret, error) {
_, secret, err := c.WriteRaw(path, data)
return secret, err
}

func (c *Logical) Delete(path string) (*Secret, error) {
Expand Down
3 changes: 3 additions & 0 deletions cli/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
credAws "github.com/hashicorp/vault/builtin/credential/aws"
credCert "github.com/hashicorp/vault/builtin/credential/cert"
credGitHub "github.com/hashicorp/vault/builtin/credential/github"
credKerberos "github.com/hashicorp/vault/builtin/credential/kerberos"
credLdap "github.com/hashicorp/vault/builtin/credential/ldap"
credOkta "github.com/hashicorp/vault/builtin/credential/okta"
credRadius "github.com/hashicorp/vault/builtin/credential/radius"
Expand Down Expand Up @@ -79,6 +80,7 @@ func Commands(metaPtr *meta.Meta) map[string]cli.CommandFactory {
"ldap": credLdap.Factory,
"okta": credOkta.Factory,
"radius": credRadius.Factory,
"kerberos": credKerberos.Factory,
},
LogicalBackends: map[string]logical.Factory{
"aws": aws.Factory,
Expand Down Expand Up @@ -123,6 +125,7 @@ func Commands(metaPtr *meta.Meta) map[string]cli.CommandFactory {
"cert": &credCert.CLIHandler{},
"aws": &credAws.CLIHandler{},
"radius": &credUserpass.CLIHandler{DefaultMount: "radius"},
"kerberos": &credKerberos.CLIHandler{DefaultMount: "kerberos"},
},
}, nil
},
Expand Down
20 changes: 20 additions & 0 deletions http/logical.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,26 @@ func respondRaw(w http.ResponseWriter, r *http.Request, resp *logical.Response)
}
}

// get any returned headers
headersRaw, ok := resp.Data[logical.HTTPHeaders]
if !ok {
retErr(w, "no headers given")
return
} else {
headers, ok := headersRaw.(map[string]string)
if !ok {
retErr(w, "returned headers are not a map")
return
} else {
for header, value := range headers {
w.Header().Set(header, value)
}

// some headers set now, so response is no longer empty
nonEmpty = false
}
}

if nonEmpty {
// Get the body
bodyRaw, ok := resp.Data[logical.HTTPRawBody]
Expand Down
5 changes: 5 additions & 0 deletions logical/response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ const (
// This can only be specified for non-secrets, and should should be similarly
// avoided like the HTTPContentType. The value must be an integer.
HTTPStatusCode = "http_status_code"

// HTTPHeaders is a mechanism to support returning HTTP headers with the response.
// This can only be specified for non-secrets, and should should be similarly
// avoided like the HTTPContentType. The value must be a map of string to string.
HTTPHeaders = "http_headers"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't the right mechanism for this. If we want to allow arbitrary headers to be returned in a response, it should be added as an object to logical.Response, with appropriate rules around HMACing in auditing and so on.

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough - I saw the existing cases where this information was being passed around. The rest seem to have been added for the PKI backend. I think the cleanest way to do this might be to expose some limited information about the HTTP request/response in logical.Request and logical.Response, rather than just the headers (then all these other 'should be avoided unless absolutely necessary' comments can be tidied up...)

WRT auditing, I think that there's a case to be made for not logging SPNEGO tokens, or other forms of authorization header (or at least the secret part of them) but instead perhaps replacing that information with some audit logging around which user is making the request.

Perhaps the headers in need of hashing should be specified by the backend when making the request for the headers that it needs to be passed? (see my other comment below).

Copy link
Member

Choose a reason for hiding this comment

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

Similarly to https://www.vaultproject.io/api/system/config-auditing.html there should be a way to specify which response headers created by a backend should be logged, and if so, hashed or not.

)

// Response is a struct that stores the response of a request.
Expand Down
10 changes: 8 additions & 2 deletions vault/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,15 @@ func (r *Router) routeCommon(req *logical.Request, existenceCheck bool) (*logica
originalClientTokenRemainingUses := req.ClientTokenRemainingUses
req.ClientTokenRemainingUses = 0

// Cache the headers and hide them from backends
// Cache the headers and hide all but a small, standard set of them from backends
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to add support for sending headers to backends, we need a mechanism, like with auditing, for the administrator to control which headers are allowed to be sent.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure an administrator view is that useful. The headers (and indeed the body) which backends need (this being one of two that need access to the underlying request) is really a function of the backend, rather than the administrator - e.g. it wouldn't seem sensible to allow an administrator to disable a header required by a backend.

So perhaps the list of headers (and whether their values should be hashed in the audit logs) should be requested by the backend at mount time?

Copy link
Member

Choose a reason for hiding this comment

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

An administrator needs to have positive control over which headers a backend sees. Given that backend plugins will be available shortly, not all backends will have been reviewed by the Vault team to make sure they're doing reasonable things.

headers := req.Headers
req.Headers = nil
for header := range headers {
switch header {
case "Authorization":
default:
delete(req.Headers, header)
}
}

// Cache the wrap info of the request
var wrapInfo *logical.RequestWrapInfo
Expand Down
6 changes: 6 additions & 0 deletions vendor/vendor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@
"revision": "85b1699d505667d13f8ac4478c1debbf85d6c5de",
"revisionTime": "2017-06-08T22:14:41Z"
},
{
"checksumSHA1": "m1aIIVtZXprQLTHexJ7e0fBZ/fc=",
"path": "github.com/apcera/gssapi",
"revision": "5fb4217df13b8e6878046fe1e5c10e560e1b86dc",
"revisionTime": "2016-10-10T21:59:02Z"
},
{
"checksumSHA1": "M+ZeYktTT2wak9ZvQ0OZBbIHAGo=",
"path": "github.com/armon/go-metrics",
Expand Down