From 25568626f79a83211558439b536cc7bd3097725b Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Wed, 29 Jan 2020 17:30:38 -0500 Subject: [PATCH] Fix a couple bugs regarding intentions with namespaces (#7169) --- agent/connect/uri_service_oss.go | 13 ++++ agent/intentions_endpoint.go | 3 - agent/xds/server.go | 5 +- command/intention/create/create.go | 38 +++++++++++- command/intention/finder/finder.go | 2 +- test/integration/connect/envoy/helpers.bash | 69 +++++++++++++++++++++ 6 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 agent/connect/uri_service_oss.go diff --git a/agent/connect/uri_service_oss.go b/agent/connect/uri_service_oss.go new file mode 100644 index 000000000..c76fea239 --- /dev/null +++ b/agent/connect/uri_service_oss.go @@ -0,0 +1,13 @@ +// +build !consulent + +package connect + +import ( + "github.com/hashicorp/consul/agent/structs" +) + +// GetEnterpriseMeta will synthesize an EnterpriseMeta struct from the SpiffeIDService. +// in OSS this just returns an empty (but never nil) struct pointer +func (id *SpiffeIDService) GetEnterpriseMeta() *structs.EnterpriseMeta { + return &structs.EnterpriseMeta{} +} diff --git a/agent/intentions_endpoint.go b/agent/intentions_endpoint.go index 52f5c0e6b..d78b31c8e 100644 --- a/agent/intentions_endpoint.go +++ b/agent/intentions_endpoint.go @@ -288,9 +288,6 @@ func parseIntentionMatchEntry(input string) (structs.IntentionMatchEntry, error) var result structs.IntentionMatchEntry result.Namespace = structs.IntentionDefaultNamespace - // TODO(mitchellh): when namespaces are introduced, set the default - // namespace to be the namespace of the requestor. - // Get the index to the '/'. If it doesn't exist, we have just a name // so just set that and return. idx := strings.IndexByte(input, '/') diff --git a/agent/xds/server.go b/agent/xds/server.go index f4acb1eb4..f15152f8d 100644 --- a/agent/xds/server.go +++ b/agent/xds/server.go @@ -505,8 +505,9 @@ func (s *Server) Check(ctx context.Context, r *envoyauthz.CheckRequest) (*envoya // Create an authz request req := &structs.ConnectAuthorizeRequest{ - Target: destID.Service, - ClientCertURI: r.Attributes.Source.Principal, + Target: destID.Service, + EnterpriseMeta: *destID.GetEnterpriseMeta(), + ClientCertURI: r.Attributes.Source.Principal, // TODO(banks): need Envoy to support sending cert serial/hash to enforce // revocation later. } diff --git a/command/intention/create/create.go b/command/intention/create/create.go index 834e2125e..40ccae050 100644 --- a/command/intention/create/create.go +++ b/command/intention/create/create.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" @@ -135,6 +136,27 @@ func (c *cmd) Run(args []string) int { return 0 } +// parseIntentionTarget parses a target of the form / and returns +// the two distinct parts. In some cases the namespace may be elided and this function +// will return the empty string for the namespace then. +func parseIntentionTarget(input string) (name string, namespace string, err error) { + // Get the index to the '/'. If it doesn't exist, we have just a name + // so just set that and return. + idx := strings.IndexByte(input, '/') + if idx == -1 { + // let the agent do token based defaulting of the namespace + return input, "", nil + } + + namespace = input[:idx] + name = input[idx+1:] + if strings.IndexByte(name, '/') != -1 { + return "", "", fmt.Errorf("target can contain at most one '/'") + } + + return name, namespace, nil +} + // ixnsFromArgs returns the set of intentions to create based on the arguments // given and the flags set. This will call ixnsFromFiles if the -file flag // was set. @@ -149,9 +171,21 @@ func (c *cmd) ixnsFromArgs(args []string) ([]*api.Intention, error) { return nil, fmt.Errorf("Must specify two arguments: source and destination") } + srcName, srcNamespace, err := parseIntentionTarget(args[0]) + if err != nil { + return nil, fmt.Errorf("Invalid intention source: %v", err) + } + + dstName, dstNamespace, err := parseIntentionTarget(args[1]) + if err != nil { + return nil, fmt.Errorf("Invalid intention destination: %v", err) + } + return []*api.Intention{&api.Intention{ - SourceName: args[0], - DestinationName: args[1], + SourceNS: srcNamespace, + SourceName: srcName, + DestinationNS: dstNamespace, + DestinationName: dstName, SourceType: api.IntentionSourceConsul, Action: c.ixnAction(), Meta: c.flagMeta, diff --git a/command/intention/finder/finder.go b/command/intention/finder/finder.go index 7788b241c..099a2c530 100644 --- a/command/intention/finder/finder.go +++ b/command/intention/finder/finder.go @@ -84,7 +84,7 @@ func (f *Finder) Find(src, dst string) (*api.Intention, error) { func StripDefaultNS(v string) string { if idx := strings.IndexByte(v, '/'); idx > 0 { if v[:idx] == api.IntentionDefaultNamespace { - return v[:idx+1] + return v[idx+1:] } } diff --git a/test/integration/connect/envoy/helpers.bash b/test/integration/connect/envoy/helpers.bash index 003c32797..ef60136f5 100755 --- a/test/integration/connect/envoy/helpers.bash +++ b/test/integration/connect/envoy/helpers.bash @@ -351,6 +351,27 @@ function assert_service_has_healthy_instances { [ "$status" -eq 0 ] } +function check_intention { + local SOURCE=$1 + local DESTINATION=$2 + + curl -s -f "localhost:8500/v1/connect/intentions/check?source=${SOURCE}&destination=${DESTINATION}" | jq ".Allowed" +} + +function assert_intention_allowed { + local SOURCE=$1 + local DESTINATION=$2 + + [ "$(check_intention "${SOURCE}" "${DESTINATION}")" == "true" ] +} + +function assert_intention_denied { + local SOURCE=$1 + local DESTINATION=$2 + + [ "$(check_intention "${SOURCE}" "${DESTINATION}")" == "false" ] +} + function docker_consul { local DC=$1 shift 1 @@ -506,6 +527,54 @@ function delete_config_entry { retry_default curl -sL -XDELETE "http://127.0.0.1:8500/v1/config/${KIND}/${NAME}" } +function list_intentions { + curl -s -f "http://localhost:8500/v1/connect/intentions" +} + +function get_intention_target_name { + awk -F / '{ if ( NF == 1 ) { print $0 } else { print $2 }}' +} + +function get_intention_target_namespace { + awk -F / '{ if ( NF != 1 ) { print $1 } }' +} + +function get_intention_by_targets { + local SOURCE=$1 + local DESTINATION=$2 + + local SOURCE_NS=$(get_intention_target_namespace <<< "${SOURCE}") + local SOURCE_NAME=$(get_intention_target_name <<< "${SOURCE}") + local DESTINATION_NS=$(get_intention_target_namespace <<< "${DESTINATION}") + local DESTINATION_NAME=$(get_intention_target_name <<< "${DESTINATION}") + + existing=$(list_intentions | jq ".[] | select(.SourceNS == \"$SOURCE_NS\" and .SourceName == \"$SOURCE_NAME\" and .DestinationNS == \"$DESTINATION_NS\" and .DestinationName == \"$DESTINATION_NAME\")") + if test -z "$existing" + then + return 1 + fi + echo "$existing" + return 0 +} + +function update_intention { + local SOURCE=$1 + local DESTINATION=$2 + local ACTION=$3 + + intention=$(get_intention_by_targets "${SOURCE}" "${DESTINATION}") + if test $? -ne 0 + then + return 1 + fi + + id=$(jq -r .ID <<< "${intention}") + updated=$(jq ".Action = \"$ACTION\"" <<< "${intention}") + + curl -s -X PUT "http://localhost:8500/v1/connect/intentions/${id}" -d "${updated}" + return $? +} + function wait_for_agent_service_register { local SERVICE_ID=$1 local DC=${2:-primary}