search

cyberark / secretless-broker

Secure your apps by making them Secretless
Apache License 2.0
234 stars 42 forks source link

MSSQL has undergone appropriate XA #977

Closed izgeri closed 4 years ago

izgeri commented 4 years ago

To complete XA, someone should follow the MSSQL docs [link TBD] to try to deploy an app in K8s/OC that connects to MSSQL via Secretless.

An XA environment will be available with:

izgeri commented 4 years ago

Experience Assurance Plan

The infra team will deploy for you:

You will be provided with:

You will follow the documentation to deploy the pet store app with Secretless so that the pet store connects to its backend MSSQL DB via Secretless (which is running as a separate container in the same pod).

izgeri commented 4 years ago
sgnn7 commented 4 years ago

Issue 1

Screen Shot 2020-01-22 at 10.52.22.png

~This needs to be much higher in the document and have a step-by-step info on how to create a service account. By the time the user reaches this point we're too far ahead operationally.~

Edit: this seems to be covered in the application manifest so it can be ignored

Issue 2

Screen Shot 2020-01-22 at 10.54.29.png

This needs to be reworded. When we add the Secretless Broker container to our application manifest, we must define the container with name secretless as well. needs to be When we add Secretless Broker container to our application manifest, it must match this container name ("secretless") otherwise the broker will not be able to authenticate itself against Conjur.

sgnn7 commented 4 years ago

Issue 3

Role binding snippet doesn't work on OC OOTB due to errors:

Error: error validating "role-binding.yml": error validating data: [ValidationError(RoleBinding): missing required field "userNames" in com.github.openshift.api.authorization.v1.RoleBinding, ValidationError(RoleBinding): missing required field "groupNames" in com.github.openshift.api.authorization.v1.RoleBinding]; if you choose to ignore these errors, turn validation off with --validate=false

It needs to change to:

---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: test-app-conjur-authenticator-role-binding-DAP_FOLLOWER_NAMESPACE
  namespace: APP_NAMESPACE
subjects:
  - kind: ServiceAccount
    name: conjur-cluster
    namespace: DAP_FOLLOWER_NAMESPACE
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: conjur-authenticator-DAP_FOLLOWER_NAMESPACE

We may need to ensure this change works on vanilla K8s too though

sgnn7 commented 4 years ago

Issue #4

In "Required Information" section, Follower URL description isn't clear enough that it should be the internal endpoint in most cases. It should be changed to:

The URL of the DAP follower service API endpoint reachable from within the cluster.
For example, in most Kubernetes clusters, this endpoint should follow a similar
pattern to this:
`https://[DAP Follower Service Name].DAP_FOLLOWER_NAMESPACE.svc.cluster.local/api`.
sgnn7 commented 4 years ago

Issue 5

Screen Shot 2020-01-22 at 14.20.54.png

$DAP_FOLLOWER_URL is incorrect. The value should be $DAP_FOLLOWER_HOST:443. This may need to be explained as it would be a new value to derive from the required information.

sgnn7 commented 4 years ago

Issue 5b

Same snippet as above: Screen Shot 2020-01-22 at 14.20.54.png

This command is ambiguous and lacks info about how to connect to the follower. This can be fixed by running a one-time pod in the namespace:

kubectl run --rm -i --tty \
  --generator=run-pod/v1 openssl \
  --image=frapsoft/openssl \
  -- s_client -showcerts -connect $DAP_FOLLOWER_HOST:443 </dev/null | sed -n '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > conjur.pem

Issue 5c

Errors in fetching the cert from these commands are not indicated on the CLI. Some verification of the cert should be done that ensures that we don't have a bad one maybe similar to this:

(openssl x509 -noout -checkend 0 -in conjur.pem && echo "Certifiate is valid!") || \
  echo "Certificate is not valid!"
sgnn7 commented 4 years ago

Issue 6

Some yaml snippets on https://docs.secretless.io/Latest/en/Content/Get%20Started/using-dap.htm have erroneously a newline before the --- line.

Correct:

---
<some text>

Incorrect:



---
<some text>
sgnn7 commented 4 years ago

Issue #7

Screen Shot 2020-01-22 at 15.16.27.png

apiVersion: apps/v1beta1 here near the bottom needs to be changed to apiVersion: apps/v1 per https://kubernetes.io/blog/2019/09/18/kubernetes-1-16-release-announcement/#significant-changes-to-the-kubernetes-api

izgeri commented 4 years ago

re: 3 - this was surprising to me, I have recently used this role-binding.yml in OC 3.9 which worked

---
kind: RoleBinding
apiVersion: v1
metadata:
  name: test-app-conjur-authenticator-role-binding-{FOLLOWER_NS}
  namespace: {APP_NS}
subjects:
  - kind: ServiceAccount
    name: conjur-cluster
    namespace: {FOLLOWER_NS}
