From 318d10186e89bea6964737f212028f31d496a78d Mon Sep 17 00:00:00 2001 From: Tan Jiang Date: Wed, 3 Aug 2016 17:25:24 +0800 Subject: [PATCH] Use AES to encrypt password for target --- .travis.yml | 2 ++ Deploy/db/registry.sql | 2 +- Deploy/harbor.cfg | 4 +++ Deploy/prepare | 9 ++++++ Deploy/templates/jobservice/env | 1 + Deploy/templates/ui/env | 1 + api/target.go | 23 +++++++++++--- job/config/config.go | 11 +++++++ job/statemachine.go | 2 +- utils/encrypt.go | 54 ++++++++++++++++++++++++++++----- utils/utils_test.go | 20 ++++++++++++ 11 files changed, 115 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index 35c01aff1..3b3b77435 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,6 +28,8 @@ env: HARBOR_ADMIN: admin HARBOR_ADMIN_PASSWD: Harbor12345 UI_SECRET: tempString + MAX_JOB_WORKERS: 3 + SECRET_KEY: 1234567890123456 before_install: - sudo ./tests/hostcfg.sh diff --git a/Deploy/db/registry.sql b/Deploy/db/registry.sql index e5f13f7d4..121e27aae 100644 --- a/Deploy/db/registry.sql +++ b/Deploy/db/registry.sql @@ -122,7 +122,7 @@ create table replication_target ( name varchar(64), url varchar(64), username varchar(40), - password varchar(40), + password varchar(128), /* target_type indicates the type of target registry, 0 means it's a harbor instance, diff --git a/Deploy/harbor.cfg b/Deploy/harbor.cfg index bf8a721e7..186c711fe 100644 --- a/Deploy/harbor.cfg +++ b/Deploy/harbor.cfg @@ -44,6 +44,10 @@ use_compressed_js = on #Maximum number of job workers in job service max_job_workers = 3 +#Secret key for encryption/decryption, its length has to be 16 chars +#**NOTE** if this changes, previously encrypted password will not be decrypted! +secret_key = secretkey1234567 + #Determine whether the job service should verify the ssl cert when it connects to a remote registry. #Set this flag to off when the remote registry uses a self-signed or untrusted certificate. verify_remote_cert = on diff --git a/Deploy/prepare b/Deploy/prepare index cdea56550..16fadf374 100755 --- a/Deploy/prepare +++ b/Deploy/prepare @@ -16,6 +16,10 @@ if sys.version_info[:3][0] == 3: import configparser as ConfigParser import io as StringIO +def validate(conf): + if len(conf.get("configuration", "secret_key")) != 16: + raise Exception("Error: The length of secret key has to be 16 characters!") + #Read configurations conf = StringIO.StringIO() conf.write("[configuration]\n") @@ -24,6 +28,8 @@ conf.seek(0, os.SEEK_SET) rcp = ConfigParser.RawConfigParser() rcp.readfp(conf) +validate(rcp) + hostname = rcp.get("configuration", "hostname") ui_url = rcp.get("configuration", "ui_url_protocol") + "://" + hostname email_server = rcp.get("configuration", "email_server") @@ -49,6 +55,7 @@ crt_commonname = rcp.get("configuration", "crt_commonname") crt_email = rcp.get("configuration", "crt_email") max_job_workers = rcp.get("configuration", "max_job_workers") verify_remote_cert = rcp.get("configuration", "verify_remote_cert") +secret_key = rcp.get("configuration", "secret_key") ######## ui_secret = ''.join(random.choice(string.ascii_letters+string.digits) for i in range(16)) @@ -101,6 +108,7 @@ render(os.path.join(templates_dir, "ui", "env"), self_registration=self_registration, use_compressed_js=use_compressed_js, ui_secret=ui_secret, + secret_key=secret_key, verify_remote_cert=verify_remote_cert) render(os.path.join(templates_dir, "ui", "app.conf"), @@ -126,6 +134,7 @@ render(os.path.join(templates_dir, "jobservice", "env"), db_password=db_password, ui_secret=ui_secret, max_job_workers=max_job_workers, + secret_key=secret_key, ui_url=ui_url, verify_remote_cert=verify_remote_cert) diff --git a/Deploy/templates/jobservice/env b/Deploy/templates/jobservice/env index 5359e8512..7cefb7fa4 100644 --- a/Deploy/templates/jobservice/env +++ b/Deploy/templates/jobservice/env @@ -3,6 +3,7 @@ MYSQL_PORT=3306 MYSQL_USR=root MYSQL_PWD=$db_password UI_SECRET=$ui_secret +SECRET_KEY=$secret_key CONFIG_PATH=/etc/jobservice/app.conf REGISTRY_URL=http://registry:5000 VERIFY_REMOTE_CERT=$verify_remote_cert diff --git a/Deploy/templates/ui/env b/Deploy/templates/ui/env index 972d94df7..dde28fc5e 100644 --- a/Deploy/templates/ui/env +++ b/Deploy/templates/ui/env @@ -12,6 +12,7 @@ AUTH_MODE=$auth_mode LDAP_URL=$ldap_url LDAP_BASE_DN=$ldap_basedn UI_SECRET=$ui_secret +SECRET_KEY=$secret_key SELF_REGISTRATION=$self_registration USE_COMPRESSED_JS=$use_compressed_js LOG_LEVEL=debug diff --git a/api/target.go b/api/target.go index 1c78b915d..c3cb205ea 100644 --- a/api/target.go +++ b/api/target.go @@ -20,6 +20,7 @@ import ( "net" "net/http" "net/url" + "os" "strconv" "github.com/vmware/harbor/dao" @@ -34,10 +35,14 @@ import ( // TargetAPI handles request to /api/targets/ping /api/targets/{} type TargetAPI struct { BaseAPI + secretKey string } // Prepare validates the user func (t *TargetAPI) Prepare() { + //TODO:move to config + t.secretKey = os.Getenv("SECRET_KEY") + userID := t.ValidateUser() isSysAdmin, err := dao.IsAdminRole(userID) if err != nil { @@ -76,7 +81,7 @@ func (t *TargetAPI) Ping() { password = target.Password if len(password) != 0 { - password, err = utils.ReversibleDecrypt(password) + 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)) @@ -136,7 +141,7 @@ func (t *TargetAPI) Get() { // modify other fields of target he does not need to input the password again. // The security issue can be fixed by enable https. if len(target.Password) != 0 { - pwd, err := utils.ReversibleDecrypt(target.Password) + pwd, err := utils.ReversibleDecrypt(target.Password, t.secretKey) if err != nil { log.Errorf("failed to decrypt password: %v", err) t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) @@ -162,7 +167,7 @@ func (t *TargetAPI) List() { continue } - str, err := utils.ReversibleDecrypt(target.Password) + str, err := utils.ReversibleDecrypt(target.Password, t.secretKey) if err != nil { log.Errorf("failed to decrypt password: %v", err) t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) @@ -201,7 +206,11 @@ func (t *TargetAPI) Post() { } if len(target.Password) != 0 { - target.Password = utils.ReversibleEncrypt(target.Password) + target.Password, err = utils.ReversibleEncrypt(target.Password, t.secretKey) + if err != nil { + log.Errorf("failed to encrypt password: %v", err) + t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + } } id, err := dao.AddRepTarget(*target) @@ -275,7 +284,11 @@ func (t *TargetAPI) Put() { target.ID = id if len(target.Password) != 0 { - target.Password = utils.ReversibleEncrypt(target.Password) + target.Password, err = utils.ReversibleEncrypt(target.Password, t.secretKey) + if err != nil { + log.Errorf("failed to encrypt password: %v", err) + t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + } } if err := dao.UpdateRepTarget(*target); err != nil { diff --git a/job/config/config.go b/job/config/config.go index b5477e2f3..1403c30b4 100644 --- a/job/config/config.go +++ b/job/config/config.go @@ -31,6 +31,7 @@ var localUIURL string var localRegURL string var logDir string var uiSecret string +var secretKey string var verifyRemoteCert string func init() { @@ -86,6 +87,11 @@ func init() { beego.LoadAppConfig("ini", configPath) } + secretKey = os.Getenv("SECRET_KEY") + if len(secretKey) != 16 { + panic("The length of secretkey has to be 16 characters!") + } + log.Debugf("config: maxJobWorkers: %d", maxJobWorkers) log.Debugf("config: localUIURL: %s", localUIURL) log.Debugf("config: localRegURL: %s", localRegURL) @@ -119,6 +125,11 @@ func UISecret() string { return uiSecret } +// SecretKey will return the secret key for encryption/decryption password in target. +func SecretKey() string { + return secretKey +} + // VerifyRemoteCert return the flag to tell jobservice whether or not verify the cert of remote registry func VerifyRemoteCert() bool { return verifyRemoteCert != "off" diff --git a/job/statemachine.go b/job/statemachine.go index 95b242740..c4f245c7c 100644 --- a/job/statemachine.go +++ b/job/statemachine.go @@ -231,7 +231,7 @@ func (sm *SM) Reset(jid int64) error { pwd := target.Password if len(pwd) != 0 { - pwd, err = uti.ReversibleDecrypt(pwd) + pwd, err = uti.ReversibleDecrypt(pwd, config.SecretKey()) if err != nil { return fmt.Errorf("failed to decrypt password: %v", err) } diff --git a/utils/encrypt.go b/utils/encrypt.go index e5ce44792..f326353e0 100644 --- a/utils/encrypt.go +++ b/utils/encrypt.go @@ -16,9 +16,14 @@ package utils import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" "crypto/sha1" "encoding/base64" + "errors" "fmt" + "io" "golang.org/x/crypto/pbkdf2" ) @@ -28,13 +33,48 @@ func Encrypt(content string, salt string) string { return fmt.Sprintf("%x", pbkdf2.Key([]byte(content), []byte(salt), 4096, 16, sha1.New)) } -// ReversibleEncrypt encrypts the str with base64 -func ReversibleEncrypt(str string) string { - return base64.StdEncoding.EncodeToString([]byte(str)) +// ReversibleEncrypt encrypts the str with aes/base64 +func ReversibleEncrypt(str, key string) (string, error) { + keyBytes := []byte(key) + var block cipher.Block + var err error + + if block, err = aes.NewCipher(keyBytes); err != nil { + return "", err + } + cipherText := make([]byte, aes.BlockSize+len(str)) + iv := cipherText[:aes.BlockSize] + if _, err = io.ReadFull(rand.Reader, iv); err != nil { + return "", err + } + + cfb := cipher.NewCFBEncrypter(block, iv) + cfb.XORKeyStream(cipherText[aes.BlockSize:], []byte(str)) + encrypted := base64.StdEncoding.EncodeToString(cipherText) + return encrypted, nil } -// ReversibleDecrypt decrypts the str with base64 -func ReversibleDecrypt(str string) (string, error) { - b, err := base64.StdEncoding.DecodeString(str) - return string(b), err +// ReversibleDecrypt decrypts the str with aes/base64 +func ReversibleDecrypt(str, key string) (string, error) { + keyBytes := []byte(key) + var block cipher.Block + var cipherText []byte + var err error + + if block, err = aes.NewCipher(keyBytes); err != nil { + return "", err + } + if cipherText, err = base64.StdEncoding.DecodeString(str); err != nil { + return "", err + } + if len(cipherText) < aes.BlockSize { + err = errors.New("cipherText too short") + return "", err + } + + iv := cipherText[:aes.BlockSize] + cipherText = cipherText[aes.BlockSize:] + cfb := cipher.NewCFBDecrypter(block, iv) + cfb.XORKeyStream(cipherText, cipherText) + return string(cipherText), nil } diff --git a/utils/utils_test.go b/utils/utils_test.go index f2b23c1da..4187c872a 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -61,3 +61,23 @@ func TestParseRepository(t *testing.T) { t.Errorf("unexpected rest: [%s] != [%s]", rest, "") } } + +func TestReversibleEncrypt(t *testing.T) { + password := "password" + key := "1234567890123456" + encrypted, err := ReversibleEncrypt(password, key) + if err != nil { + t.Errorf("Failed to encrypt: %v", err) + } + t.Logf("Encrypted password: %s", encrypted) + if encrypted == password { + t.Errorf("Encrypted password is identical to the original") + } + decrypted, err := ReversibleDecrypt(encrypted, key) + if err != nil { + t.Errorf("Failed to decrypt: %v", err) + } + if decrypted != password { + t.Errorf("decrypted password: %s, is not identical to original", decrypted) + } +}