Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Encrypt password returned to user, add support for Login 
On Behalf
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/42291/3/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/OAuthTokenInfo.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/servlets/OAuthTokenInfo.java:

Line 34:             Map<String, Object> sessionData = 
SSOUtils.getSessionData(request.getServletContext(), token);
Line 35:             if (sessionData == null) {
Line 36:                 throw new 
OAuthException(SSOUtils.ERR_CODE_INVALID_GRANT, "The provided authorization 
grant for the token has expired");
Line 37:             }
Line 38:             String password = (String) 
sessionData.get(SSOUtils.PASSWORD);
please store the password within context as already encrypted. there is no need 
to encrypt it over and over nor to store in clear text.
Line 39:             if (StringUtils.isNotEmpty(password) &&
Line 40:                     
SSOUtils.scopeAsList(scope).contains("ovirt-auth-password-access")) {
Line 41:                 password = 
SSOUtils.encrypt(request.getServletContext(), password);
Line 42:             }


https://gerrit.ovirt.org/#/c/42291/3/backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOUtils.java
File 
backend/manager/modules/enginesso/src/main/java/org/ovirt/engine/core/sso/utils/SSOUtils.java:

Line 484:             storePassword = 
ssoConfig.getSsoLocalConfig().getProperty("ENGINE_PKI_ENGINE_STORE_PASSWORD", 
true);
Line 485:             storeAlias = 
ssoConfig.getSsoLocalConfig().getProperty("ENGINE_PKI_ENGINE_STORE_ALIAS", 
true);
Line 486:         }
Line 487: 
Line 488:         KeyStore.PrivateKeyEntry entry = 
getPrivateKeyEntry(getKeyStore(storeType, store, storePassword), storeAlias, 
storePassword);
best implementation:

each client should have its own certificate reference to encrypt to. when user 
login you should find all clients that are able to receive password and encrypt 
password to these.

this way you do not store password in plain text, and you reduce encryption 
operation when sending out tokens.
Line 489: 
Line 490:         return EnvelopeEncryptDecrypt.encrypt(
Line 491:                 "AES/OFB/PKCS5Padding",
Line 492:                 256,


Line 491:                 "AES/OFB/PKCS5Padding",
Line 492:                 256,
Line 493:                 entry.getCertificate(),
Line 494:                 100,
Line 495:                 rawText.getBytes(Charset.forName("UTF-8")));
these all should be parameters (except of the certificate).
Line 496:     }
Line 497: 
Line 498:     private static KeyStore getKeyStore(String storeType, String 
store, String password) throws Exception {
Line 499:         KeyStore ks = KeyStore.getInstance(storeType);


Line 494:                 100,
Line 495:                 rawText.getBytes(Charset.forName("UTF-8")));
Line 496:     }
Line 497: 
Line 498:     private static KeyStore getKeyStore(String storeType, String 
store, String password) throws Exception {
probably all you need is the certificate, you do not need the keystore that 
holds the private key.
Line 499:         KeyStore ks = KeyStore.getInstance(storeType);
Line 500:         try (InputStream is = new FileInputStream(store)) {
Line 501:             ks.load(is, password.toCharArray());
Line 502:         }


-- 
To view, visit https://gerrit.ovirt.org/42291
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1422774b0fbf52a59299f521ad92c6d576f7a1b2
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to