Commit Graph

692 Commits

Author SHA1 Message Date
Ryan Richard
629bf61655 Extract some trivial helpers for identical code usages 2021-06-30 15:02:14 -07:00
Monis Khan
5ff2be973c
credentialrequest: use safer approximation for ExpirationTimestamp
We want the value of time.Now() to be calculated before the call to
IssueClientCertPEM to prevent the ExpirationTimestamp from being
later than the notAfter timestamp on the issued certificate.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-23 11:07:00 -04:00
Monis Khan
d78b845575
Fix bad test package name
Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-22 11:23:19 -04:00
Matt Moyer
551249fb69
Use a custom type for our static CLI client (smaller change).
Before this change, we used the `fosite.DefaultOpenIDConnectClient{}` struct, which implements the  `fosite.Client` and `fosite.OpenIDConnectClient` interfaces. For a future change, we also need to implement some additional optional interfaces, so we can no longer use the provided default types. Instead, we now use a custom `clientregistry.Client{}` struct, which implements all the requisite interfaces and can be extended to handle the new functionality (in a future change).

There is also a new `clientregistry.StaticRegistry{}` struct, which implements the `fosite.ClientManager` and looks up our single static client. We could potentially extend this in the future with a registry backed by Kubernetes API, for example.

This should be 100% refactor, with no user-observable change.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-06-15 15:31:48 -05:00
Monis Khan
269db6b7c2
impersonator: always authorize every request
This change updates the impersonator to always authorize every
request instead of relying on the Kuberentes API server to perform
the check on the impersonated request.  This protects us from
scenarios where we fail to correctly impersonate the user due to
some bug in our proxy logic.  We still rely completely on the API
server to perform admission checks on the impersonated requests.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-14 12:53:09 -04:00
Monis Khan
addf632e7c
impersonator: add docs regarding limited serivce account
Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-11 15:37:55 -04:00
Monis Khan
898f2bf942
impersonator: run as a distinct SA with minimal permissions
This change updates the impersonation proxy code to run as a
distinct service account that only has permission to impersonate
identities.  Thus any future vulnerability that causes the
impersonation headers to be dropped will fail closed instead of
escalating to the concierge's default service account which has
significantly more permissions.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-11 12:13:53 -04:00
Monis Khan
5b327a2b37
impersonator: remove redundant deleteKnownImpersonationHeaders logic
WithImpersonation already deletes impersonation headers and has done
so since the early days:

https://github.com/kubernetes/kubernetes/pull/36769

ensureNoImpersonationHeaders will still reject any request that has
impersonation headers set so we will always fail closed.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-04 15:22:01 -04:00
Benjamin A. Petersen
492f6cfddf
impersonator: honor anonymous authentication being disabled
When anonymous authentication is disabled, the impersonation proxy
will no longer authenticate anonymous requests other than calls to
the token credential request API (this API is used to retrieve
credentials and thus must be accessed anonymously).

Signed-off-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
Signed-off-by: Monis Khan <mok@vmware.com>
2021-06-04 09:00:56 -04:00
Matt Moyer
af4cd1b515
Tolerate NotFound when deleting services in impersonatorconfig.
When a CredentialIssuer is switched from one service type to another (or switched to disabled mode), the `impersonatorconfig` controller will delete the previous Service, if any. Normally one Concierge pod will succeed to delete this initially and any other pods will see a NotFound error.

