-
Notifications
You must be signed in to change notification settings - Fork 38
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.
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
Issue 479: Addressing Security vulnerabilities with authentication #488
Changes from 7 commits
7d5e702
4755baa
df6da84
aed02aa
7725877
81906e5
fe86d8d
4333ee9
f72f419
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,3 +76,83 @@ where username and password are credentials you intend to use. | |
Note that Pravega operator uses `/etc/auth-passwd-volume` as the mounting directory for secrets. | ||
|
||
For more security configurations, please check [here](https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md). | ||
|
||
Pravega Operator Supports Passing of Auth Parametes as Secret which are mounted as file in both Segment Store and Controller (Operator mounts these secrets in Segementstore pod it's mounted at `/etc/ss-auth-volume` and in Controller pod at `/etc/controller-auth-volume` respectively) | ||
|
||
Note that Pravega has to use this feature and start Picking below specified values from file insted of jvm properties which it currently does. | ||
(This is not implemented at pravega end currently) | ||
|
||
Below is how we can create secret and expose them as file for Auth related properties:- | ||
|
||
1. Create a File containg `controller.security.auth.delegationToken.signingKey.basis` as `delegationToken.signingKey.basis` which represent the tokensigning key used to connect to the controller: | ||
|
||
Sample encrypted password file: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 89 was maybe a left over from a copy paste from somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have removed it now. |
||
``` | ||
$ cat controllerauthdata.txt | ||
delegationToken.signingKey.basis: "secret" | ||
``` | ||
|
||
2. Create a kubernetes secret with this file: | ||
|
||
``` | ||
$ kubectl create secret generic controllertokensecret \ | ||
--from-file=./controllerauthdata.txt \ | ||
``` | ||
|
||
Ensure Secret is created:- | ||
|
||
``` | ||
$ kubectl describe secret controllertokensecret | ||
Name: controllertokensecret | ||
Namespace: default | ||
Labels: <none> | ||
Annotations: <none> | ||
|
||
Type: Opaque | ||
|
||
Data | ||
==== | ||
controllerauthdata.txt: 67 bytes | ||
|
||
``` | ||
|
||
3. Create a File containg `autoScale.security.auth.token.signingKey.basis` as `delegationToken.signingKey.basis` which represent the tokensigning key used to connect to the Segmentstore along with other 3 values `pravega.client.auth.method`, `pravega.client.auth.token` and `controller.connect.auth.credentials.dynamic` as `controller.connect.auth.params` which contains all the 3 values in the same order seprated by semicolon: | ||
|
||
``` | ||
$ cat segmentstoreauthdata.txt | ||
delegationToken.signingKey.basis: "secret" | ||
controller.connect.auth.params: {method};{token};{dynamic} | ||
``` | ||
4. Create a kubernetes secret with this file: | ||
|
||
``` | ||
$ kubectl create secret generic sstokensecret \ | ||
--from-file=./segmentstoreauthdata.txt \ | ||
``` | ||
Ensure Secret is created:- | ||
|
||
``` | ||
$ kubectl describe secret sstokensecret | ||
Name: sstokensecret | ||
Namespace: default | ||
Labels: <none> | ||
Annotations: <none> | ||
|
||
Type: Opaque | ||
|
||
Data | ||
==== | ||
segmentstoreauthdata.txt: 106 bytes | ||
|
||
``` | ||
|
||
5. Use these secrets instead of specifying the values in the option:- | ||
|
||
``` | ||
spec: | ||
authentication: | ||
enabled: true | ||
passwordAuthSecret: password-auth | ||
segmentStoreTokenSecret: sstokensecret | ||
controllerTokenSecret: controllertokensecret | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,6 +290,12 @@ type AuthenticationParameters struct { | |
// name of Secret containing Password based Authentication Parameters like username, password and acl | ||
// optional - used only by PasswordAuthHandler for authentication | ||
PasswordAuthSecret string `json:"passwordAuthSecret,omitempty"` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For integration purposes, there needs to be a clear description of how to create the two secrets. It won't be obvious to the common dweller though. :) Having clear couple kubectl commands in the README that show exactly how to generate these 2 secrets is crucial so integrators know how to leverage this welcome enhancement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @sarlaccpit, I have added documentation in the auth section of our documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, the auth.md is very useful and detailed. It will be crucial information. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @sarlaccpit . |
||
//name of secret containg TokenSigningKey | ||
ControllerTokenSecret string `json:"controllerTokenSecret,omitempty"` | ||
|
||
//name of secret containg TokenSigningKey and AuthToken | ||
SegmentStoreTokenSecret string `json:"segmentStoreTokenSecret,omitempty"` | ||
} | ||
|
||
func (ap *AuthenticationParameters) IsEnabled() bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,8 +103,9 @@ var _ = Describe("Controller", func() { | |
}, | ||
}, | ||
Authentication: &v1beta1.AuthenticationParameters{ | ||
Enabled: true, | ||
PasswordAuthSecret: "authentication-secret", | ||
Enabled: true, | ||
PasswordAuthSecret: "authentication-secret", | ||
ControllerTokenSecret: "controllerauthsecret", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency, maybe controllerauthsecret --> controllerauth-secret ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have changed it. |
||
}, | ||
} | ||
p.WithDefaults() | ||
|
@@ -219,8 +220,9 @@ var _ = Describe("Controller", func() { | |
}, | ||
}, | ||
Authentication: &v1beta1.AuthenticationParameters{ | ||
Enabled: true, | ||
PasswordAuthSecret: "authentication-secret", | ||
Enabled: true, | ||
PasswordAuthSecret: "authentication-secret", | ||
ControllerTokenSecret: "controllerauthsecret", | ||
Prabhaker24 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
} | ||
p.WithDefaults() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit but I'm not getting what the "basis" suffix means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pravega uses this suffix so I tried keeping it the same. In any case this is just a key in a secret so if pravega changes the name for this parameter we can always have the name of the key different.