roleRef:
  kind: ClusterRole
  name: conjur-authenticator-{FOLLOWER_NS}
izgeri commented 4 years ago

re: 5 - I would rather like to delegate the nuance of this discussion to the DAP / Conjur docs - it starts to add too many if/else statements to the secretless instructions which should be as simple as can be.

sgnn7 commented 4 years ago

@izgeri re: 3 - this was surprising to me

The version I added works for both kubectl and oc CLI against both k8s and oc clusters. I think the original version only works with oc tool against an oc cluster .

sgnn7 commented 4 years ago

Issue 8

Screen Shot 2020-01-22 at 16.19.42.png

~The AUTHENTICATOR_NAME in the CONJUR_AUTHN_URL must be URL encoded. needs to be The AUTHENTICATOR_NAME and the APP_SERVICE_ACCOUNT in must be URL encoded.~

Edit: Not valid

Issue 9

Screen Shot 2020-01-22 at 16.27.06.png

This part should also specify that it might be !group prefix instead of !layer

sgnn7 commented 4 years ago

Hit issues with broker - it does not seem to be able to proxy data for the xa server:

Start the broker with:

$ go run cmd/secretless-broker/main.go -f /path/to/secretless.yml -debug

In another terminal:

$ sqlcmd -S <REDACTED>.rds.amazonaws.com -U <REDACTED> -P <REDACTED> -d secretless -Q "select * from pet;"
<returns correct data>

$ sqlcmd -S localhost -d secretless -Q "select * from pet;"
Sqlcmd: Error: Microsoft ODBC Driver 17 for SQL Server : Login timeout expired.
Sqlcmd: Error: Microsoft ODBC Driver 17 for SQL Server : TCP Provider: Timeout error [258]. .
Sqlcmd: Error: Microsoft ODBC Driver 17 for SQL Server : Unable to complete login process due to delay in prelogin response.

Secretless broker log:

2020/01/23 11:40:42 Secretless v1.4.2-dev starting up...
2020/01/23 11:40:42 Initializing health check on :5335...
2020/01/23 11:40:42 Initialization of health check done. You can access the endpoint at `/live` and `/ready`.
2020/01/23 11:40:42 [WARN]  Plugin directory '/usr/local/lib/secretless' not found. Ignoring external plugins...
2020/01/23 11:40:42 Trying to load configuration file: /Users/srdjan/checkout/tmp/broker-mssql/secretless.yml
2020/01/23 11:40:42 [INFO]  Waiting for new configuration...
2020/01/23 11:40:42 [DEBUG] Got new configuration
2020/01/23 11:40:42 [INFO]  Validating config against available plugins: ssh,ssh-agent,pg,mysql,mssql,conjur,generic_http,aws,basic_auth
2020/01/23 11:40:42 Registering reload signal listeners...
2020/01/23 11:40:42 [WARN]  Starting TCP listener on 0.0.0.0:1433...
2020/01/23 11:40:42 [INFO]  mssql: Starting service
2020/01/23 11:40:42 [INFO]  Waiting for new configuration...
2020/01/23 11:40:45 Instantiating provider 'literal'
2020/01/23 11:40:45 [INFO]  mssql: New connection on 192.168.4.123:1433.
2020/01/23 11:41:00 [ERROR] mssql: Failed on handle connection: failed on connect: Unable to open tcp connection with host '<REDACTED>.rds.amazonaws.com:1443': dial tcp <REDACTED>:1443: i/o timeout
sgnn7 commented 4 years ago

Issue 10

App's connection string to secretless may need to be forced to use something other than localhost (eg. 127.0.0.1. This may not have any impacts directly on the docs but to testing and development in envs like K8s/OC.

Issue 11

Username and password must be set in the client to dummy values when connecting to the broker (they must not be omitted) otherwise the client may attempt to use local user auth instead of username/password authentication and fail during this step.

sgnn7 commented 4 years ago

More details about broker breaking pet store communication with the database:

CC: @doodlesbykumbi

doodlesbykumbi commented 4 years ago

@sgnn7 The issues you're facing can be attributed to an incomplete translation layer in Secretless (I think).

The translation layers in question are those for:

  1. sending the login request from the client to the server (https://github.com/cyberark/secretless-broker/issues/1107)
  2. sending the login response from the server to the client (https://github.com/cyberark/secretless-broker/issues/1106)

I've created a draft PR to address these issues https://github.com/cyberark/secretless-broker/pull/1105

izgeri commented 4 years ago

This card remains open just so that we can log any remaining doc issues that we'd recommend to improve the documentation after running through XA - as soon as a doc issue has been created, this issue will be closed.

izgeri commented 4 years ago

follow-up docs issues have been logged in cyberark/secretless-docs#236