Before this change, the NotFound would bubble up and cause the strategy to enter a ErrorDuringSetup status until the next reconcile loop. We now handle this case without reporting an error.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-06-03 12:07:19 -05:00
Matt Moyer
7ee1f8c441
In LDAP, do not log username until we know the user exists.
This prevents accidentally logging a password if the user enters it into the username field by mistake.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-28 16:57:48 -05:00
Ryan Richard
cedbe82bbb Default groupSearch.attributes.groupName to "dn" instead of "cn"
- DNs are more unique than CNs, so it feels like a safer default
2021-05-28 13:27:11 -07:00
Matt Moyer
e25de9e559
Update ID token tests for latest Fosite.
The new version has different behavior for the `nonce` claim, which is now omitted if it would be empty (see https://github.com/ory/fosite/pull/570).

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-28 12:53:37 -05:00
Matt Moyer
a69fe68362
Merge branch 'main' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-27 17:11:40 -05:00
Matt Moyer
01713c7ce1
Don't reconcile Service ports in impersonatorconfig.
These are tricky because a real load balancer controller (e.g., on GKE) will overwrite and set NodePort, so we can't blindly set the desired state of this fields.

For now, we will just skip reconciling these. In the future, we could be more clever about merging them together with the current state.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-27 17:10:25 -05:00
Matt Moyer
ab750f48aa
When merging CredentialIssuer updates, don't overwrite LastUpdated.
If the only thing that has changed about a strategy is the LastUpdated timestamp, then we should not update the object.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-27 17:09:12 -05:00
Ryan Richard
d2251d2ea7 Use base64 binary-encoded value as UID for LDAP
This is to allow the use of binary LDAP entry attributes as the UID.
For example, a user might like to configure AD’s objectGUID or maybe
objectSid attributes as the UID attribute.

This negatively impacts the readability of the UID when it did not come
from a binary value, but we're considering this an okay trade-off to
keep things simple for now. In the future, we may offer more
customizable encoding options for binary attributes.

These UIDs are currently only used in the downstream OIDC `sub` claim.
They do not effect the user's identity on the Kubernetes cluster,
which is only based on their mapped username and group memberships from
the upstream identity provider. We are not currently supporting any
special encoding for those username and group name LDAP attributes, so
their values in the LDAP entry must be ASCII or UTF-8 in order for them
to be interpreted correctly.
2021-05-27 13:47:10 -07:00
Matt Moyer
af2af567be
Merge branch 'main' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-27 15:13:36 -05:00
Ryan Richard
81148866e0 URL query escape the upstream OIDC subject in the downstream subject URL 2021-05-27 09:25:48 -07:00
Matt Moyer
349d3dad83
Make temporary errors return Pending in impersonatorconfig.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-27 11:13:10 -05:00
Ryan Richard
033e1f0399 Add user search base to downstream subject for upstream LDAP
- Also add some tests about UTF-8 characters in LDAP attributes
2021-05-26 17:04:20 -07:00
Matt Moyer
0a47aa4843
Adjust log levels in impersonatorconfig controller.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-26 16:47:02 -05:00
Matt Moyer
d780bf64bc
Remove references to impersonationConfigMap.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-26 15:24:59 -05:00
Matt Moyer
b57878ebc5
Remove TODO from impersonator.go.
We're now tracking this in an issue: https://github.com/vmware-tanzu/pinniped/issues/642

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 15:08:29 -05:00
Matt Moyer
1932b03c39
Refactor createOrUpdateService() method.
This updates the code to use a different mechanism for driving desired state:

- Read existing object
- If it does not exist, create desired object
- If it does exist, make a copy and set all the desired fields
- Do a deepequal to see if an update is necessary.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 15:03:04 -05:00
Matt Moyer
be8118ec2e
Re-enable parallelism on TestImpersonatorConfigControllerSync.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:57:51 -05:00
Matt Moyer
1a4687a40a
Switch impersonatorconfig to all singleton queues.
We also no longer need an initial event, since we don't do anything unless the CredentialIssuer exists, so we'll always be triggered at the appropriate time.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:54:40 -05:00
Matt Moyer
b13c494f93
Migrate off global logger in impersonatorconfig.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:44:05 -05:00
Margo Crawford
e5a61f3b95 IPv6 address in unit tests for ClusterIPs 2021-05-26 10:30:33 -07:00
Margo Crawford
f2021f1b53 Merge branch 'credentialissuer-spec-api' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-25 17:06:26 -07:00
Margo Crawford
e2fad6932f multiple cluster ips 2021-05-25 17:01:42 -07:00
Matt Moyer
450ce6a4aa
Switch impersonatorconfig to new endpointaddr package.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 17:44:25 -05:00
Matt Moyer
c970dd1fb0
Merge branch 'main' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-25 17:32:58 -05:00
Matt Moyer
89eff28549
Convert LDAP code to use endpointaddr package.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 16:17:27 -05:00
Matt Moyer
d9a3992b3b
Add endpointaddr pkg for parsing host+port inputs.
This type of field appears in more than one of our APIs, so this package will provide a single source of truth for validating and parsing inputs.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 16:17:26 -05:00
Ryan Richard
2014f4623d Move require.NoError() to t.Cleanup() 2021-05-24 14:24:09 -07:00
Matt Moyer
fabc08b01b
Merge branch 'main' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-24 15:49:13 -05:00
Margo Crawford
5de9bac4ac Oof... good I wrote an integration test because that's not how updating works!
Now updating the existing service in kubernetes but with the new
annotations
2021-05-24 09:41:49 -07:00
Margo Crawford
150e879a68 Add tests for deleting services 2021-05-21 13:47:06 -07:00
Ryan Richard
b16e84d90a Add another unit test for the LDAP client code 2021-05-21 12:44:01 -07:00
Margo Crawford
b4bb0db6e5 Refactor some shared code between load balancer and cluster ip creation 2021-05-21 09:57:46 -07:00
Margo Crawford
4606f1d8bd More error handling for cluster ip 2021-05-20 16:21:10 -07:00
Margo Crawford
599d70d6dc Wire generatedClusterIPServiceName through from NamesConfig 2021-05-20 14:11:35 -07:00
Ryan Richard
901ddd1870 Merge branch 'main' into ldap_starttls 2021-05-20 13:40:56 -07:00
Ryan Richard
8b549f66d4 Add integration test for LDAP StartTLS 2021-05-20 13:39:48 -07:00
Ryan Richard
7e76b66639 LDAP upstream watcher controller tries using both TLS and StartTLS
- Automatically try to fall back to using StartTLS when using TLS
  doesn't work. Only complain when both don't work.
- Remember (in-memory) which one worked and keeping using that one
  in the future (unless the pod restarts).
2021-05-20 12:46:33 -07:00
Ryan Richard
fff90ed2ca
Merge branch 'main' into initial_ldap_group_support 2021-05-20 12:36:04 -07:00
Margo Crawford
62651eddb0 Took care of some impersonation cluster ip related todos 2021-05-20 11:57:07 -07:00
Matt Moyer
ec25259901
Update impersonatorconfig controller to use new CredentialIssuer update helper.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-20 12:26:07 -05:00
Matt Moyer
e4dd83887a
Merge remote-tracking branch 'origin/main' into credentialissuer-spec-api 2021-05-20 10:53:53 -05:00