Skip to content

Commit

Permalink
Improved: Upgrade Apache Shiro from 1.13.0 to 2.0.0 (OFBIZ-12961)
Browse files Browse the repository at this point in the history
Summary, TL;DR: the changes are minimal and things work like before. OFBiz uses
now Shiro 2.0.0 for AES ciphering instead of Shiro 1.13.0.
OFBiz still uses 3-DES and other (older) ciphering methods in case AES would
fail facing old data.
This also removes now useless
"temporary workaround to compile Shiro 2.0.0 without LDAP"
component block in dependencies.gradle

Details:
This uses
'org.apache.shiro:shiro-crypto-cipher:2.0.0'
instead of previously wrongly committed
org.apache.shiro:shiro-crypto:2.0.0

It re-installs org.apache.shiro:shiro-core:1.13.0
I have still to completely review apache/shiro#1022
According to it, it seems that for now we need to keep shiro-core:1.13.0

http://svn.apache.org/viewvc?view=revision&revision=1814704, and the more
complete dev ML discussion referred in the commit message explains why we keep
3-DES and other (older) ciphering methods. I see no problems with that.
But, we may want to completely get rid of the old 3-DES and old ways by
refactoring this part of code. And maybe offering a way to migrate the data to
AES. The Shiro issue referred above may help in this way.

Thanks: Lenny from Apache Shiro project for the idea.
  • Loading branch information
JacquesLeRoux committed Mar 26, 2024
1 parent 0c24cb1 commit eff3a6a
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 17 deletions.
9 changes: 2 additions & 7 deletions dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ dependencies {
implementation 'org.apache.logging.log4j:log4j-core:2.20.0' // Somehow needed by Buildbot to compile OFBizDynamicThresholdFilter.java
implementation 'org.apache.poi:poi:4.1.2' // poi-ooxml-schemas-5.0.0.pom'. Received status code 401 from server
implementation 'org.apache.pdfbox:pdfbox:2.0.29' // 3.0.1 does not compile
implementation 'org.apache.shiro:shiro-crypto:2.0.0'
implementation 'org.apache.shiro:shiro-core:1.13.0'
implementation 'org.apache.shiro:shiro-crypto-cipher:2.0.0'
implementation 'org.apache.sshd:sshd-core:2.10.0'
implementation 'org.apache.sshd:sshd-sftp:2.10.0'
implementation 'org.apache.tika:tika-core:2.5.0'
Expand Down Expand Up @@ -82,12 +83,6 @@ dependencies {
implementation 'xerces:xercesImpl:2.12.2'
implementation 'org.mustangproject:library:2.8.0' // 2.10.0 did not work, cf. OFBIZ-12920 (https://github.com/apache/ofbiz-framework/pull/712#issuecomment-1968960963)

// Temporary workaround to compile Shiro 2.0.0 without LDAP
implementation 'org.apereo.cas:cas-server-core-api-authentication:5.0.10'
implementation 'org.apereo.cas:cas-server-core-util:5.0.10'
implementation 'org.apereo.cas:cas-server-support-ldap-core:5.0.10'


testImplementation 'org.hamcrest:hamcrest-library:2.2' // Enable junit4 to not depend on hamcrest-1.3
testImplementation 'org.mockito:mockito-core:5.10.0'
testImplementation 'org.jmockit:jmockit:1.49'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import org.apache.commons.codec.binary.Base64;
import org.apache.ofbiz.base.util.Debug;
import org.apache.shiro.crypto.AesCipherService;
import org.apache.shiro.crypto.cipher.AesCipherService;

public class Main {
private static final String MODULE = Main.class.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.model.ModelField.EncryptMethod;
import org.apache.ofbiz.entity.transaction.TransactionUtil;
import org.apache.shiro.crypto.AesCipherService;
import org.apache.shiro.crypto.OperationMode;
import org.apache.shiro.crypto.PaddingScheme;
import org.apache.shiro.crypto.cipher.AesCipherService;
import org.apache.shiro.crypto.cipher.OperationMode;
import org.apache.shiro.crypto.cipher.PaddingScheme;
import org.apache.shiro.crypto.hash.DefaultHashService;
import org.apache.shiro.crypto.hash.HashRequest;
import org.apache.shiro.crypto.hash.HashService;
Expand Down Expand Up @@ -129,7 +129,7 @@ public Object decrypt(String keyName, EncryptMethod encryptMethod, String encryp
} catch (Exception e) {
/*
When the field is encrypted with the old algorithm (3-DES), the new Shiro code will fail to decrypt it (using AES) and then it will
throw an org.apache.shiro.crypto.CryptoException that is a RuntimeException.
throw an org.apache.shiro.crypto.cipher.CryptoException that is a RuntimeException.
For backward compatibility we want instead to catch the exception and decrypt the code using the old algorithm.
*/
Debug.logInfo("Decrypt with DES key from standard key name hash failed, trying old/funny variety of key name hash", MODULE);
Expand Down Expand Up @@ -263,7 +263,7 @@ protected String getKeyMapPrefix(String hashedKeyName) {
protected byte[] decodeKeyBytes(String keyText) throws GeneralException {
byte[] keyBytes = Base64.decodeBase64(keyText);
if (kek != null) {
keyBytes = saltedCipherService.decrypt(keyBytes, kek).getBytes();
keyBytes = saltedCipherService.decrypt(keyBytes, kek).getClonedBytes();
}
return keyBytes;
}
Expand All @@ -281,9 +281,9 @@ protected String encodeKey(byte[] key) throws GeneralException {
protected byte[] decryptValue(byte[] key, EncryptMethod encryptMethod, String encryptedString) throws GeneralException {
switch (encryptMethod) {
case SALT:
return saltedCipherService.decrypt(Base64.decodeBase64(encryptedString), key).getBytes();
return saltedCipherService.decrypt(Base64.decodeBase64(encryptedString), key).getClonedBytes();
default:
return cipherService.decrypt(Base64.decodeBase64(encryptedString), key).getBytes();
return cipherService.decrypt(Base64.decodeBase64(encryptedString), key).getClonedBytes();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
import org.apache.ofbiz.service.GenericServiceException;
import org.apache.ofbiz.service.LocalDispatcher;
import org.apache.ofbiz.service.ServiceUtil;
import org.apache.shiro.crypto.AesCipherService;
import org.apache.shiro.crypto.cipher.AesCipherService;

/**
* Entity Data Import/Export Services
Expand Down Expand Up @@ -473,7 +473,7 @@ public static Map<String, Object> reencryptPrivateKeys(DispatchContext dctx, Map
if (oldKey != null) {
Debug.logInfo("Decrypting with old key: " + oldKey, MODULE);
try {
keyBytes = cipherService.decrypt(keyBytes, Base64.decodeBase64(oldKey)).getBytes();
keyBytes = cipherService.decrypt(keyBytes, Base64.decodeBase64(oldKey)).getClonedBytes();
} catch (Exception e) {
Debug.logInfo("Failed to decrypt with Shiro cipher; trying with old cipher", MODULE);
try {
Expand Down

0 comments on commit eff3a6a

Please sign in to comment.