Skip to content

Commit

Permalink
Move certificate verification to target level
Browse files Browse the repository at this point in the history
The certificate verification is on system level before this commit. Moving it
to target level makes the configuration more flexible for different targets.
  • Loading branch information
ywk253100 committed Oct 20, 2017
1 parent 75af80b commit 2156750
Show file tree
Hide file tree
Showing 35 changed files with 82 additions and 96 deletions.
12 changes: 12 additions & 0 deletions docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2339,6 +2339,9 @@ definitions:
type: integer
format: int
description: Reserved field.
insecure:
type: boolean
description: Whether or not the certificate will be verified when Harbor tries to access the server.
creation_time:
type: string
description: The create time of the policy.
Expand All @@ -2360,6 +2363,9 @@ definitions:
password:
type: string
description: The target server password.
insecure:
type: boolean
description: Whether or not the certificate will be verified when Harbor tries to access the server.
PingTarget:
type: object
properties:
Expand All @@ -2372,6 +2378,9 @@ definitions:
password:
type: string
description: The target server password.
insecure:
type: boolean
description: Whether or not the certificate will be verified when Harbor tries to access the server.
PutTarget:
type: object
properties:
Expand All @@ -2387,6 +2396,9 @@ definitions:
password:
type: string
description: The target server password.
insecure:
type: boolean
description: Whether or not the certificate will be verified when Harbor tries to access the server.
HasAdminRole:
type: object
properties:
Expand Down
1 change: 1 addition & 0 deletions make/common/db/registry.sql
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ create table replication_target (
1 means it's a regulart registry
*/
target_type tinyint(1) NOT NULL DEFAULT 0,
insecure tinyint(1) NOT NULL DEFAULT 0,
creation_time timestamp default CURRENT_TIMESTAMP,
update_time timestamp default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP,
PRIMARY KEY (id)
Expand Down
1 change: 1 addition & 0 deletions make/common/db/registry_sqlite.sql
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ create table replication_target (
1 means it's a regulart registry
*/
target_type tinyint(1) NOT NULL DEFAULT 0,
insecure tinyint(1) NOT NULL DEFAULT 0,
creation_time timestamp default CURRENT_TIMESTAMP,
update_time timestamp default CURRENT_TIMESTAMP
);
Expand Down
1 change: 0 additions & 1 deletion make/common/templates/adminserver/env
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ EMAIL_FROM=$email_from
EMAIL_IDENTITY=$email_identity
HARBOR_ADMIN_PASSWORD=$harbor_admin_password
PROJECT_CREATION_RESTRICTION=$project_creation_restriction
VERIFY_REMOTE_CERT=$verify_remote_cert
MAX_JOB_WORKERS=$max_job_workers
UI_SECRET=$ui_secret
JOBSERVICE_SECRET=$jobservice_secret
Expand Down
2 changes: 0 additions & 2 deletions make/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ if protocol == "https":
customize_crt = rcp.get("configuration", "customize_crt")
max_job_workers = rcp.get("configuration", "max_job_workers")
token_expiration = rcp.get("configuration", "token_expiration")
verify_remote_cert = rcp.get("configuration", "verify_remote_cert")
proj_cre_restriction = rcp.get("configuration", "project_creation_restriction")
secretkey_path = rcp.get("configuration", "secretkey_path")
if rcp.has_option("configuration", "admiral_url"):
Expand Down Expand Up @@ -239,7 +238,6 @@ render(os.path.join(templates_dir, "adminserver", "env"),
email_identity=email_identity,
harbor_admin_password=harbor_admin_password,
project_creation_restriction=proj_cre_restriction,
verify_remote_cert=verify_remote_cert,
max_job_workers=max_job_workers,
ui_secret=ui_secret,
jobservice_secret=jobservice_secret,
Expand Down
4 changes: 0 additions & 4 deletions src/adminserver/systemcfg/systemcfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ var (
env: "MAX_JOB_WORKERS",
parse: parseStringToInt,
},
common.VerifyRemoteCert: &parser{
env: "VERIFY_REMOTE_CERT",
parse: parseStringToBool,
},
common.ProjectCreationRestriction: "PROJECT_CREATION_RESTRICTION",
common.AdminInitialPassword: "HARBOR_ADMIN_PASSWORD",
common.AdmiralEndpoint: "ADMIRAL_URL",
Expand Down
1 change: 0 additions & 1 deletion src/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ const (
EmailIdentity = "email_identity"
EmailInsecure = "email_insecure"
ProjectCreationRestriction = "project_creation_restriction"
VerifyRemoteCert = "verify_remote_cert"
MaxJobWorkers = "max_job_workers"
TokenExpiration = "token_expiration"
CfgExpiration = "cfg_expiration"
Expand Down
2 changes: 1 addition & 1 deletion src/common/dao/replication_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func DeleteRepTarget(id int64) error {
func UpdateRepTarget(target models.RepTarget) error {
o := GetOrmer()
target.UpdateTime = time.Now()
_, err := o.Update(&target, "URL", "Name", "Username", "Password", "UpdateTime")
_, err := o.Update(&target, "URL", "Name", "Username", "Password", "Insecure", "UpdateTime")
return err
}

Expand Down
1 change: 1 addition & 0 deletions src/common/models/replication_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ type RepTarget struct {
Username string `orm:"column(username)" json:"username"`
Password string `orm:"column(password)" json:"password"`
Type int `orm:"column(target_type)" json:"type"`
Insecure bool `orm:"column(insecure)" json:"insecure"`
CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"`
UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"`
}
Expand Down
1 change: 0 additions & 1 deletion src/common/utils/test/adminserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ var adminServerDefaultConfig = map[string]interface{}{
common.EmailInsecure: false,
common.EmailIdentity: "",
common.ProjectCreationRestriction: common.ProCrtRestrAdmOnly,
common.VerifyRemoteCert: false,
common.MaxJobWorkers: 3,
common.TokenExpiration: 30,
common.CfgExpiration: 5,
Expand Down
9 changes: 0 additions & 9 deletions src/jobservice/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,6 @@ func initKeyProvider() {
keyProvider = comcfg.NewFileKeyProvider(path)
}

// VerifyRemoteCert returns bool value.
func VerifyRemoteCert() (bool, error) {
cfg, err := mg.Get()
if err != nil {
return true, err
}
return cfg[common.VerifyRemoteCert].(bool), nil
}

// Database ...
func Database() (*models.Database, error) {
cfg, err := mg.Get()
Expand Down
4 changes: 0 additions & 4 deletions src/jobservice/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ func TestConfig(t *testing.T) {
t.Fatalf("failed to initialize configurations: %v", err)
}

if _, err := VerifyRemoteCert(); err != nil {
t.Fatalf("failed to get verify remote cert: %v", err)
}

if _, err := Database(); err != nil {
t.Fatalf("failed to get database settings: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion src/jobservice/job/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestRepJob(t *testing.T) {
j, err := dao.GetRepJob(repJobID)
assert.Equal(models.JobRetrying, j.Status)
assert.Equal(1, rj.parm.Enabled)
assert.True(rj.parm.Insecure)
assert.False(rj.parm.Insecure)
rj2 := NewRepJob(99999)
err = rj2.Init()
assert.NotNil(err)
Expand Down
6 changes: 1 addition & 5 deletions src/jobservice/job/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,12 @@ func (rj *RepJob) Init() error {
if err != nil {
return err
}
verify, err := config.VerifyRemoteCert()
if err != nil {
return err
}
rj.parm = &RepJobParm{
LocalRegURL: regURL,
Repository: job.Repository,
Tags: job.TagList,
Enabled: policy.Enabled,
Operation: job.Operation,
Insecure: !verify,
}
if policy.Enabled == 0 {
//worker will cancel this job
Expand Down Expand Up @@ -159,6 +154,7 @@ func (rj *RepJob) Init() error {
}

rj.parm.TargetPassword = pwd
rj.parm.Insecure = target.Insecure
return nil
}

Expand Down
2 changes: 0 additions & 2 deletions src/ui/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ var (
common.EmailIdentity,
common.EmailInsecure,
common.ProjectCreationRestriction,
common.VerifyRemoteCert,
common.TokenExpiration,
common.ScanAllPolicy,
}
Expand Down Expand Up @@ -81,7 +80,6 @@ var (
common.EmailSSL,
common.EmailInsecure,
common.SelfRegistration,
common.VerifyRemoteCert,
}

passwordKeys = []string{
Expand Down
8 changes: 4 additions & 4 deletions src/ui/api/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestPutConfig(t *testing.T) {
apiTest := newHarborAPI()

cfg := map[string]interface{}{
common.VerifyRemoteCert: false,
common.TokenExpiration: 60,
}

code, err := apiTest.PutConfig(*admin, cfg)
Expand Down Expand Up @@ -104,13 +104,13 @@ func TestResetConfig(t *testing.T) {
return
}

value, ok := cfgs[common.VerifyRemoteCert]
value, ok := cfgs[common.TokenExpiration]
if !ok {
t.Errorf("%s not found", common.VerifyRemoteCert)
t.Errorf("%s not found", common.TokenExpiration)
return
}

assert.Equal(value.Value.(bool), true, "unexpected value")
assert.Equal(int(value.Value.(float64)), 30, "unexpected 30")

ccc, err := config.GetSystemCfg()
if err != nil {
Expand Down
19 changes: 10 additions & 9 deletions src/ui/api/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,8 @@ func (t *TargetAPI) Prepare() {
}
}

func (t *TargetAPI) ping(endpoint, username, password string) {
verify, err := config.VerifyRemoteCert()
if err != nil {
log.Errorf("failed to check whether insecure or not: %v", err)
t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
}
registry, err := newRegistryClient(endpoint, !verify, username, password)
func (t *TargetAPI) ping(endpoint, username, password string, insecure bool) {
registry, err := newRegistryClient(endpoint, insecure, username, password)
if err != nil {
// timeout, dns resolve error, connection refused, etc.
if urlErr, ok := err.(*url.Error); ok {
Expand Down Expand Up @@ -105,14 +100,15 @@ func (t *TargetAPI) PingByID() {
endpoint := target.URL
username := target.Username
password := target.Password
insecure := target.Insecure
if len(password) != 0 {
password, err = utils.ReversibleDecrypt(password, t.secretKey)
if err != nil {
log.Errorf("failed to decrypt password: %v", err)
t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
}
}
t.ping(endpoint, username, password)
t.ping(endpoint, username, password, insecure)
}

// Ping validates whether the target is reachable and whether the credential is valid
Expand All @@ -121,14 +117,15 @@ func (t *TargetAPI) Ping() {
Endpoint string `json:"endpoint"`
Username string `json:"username"`
Password string `json:"password"`
Insecure bool `json:"insecure"`
}{}
t.DecodeJSONReq(&req)

if len(req.Endpoint) == 0 {
t.CustomAbort(http.StatusBadRequest, "endpoint is required")
}

t.ping(req.Endpoint, req.Username, req.Password)
t.ping(req.Endpoint, req.Username, req.Password, req.Insecure)
}

// Get ...
Expand Down Expand Up @@ -255,6 +252,7 @@ func (t *TargetAPI) Put() {
Endpoint *string `json:"endpoint"`
Username *string `json:"username"`
Password *string `json:"password"`
Insecure *bool `json:"insecure"`
}{}
t.DecodeJSONReq(&req)

Expand All @@ -273,6 +271,9 @@ func (t *TargetAPI) Put() {
if req.Password != nil {
target.Password = *req.Password
}
if req.Insecure != nil {
target.Insecure = *req.Insecure
}

t.Validate(target)

Expand Down
9 changes: 0 additions & 9 deletions src/ui/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,15 +276,6 @@ func OnlyAdminCreateProject() (bool, error) {
return cfg[common.ProjectCreationRestriction].(string) == common.ProCrtRestrAdmOnly, nil
}

// VerifyRemoteCert returns bool value.
func VerifyRemoteCert() (bool, error) {
cfg, err := mg.Get()
if err != nil {
return true, err
}
return cfg[common.VerifyRemoteCert].(bool), nil
}

// Email returns email server settings
func Email() (*models.Email, error) {
cfg, err := mg.Get()
Expand Down
4 changes: 0 additions & 4 deletions src/ui/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,6 @@ func TestConfig(t *testing.T) {
t.Fatalf("failed to get onldy admin create project: %v", err)
}

if _, err := VerifyRemoteCert(); err != nil {
t.Fatalf("failed to get verify remote cert: %v", err)
}

if _, err := Email(); err != nil {
t.Fatalf("failed to get email settings: %v", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export const CREATE_EDIT_ENDPOINT_TEMPLATE: string = `
<label for="destination_password" class="col-md-4 form-group-label-override">{{ 'DESTINATION.PASSWORD' | translate }}</label>
<input type="password" class="col-md-8" id="destination_password" [disabled]="testOngoing" [readonly]="!editable" [(ngModel)]="target.password" size="20" name="password" #password="ngModel" (focus)="clearPassword($event)">
</div>
<div class="form-group">
<label for="destination_insecure" class="col-md-4 form-group-label-override">{{'CONFIG.VERIFY_REMOTE_CERT' | translate }}</label>
<clr-checkbox #insecure class="col-md-8" name="insecure" id="destination_insecure" [(ngModel)]="target.insecure"></clr-checkbox>
</div>
<div class="form-group">
<label for="spin" class="col-md-4"></label>
<span class="col-md-8 spinner spinner-inline" [hidden]="!inProgress"></span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('CreateEditEndpointComponent (inline template)', () => {
"name": "target_01",
"username": "admin",
"password": "",
"insecure": false,
"type": 0
};

Expand Down
Loading

0 comments on commit 2156750

Please sign in to comment.