Feature: Secrets API

[ram-infrac]

Is this a BUG REPORT or FEATURE REQUEST?: FEATURE

Reason needed

To create OSD-SDK Move internal PX API to OSD to create external API SDK. This is first API set we will move to OSD.

1. Manage secret commands

Secret commands allows user to manage secret keys. You can encrypt volumes using given secret key.

a. Set Cluster Secret Key;

Cluster wide secret key is basically a key value pair where the value part is the secret that is used as a passphrase for encrypting volumes. A cluster wide secret key is a common key that can be used to encrypt all volumes.

• Method: POST
• Endpoints: /setClusterSecretKey
• content-type: application/json
• Response HTTP Status code: 200
• Params: clustersecretkey(string): Cluster wide Secret Key override(bool) : override previous secret key [true/false]

Example Request: POST /v1/setClusterSecretKey

Content-type : application/json
{
    clustersecretkey: "testkey1",
    override: true
}

Example Response: Cluster Secret Key successfully set.

b. Login to Secret

Allow login to secure secret store e.g vault, kvdb,aws etc

• Method: Post
• Endpoints: /secretsLogin
• Response HTTP Status code: 200
• Params: secret(int): type of secret (kvdb - 1, vault - 2, aws - 3, Docker - 4, kubernetes -5, default - 0) secretConfig(string): config for secret login or null

Example Request:

POST /v1/secretsLogin

Content-type : application/json
{
secret:2,
secretConfig: null
}

Example Response: Secrets Login Succeeded

c. Set value for given secret key

stores the data against the given key in the secret store

• Method: PUT
• Endpoints: /setSecret
• Content-Type: application/json
• Response HTTP Status code: 200
• Params: secretid(string) : key for secret value. secretvalue(string) : value for secret passpharase.

Example Request: POST /v1/setSecret

Content-type : application/json
{
secretid : testkey1,
secretvalue :testval1
}

Example Response: Secret succuessfully set.

d. Get value of secret key

retrieves the data for the given key.

• Method: GET
• Endpoints: /getSecret
• Content-Type:application/json
• Response HTTP Status code: 200
• Params: secretid(string) : passphrase whose value to be retrived.

Example Request:

GET /v1/getSecret

Content-type : application/json
{
secretid: testkey1
}

Example Response: testval1

Interface

type Secrets interface {
    // Login create session with secret store
    SecretLogin(secretType int, secretConfig map[string]string) error
    // SetClusterSecretKey sets the cluster wide secret key
    SetClusterSecretKey(secretKey string, override bool) error
    //CheckSecretLogin validates session with secret store
    CheckSecretLogin() error
}

• api/server/secrets.go - will implement restServer interface, also will have appropiate handler function implementation for endpoints.
• openstorage/secrets/ - package will define above interfaces and implementation for same. Ques- 1 : how should we manage packages for PX API moving into osd, let's say like above should we create seperate package for each section /openstorage/schedule, openstorage/services etc? Ques- 2 : how this interface implementation will be hooked with storage vendor specific implementation?

[lpabon]

This looks ok to me. My only concern is that the API may be over a non-secure HTTP connection.

[adityadani]

Openstorage should only define the Secrets interface and provide a REST layer. The actual implementation of this interface does not need to be in openstorage. We can follow the VolumeDriver model where the actual implementation may not reside in openstorage package but the volume driver is still supported.

Ques- 1 : how should we manage packages for PX API moving into osd, let's say like above should we create seperate package for each section /openstorage/schedule, openstorage/services etc? You can keep secrets as a separate package with its own interface. But the REST server routes will have to be added in api/server/routes.go

Ques- 2 : how this interface implementation will be hooked with storage vendor specific implementation? We do not need to have a tight coupling between a storage vendor and the secrets implementation. A storage vendor can opt out of using a Secrets interface just like it can opt out of using the Cluster interface.

So here is a rough guideline what I think we should do.

openstorage/secrets/manager.go

