Upstream
/
harbor
mirror of https://github.com/goharbor/harbor.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

update robot secret (#13654) 

* update robot secret

1, use SHA256 to generate and validate robot secret instread of symmetric encryption.
2, update the patch input object

Signed-off-by: Wang Yan <wangyan@vmware.com>

* update robot secret

1, use SHA256 to generate and validate robot secret instread of symmetric encryption.
2, update the patch input object

Signed-off-by: Wang Yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2020-12-03 18:13:06 +08:00 committed by GitHub
parent 878ef7c205
commit d2fa2e6b84
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 88 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/v2.0/swagger.yaml
View File

@ -1715,12 +1715,12 @@ paths:
parameters:
- $ref: '#/parameters/requestId'
- $ref: '#/parameters/robotId'
- name: robot
- name: robotSec
description: The JSON object of a robot account.
in: body
required: true
schema:
$ref: '#/definitions/Robot'
$ref: '#/definitions/RobotSec'
responses:
'200':
description: Return refreshed robot sec.

1
make/migrations/postgresql/0050_2.2.0_schema.up.sql
View File

@ -14,6 +14,7 @@ DELETE FROM quota_usage WHERE reference='project' AND reference_id::integer NOT
ALTER TABLE schedule ADD COLUMN IF NOT EXISTS cron_type varchar(64);
ALTER TABLE robot ADD COLUMN IF NOT EXISTS secret varchar(2048);
ALTER TABLE robot ADD COLUMN IF NOT EXISTS salt varchar(64);
ALTER TABLE task ADD COLUMN IF NOT EXISTS vendor_type varchar(16);
UPDATE task SET vendor_type = execution.vendor_type FROM execution WHERE task.execution_id = execution.id;

44
src/controller/robot/controller.go
View File

@ -32,13 +32,13 @@ type Controller interface {
Count(ctx context.Context, query *q.Query) (total int64, err error)
// Create ...
Create(ctx context.Context, r *Robot) (int64, error)
Create(ctx context.Context, r *Robot) (int64, string, error)
// Delete ...
Delete(ctx context.Context, id int64) error
// Update ...
Update(ctx context.Context, r *Robot) error
Update(ctx context.Context, r *Robot, option *Option) error
// List ...
List(ctx context.Context, query *q.Query, option *Option) ([]*Robot, error)
@ -75,9 +75,9 @@ func (d *controller) Count(ctx context.Context, query *q.Query) (int64, error) {
}
// Create ...
func (d *controller) Create(ctx context.Context, r *Robot) (int64, error) {
func (d *controller) Create(ctx context.Context, r *Robot) (int64, string, error) {
if err := d.setProjectID(ctx, r); err != nil {
return 0, err
return 0, "", err
}
var expiresAt int64
@ -91,15 +91,9 @@ func (d *controller) Create(ctx context.Context, r *Robot) (int64, error) {
expiresAt = time.Now().AddDate(0, 0, int(r.Duration)).Unix()
}
key, err := config.SecretKey()
if err != nil {
return 0, err
}
str := utils.GenerateRandomString()
secret, err := utils.ReversibleEncrypt(str, key)
if err != nil {
return 0, err
}
pwd := utils.GenerateRandomString()
salt := utils.GenerateRandomString()
secret := utils.Encrypt(pwd, salt, utils.SHA256)
robotID, err := d.robotMgr.Create(ctx, &model.Robot{
Name: r.Name,
@ -108,15 +102,16 @@ func (d *controller) Create(ctx context.Context, r *Robot) (int64, error) {
ExpiresAt: expiresAt,
Secret: secret,
Duration: r.Duration,
Salt: salt,
})
if err != nil {
return 0, err
return 0, "", err
}
r.ID = robotID
if err := d.createPermission(ctx, r); err != nil {
return 0, err
return 0, "", err
}
return robotID, nil
return robotID, pwd, nil
}
// Delete ...
@ -131,22 +126,21 @@ func (d *controller) Delete(ctx context.Context, id int64) error {
}
// Update ...
func (d *controller) Update(ctx context.Context, r *Robot) error {
func (d *controller) Update(ctx context.Context, r *Robot, option *Option) error {
if r == nil {
return errors.New("cannot update a nil robot").WithCode(errors.BadRequestCode)
}
if err := d.robotMgr.Update(ctx, &r.Robot, "secret", "description", "disabled", "duration"); err != nil {
return err
}
if err := d.setProjectID(ctx, r); err != nil {
return err
}
// update the permission
if err := d.rbacMgr.DeletePermissionsByRole(ctx, ROBOTTYPE, r.ID); err != nil {
return err
}
if err := d.createPermission(ctx, r); err != nil {
return err
if option != nil && option.WithPermission {
if err := d.rbacMgr.DeletePermissionsByRole(ctx, ROBOTTYPE, r.ID); err != nil {
return err
}
if err := d.createPermission(ctx, r); err != nil {
return err
}
}
return nil
}

4
src/controller/robot/controller_test.go
View File

@ -105,7 +105,7 @@ func (suite *ControllerTestSuite) TestCreate() {
rbacMgr.On("CreateRbacPolicy", mock.Anything, mock.Anything, mock.Anything).Return(int64(1), nil)
rbacMgr.On("CreatePermission", mock.Anything, mock.Anything, mock.Anything).Return(int64(1), nil)
id, err := c.Create(ctx, &Robot{
id, _, err := c.Create(ctx, &Robot{
Robot: model.Robot{
Name: "testcreate",
Description: "testcreate",
@ -193,6 +193,8 @@ func (suite *ControllerTestSuite) TestUpdate() {
},
},
},
}, &Option{
WithPermission: true,
})
suite.Nil(err)
}

4
src/controller/scan/base_controller.go
View File

@ -620,7 +620,7 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64
},
}
rb, err := bc.rc.Create(ctx, robotReq)
rb, pwd, err := bc.rc.Create(ctx, robotReq)
if err != nil {
return nil, errors.Wrap(err, "scan controller: make robot account")
}
@ -629,7 +629,7 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64
if err != nil {
return nil, errors.Wrap(err, "scan controller: make robot account")
}
r.Secret = pwd
return r, nil
}

4
src/controller/scan/base_controller_test.go
View File

@ -205,7 +205,7 @@ func (suite *ControllerTestSuite) SetupSuite() {
},
}
rc.On("Create", context.TODO(), account).Return(int64(1), nil)
rc.On("Create", context.TODO(), account).Return(int64(1), "robot-account", nil)
rc.On("Get", context.TODO(), int64(1), &robot.Option{
WithPermission: false,
}).Return(&robot.Robot{
@ -238,7 +238,7 @@ func (suite *ControllerTestSuite) SetupSuite() {
regJSON, err := suite.registration.ToJSON()
require.NoError(suite.T(), err)
id, _ := rc.Create(context.TODO(), account)
id, _, _ := rc.Create(context.TODO(), account)
rb, _ := rc.Get(context.TODO(), id, &robot.Option{WithPermission: false})
robotJSON, err := rb.ToJSON()
require.NoError(suite.T(), err)

1
src/pkg/robot2/model/model.go
View File

@ -18,6 +18,7 @@ type Robot struct {
Name string `orm:"column(name)" json:"name"`
Description string `orm:"column(description)" json:"description"`
Secret string `orm:"column(secret)" json:"secret"`
Salt string `orm:"column(salt)" json:"-"`
Duration int64 `orm:"column(duration)" json:"duration"`
ProjectID int64 `orm:"column(project_id)" json:"project_id"`
ExpiresAt int64 `orm:"column(expiresat)" json:"expires_at"`

18
src/server/middleware/security/robot2.go
View File

@ -27,16 +27,6 @@ func (r *robot2) Generate(req *http.Request) security.Context {
if !strings.HasPrefix(name, config.RobotPrefix()) {
return nil
}
key, err := config.SecretKey()
if err != nil {
log.Error("failed to get secret key")
return nil
}
_, err = utils.ReversibleDecrypt(secret, key)
if err != nil {
log.Errorf("failed to decode secret key: %s, %v", secret, err)
return nil
}
// TODO use the naming pattern to avoid the permission boundary crossing.
robots, err := robot_ctl.Ctl.List(req.Context(), q.New(q.KeyWords{
@ -52,18 +42,18 @@ func (r *robot2) Generate(req *http.Request) security.Context {
return nil
}
var accesses []*types.Policy
robot := robots[0]
if secret != robot.Secret {
log.Error("the secret provided is not correct.")
if utils.Encrypt(secret, robot.Salt, utils.SHA256) != robot.Secret {
log.Errorf("failed to authenticate robot account: %s", name)
return nil
}
if robot.Disabled {
log.Error("the robot is disabled.")
log.Errorf("failed to authenticate disabled robot account: %s", name)
return nil
}
// add the expiration check
var accesses []*types.Policy
for _, p := range robot.Permissions {
for _, a := range p.Access {
accesses = append(accesses, &types.Policy{

60
src/server/v2.0/handler/robot.go
View File

@ -15,6 +15,7 @@ import (
"github.com/goharbor/harbor/src/server/v2.0/handler/model"
"github.com/goharbor/harbor/src/server/v2.0/models"
operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/robot"
"regexp"
"strconv"
"strings"
)
@ -50,7 +51,7 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
rid, err := rAPI.robotCtl.Create(ctx, r)
rid, pwd, err := rAPI.robotCtl.Create(ctx, r)
if err != nil {
return rAPI.SendError(ctx, err)
}
@ -64,7 +65,7 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo
return operation.NewCreateRobotCreated().WithLocation(location).WithPayload(&models.RobotCreated{
ID: created.ID,
Name: created.Name,
Secret: created.Secret,
Secret: pwd,
CreationTime: strfmt.DateTime(created.CreationTime),
ExpiresAt: created.ExpiresAt,
})
@ -213,7 +214,9 @@ func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRo
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
}
if err := rAPI.robotCtl.Update(ctx, r); err != nil {
if err := rAPI.robotCtl.Update(ctx, r, &robot.Option{
WithPermission: true,
}); err != nil {
return rAPI.SendError(ctx, err)
}
@ -221,42 +224,39 @@ func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRo
}
func (rAPI *robotAPI) RefreshSec(ctx context.Context, params operation.RefreshSecParams) middleware.Responder {
if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil {
if err := rAPI.RequireAuthenticated(ctx); err != nil {
return rAPI.SendError(ctx, err)
}
if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionUpdate); err != nil {
return rAPI.SendError(ctx, err)
}
r, err := rAPI.robotCtl.Get(ctx, params.RobotID, &robot.Option{
WithPermission: true,
})
r, err := rAPI.robotCtl.Get(ctx, params.RobotID, nil)
if err != nil {
return rAPI.SendError(ctx, err)
}
if params.Robot.Secret != r.Secret {
return rAPI.SendError(ctx, errors.New(nil).WithMessage("the secret must be same with current").WithCode(errors.BadRequestCode))
if err := rAPI.requireAccess(ctx, r.Level, r.ProjectID, rbac.ActionUpdate); err != nil {
return rAPI.SendError(ctx, err)
}
key, err := config.SecretKey()
if err != nil {
return rAPI.SendError(ctx, err)
}
str := utils.GenerateRandomString()
secret, err := utils.ReversibleEncrypt(str, key)
if err != nil {
return rAPI.SendError(ctx, err)
var secret string
robotSec := &models.RobotSec{}
if params.RobotSec.Secret != "" {
if !isValidSec(params.RobotSec.Secret) {
return rAPI.SendError(ctx, errors.New("the secret must longer than 8 chars with at least 1 uppercase letter, 1 lowercase letter and 1 number").WithCode(errors.BadRequestCode))
}
secret = utils.Encrypt(params.RobotSec.Secret, r.Salt, utils.SHA256)
robotSec.Secret = ""
} else {
pwd := utils.GenerateRandomString()
secret = utils.Encrypt(pwd, r.Salt, utils.SHA256)
robotSec.Secret = pwd
}
r.Secret = secret
if err := rAPI.robotCtl.Update(ctx, r); err != nil {
if err := rAPI.robotCtl.Update(ctx, r, nil); err != nil {
return rAPI.SendError(ctx, err)
}
return operation.NewRefreshSecOK().WithPayload(&models.RobotSec{
Secret: r.Secret,
})
return operation.NewRefreshSecOK().WithPayload(robotSec)
}
func (rAPI *robotAPI) requireAccess(ctx context.Context, level string, projectIDOrName interface{}, action rbac.Action) error {
@ -296,3 +296,13 @@ func isValidLevel(l string) bool {
func isValidDuration(d int64) bool {
return d >= int64(-1)
}
func isValidSec(sec string) bool {
hasLower := regexp.MustCompile(`[a-z]`)
hasUpper := regexp.MustCompile(`[A-Z]`)
hasNumber := regexp.MustCompile(`[0-9]`)
if len(sec) >= 8 && hasLower.MatchString(sec) && hasUpper.MatchString(sec) && hasNumber.MatchString(sec) {
return true
}
return false
}

4
src/server/v2.0/handler/robotV1.go
View File

@ -96,7 +96,7 @@ func (rAPI *robotV1API) CreateRobotV1(ctx context.Context, params operation.Crea
permission.Access = policies
r.Permissions = append(r.Permissions, permission)
rid, err := rAPI.robotCtl.Create(ctx, r)
rid, pwd, err := rAPI.robotCtl.Create(ctx, r)
if err != nil {
return rAPI.SendError(ctx, err)
}
@ -110,7 +110,7 @@ func (rAPI *robotV1API) CreateRobotV1(ctx context.Context, params operation.Crea
return operation.NewCreateRobotV1Created().WithLocation(location).WithPayload(&models.RobotCreated{
ID: created.ID,
Name: created.Name,
Secret: created.Secret,
Secret: pwd,
CreationTime: strfmt.DateTime(created.CreationTime),
})
}

27
src/testing/controller/robot/controller.go
View File

@ -38,7 +38,7 @@ func (_m *Controller) Count(ctx context.Context, query *q.Query) (int64, error)
}
// Create provides a mock function with given fields: ctx, r
func (_m *Controller) Create(ctx context.Context, r *robot.Robot) (int64, error) {
func (_m *Controller) Create(ctx context.Context, r *robot.Robot) (int64, string, error) {
ret := _m.Called(ctx, r)
var r0 int64
@ -48,14 +48,21 @@ func (_m *Controller) Create(ctx context.Context, r *robot.Robot) (int64, error)
r0 = ret.Get(0).(int64)
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, *robot.Robot) error); ok {
var r1 string
if rf, ok := ret.Get(1).(func(context.Context, *robot.Robot) string); ok {
r1 = rf(ctx, r)
} else {
r1 = ret.Error(1)
r1 = ret.Get(1).(string)
}
return r0, r1
var r2 error
if rf, ok := ret.Get(2).(func(context.Context, *robot.Robot) error); ok {
r2 = rf(ctx, r)
} else {
r2 = ret.Error(2)
}
return r0, r1, r2
}
// Delete provides a mock function with given fields: ctx, id
@ -118,13 +125,13 @@ func (_m *Controller) List(ctx context.Context, query *q.Query, option *robot.Op
return r0, r1
}
// Update provides a mock function with given fields: ctx, r
func (_m *Controller) Update(ctx context.Context, r *robot.Robot) error {
ret := _m.Called(ctx, r)
// Update provides a mock function with given fields: ctx, r, option
func (_m *Controller) Update(ctx context.Context, r *robot.Robot, option *robot.Option) error {
ret := _m.Called(ctx, r, option)
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, *robot.Robot) error); ok {
r0 = rf(ctx, r)
if rf, ok := ret.Get(0).(func(context.Context, *robot.Robot, *robot.Option) error); ok {
r0 = rf(ctx, r, option)
} else {
r0 = ret.Error(0)
}