type SecretsManager interface {
// Register registers a new secret implementation
Register(name string, secrets Secrets)
// Get returns a secret implementation
Get(name string) Secrets
}
// Actual implementation of the above API here

openstorage/secrets/api.go

type Secrets interface {
    // Login create session with secret store
    SecretLogin(secretType int, secretConfig map[string]string) error
    // SetClusterSecretKey sets the cluster wide secret key
    SetClusterSecretKey(secretKey string, override bool) error
    //CheckSecretLogin validates session with secret store
    CheckSecretLogin() error
}

// Actual implementation of above API not needed.

The REST server layer would look something like this

func setClusterSecretKey(r http.Request, w http.ResponseWriter) error {
        sname := r.GetParam("secretsImplementor")
        secretKey := r.GetParam("secretkey")
        override := r.GetParam("override")
        secretManager.Get(sname).SetClusterSecretKey(secretKey, override)
}

The REST client layer would set the above secretsImplementor param value.

This is just one way of implementing it and we can discuss other approaches here as well.

@piyush-nimbalkar What do you think?

[ram-infrac]

Thanks for feedback aditya, just one question with above implementation approach for other PX API sections lets say service PX API, do we need to create similar separate model for each of them or we can combine them in some way

[lpabon]

@adityadani , @ram-infrac , and myself met offline and came up with the following design for this (and possibly future APIs):

1. No more singletons.
2. Server would have variable holding the implementation to the interface.
3. Routes to the server would be coded to have handlers which call the interface implementation.
4. Extend the current servers to have a RegisterXXXManager, in this case we propose to extend the clusterAPI to have a RegisterSecretManager which would accept an implementation of the SecretManager interface. Any call to RegisterXXXManager would override the current handler in the server.
5. The server would have a "fake/generic" implementation on startup which will could return Not implemented or have a generic implementation. This API would have its own simple/fake implementation of the SecretManager.

This is based on the pattern used by CSI and Rico.

[piyush-nimbalkar]

Few suggestions, have commented on the PR as well, but would be good to discuss here as well -

1. Instead of calling ClusterSecretKey, we should rename it to DefaultSecretKey or ClusterWideSecretKey
2. Should we have an interface for GetClusterSecretKey/GetDefaultSecretKey? This will not give the value of the actual cluster wide secret key but the can return the default secret key which could be used to retrieve the value using GetSecret()
3. Should the SetSecret() take a map[string]interface{} to keep it generic and the GetSecret() return the same?
4. SecretLogin(secretType int, secretConfig map[string]string) error. Instead of int secretType, we should take string because it seems more readable and easy to manage.

@lpabon @adityadani @ram-infrac

[adityadani]

+1 for renaming ClusterSecretKey to DefaultSecretKey Yes we should have a GetDefaultSecretKey API.

Instead of a map[string]interface{} why not keep the value simple as an interface{} ?

Also the secretType should be taken from libopenstorage/secrets and enum string can be defined with all the supported secret types.

[piyush-nimbalkar]

Yes interface{} is better

Instead of a map[string]interface{} why not keep the value simple as an interface{}

[ram-infrac]

REST API endpoints will be called as,

GET /cluster/secret/secrets/
PUT /cluster/secret/secrets/ Body: {value: } GET /cluster/secret/defaultsecretkey PUT /cluster/secret/defaultsecretkey Body: {value: } GET /cluster/secret/secrets/verify POST /cluster/secrets/login

Should first two listen on /cluster/secret or seperate verb to be added as above?

[piyush-nimbalkar]

Discussed with @adityadani. Let's have it this way, instead of the double secret/secrets paths:

GET /cluster/secrets/<secret_id>
PUT /cluster/secrets/<secret_id> Body: {value: }
GET /cluster/secrets/defaultsecretkey
PUT /cluster/secrets/defaultsecretkey Body: {value: }
GET /cluster/secrets/verify
POST /cluster/secrets/login Body: {}