Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-3.30] E2EE cert validation fix #295

Open
wants to merge 1 commit into
base: stable-3.30
Choose a base branch
from
Open

[stable-3.30] E2EE cert validation fix #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/src/androidTest/assets/invalid_certs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"publicKey": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2Fujuh6usrHcIrIV1Dqb\nZ5JZ4ddl0ux/GW4SSMdfxXaL2HU6C/SzmrHVBwEoZYdK+1P0VnJqBgLPHEdvbf79\nazaJpR2Ll3VMN7GPYbreBcImyKezAOSzZaf1pIxOVAjRd+hMdT//6p+kYyxvS1q8\nNwbEp/bRwx8qa1+zaZ8rJPuriQEl/qd3ozUuG2P6jNlDaJYcFWKJJJsNFRYowhY7\nm4qnF8q3pgFPadY/HokE0T/pYUJ/jdUkeu/Nzl2F2oMoRpnYgyB9ZqjVB599bhKc\nmv9ey8nsFgo4u39dKTqXqIJRkvMMkG92g86qChYln3RCqHzDS+pzGJNVyUX51QPo\nYwIDAQAB\n-----END PUBLIC KEY-----\n",
"certificate": "-----BEGIN CERTIFICATE-----\nMIIDFDCCAfygAwIBAgIBADANBgkqhkiG9w0BAQUFADAjMSEwHwYDVQQDDBgxMjAw\nNDkwMTAwMDAwMDAzMTIzNjcyODMwHhcNMjQwMzA3MTQxNTA2WhcNNDQwMzAyMTQx\nNTA2WjAjMSEwHwYDVQQDDBgxMjAwNDkwMTAwMDAwMDAzMTIzNjcyODMwggEiMA0G\nCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCoQKjjsyhGZbnYM6+tv7HVNTw6P+xq\n0U+247wzdLfVPzyDOw41F2+UvL6hPY7GYQSpEGHVFUMlaJCP3TOBYxtbqYhj23ew\nL6oeIZia7DZbMNV9NvKHtFvyPzPPusFpKJuhFhcjULEsQ4RmsMvam3fJpPEm+Ngh\nDbUkNB1oEdEhB8qtqDI3QY3KGOH8SPeeMad38ALmsnxRrxqZtLGSRGBuMFFfo1HM\n+KEVlCIirwxcmciGThR0w9lsOe1Sjfi+3pj3BOjRAUxA7TTC1ec2BYCfk0TWXvVp\n7GT4SWeIEIQVgEY261hTdLywDeMUkoQAwZwvLKYyDLTuRgbcqjE/m1ILAgMBAAGj\nUzBRMB0GA1UdDgQWBBTokFvhPZPUAUPgGwFwSnzNpig6dTAfBgNVHSMEGDAWgBTo\nkFvhPZPUAUPgGwFwSnzNpig6dTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEB\nBQUAA4IBAQC8al3bczyOTdMLflrjsY6Su7HEDzemtwgMhtAO++IIfqNoddemk4xM\n+gRjwJrk9W6UjwtUagaSQlFFnGn8SEZsBfrcJcTuxB+BSNw7mU1YmIGE0GnYzJ6U\n/OgM6XLzTCrxtpzVGCtnbTgFWZDE7oLdnv8ALD0AByz8EGGLyQPBiVCD+8Lk/Hic\nSOv+cl/6zkbW4pMgqMYlmPR5JNhx9vG9TEgM9V++W8qZGzdYxEkOLlIq2AV4z1Uz\nl/HLHmqU9o9+6kG3Jq/bhxmSb5IsDuzBFsL/ejogLcRx2l7aPn4Q7/9eFdz/dh6E\nWA0ZtxVcBbrQKKdzTMkazJHqOQ3AZwzL\n-----END CERTIFICATE-----"
}
19 changes: 15 additions & 4 deletions app/src/androidTest/java/com/nextcloud/utils/CertificateValidatorTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import java.io.InputStreamReader
class CertificateValidatorTests {

private var sut: CertificateValidator? = null
private val gson = Gson()

@Before
fun setup() {
Expand All @@ -32,14 +33,24 @@ class CertificateValidatorTests {

@Test
fun testValidateWhenGivenValidServerKeyAndCertificateShouldReturnTrue() {
val isCertificateValid = validateCertificate("credentials.json")
assert(isCertificateValid)
}

@Test
fun testValidateWhenGivenAnotherValidServerKeyAndCertificateShouldReturnTrue() {
val isCertificateValid = validateCertificate("invalid_certs.json")
assert(isCertificateValid)
}

private fun validateCertificate(filename: String): Boolean {
val inputStream =
InstrumentationRegistry.getInstrumentation().context.assets.open("credentials.json")
InstrumentationRegistry.getInstrumentation().context.assets.open(filename)

val credentials = InputStreamReader(inputStream).use { reader ->
Gson().fromJson(reader, Credentials::class.java)
gson.fromJson(reader, Credentials::class.java)
}

val isCertificateValid = sut?.validate(credentials.publicKey, credentials.certificate) ?: false
assert(isCertificateValid)
return sut?.validate(credentials.publicKey, credentials.certificate) ?: false
}
}
64 changes: 27 additions & 37 deletions app/src/main/java/com/owncloud/android/operations/CreateFolderOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private RemoteOperationResult encryptedCreateV2(OCFile parent, OwnCloudClient cl

// check if filename already exists
if (isFileExisting(metadata, filename)) {
return new RemoteOperationResult(RemoteOperationResult.ResultCode.FOLDER_ALREADY_EXISTS);
return new RemoteOperationResult<>(RemoteOperationResult.ResultCode.FOLDER_ALREADY_EXISTS);
}

// generate new random file name, check if it exists in metadata
Expand All @@ -279,9 +279,9 @@ private RemoteOperationResult encryptedCreateV2(OCFile parent, OwnCloudClient cl
token)
.execute(client);

String remoteId = result.getResultData();

if (result.isSuccess()) {
String remoteId = result.getResultData();

DecryptedFolderMetadataFile subFolderMetadata = encryptionUtilsV2.createDecryptedFolderMetadataFile();

// upload metadata
Expand All @@ -294,9 +294,7 @@ private RemoteOperationResult encryptedCreateV2(OCFile parent, OwnCloudClient cl
user,
parent,
getStorageManager());
}

if (result.isSuccess()) {
// update metadata
DecryptedFolderMetadataFile updatedMetadataFile = encryptionUtilsV2.addFolderToMetadata(encryptedFileName,
filename,
Expand All @@ -314,48 +312,43 @@ private RemoteOperationResult encryptedCreateV2(OCFile parent, OwnCloudClient cl
user,
getStorageManager());

// unlock folder
RemoteOperationResult unlockFolderResult = EncryptionUtils.unlockFolder(parent, client, token);

if (unlockFolderResult.isSuccess()) {
token = null;
} else {
// TODO E2E: do better
throw new RuntimeException("Could not unlock folder!");
}

RemoteOperationResult remoteFolderOperationResult = new ReadFolderRemoteOperation(encryptedRemotePath)
final var remoteFolderOperationResult = new ReadFolderRemoteOperation(encryptedRemotePath)
.execute(client);

createdRemoteFolder = (RemoteFile) remoteFolderOperationResult.getData().get(0);
OCFile newDir = createRemoteFolderOcFile(parent, filename, createdRemoteFolder);
getStorageManager().saveFile(newDir);
if (remoteFolderOperationResult.isSuccess()) {
createdRemoteFolder = (RemoteFile) remoteFolderOperationResult.getData().get(0);
OCFile newDir = createRemoteFolderOcFile(parent, filename, createdRemoteFolder);
getStorageManager().saveFile(newDir);

RemoteOperationResult encryptionOperationResult = new ToggleEncryptionRemoteOperation(
newDir.getLocalId(),
newDir.getRemotePath(),
true)
.execute(client);
final var encryptionOperationResult = new ToggleEncryptionRemoteOperation(
newDir.getLocalId(),
newDir.getRemotePath(),
true)
.execute(client);

if (!encryptionOperationResult.isSuccess()) {
throw new RuntimeException("Error creating encrypted subfolder!");
if (!encryptionOperationResult.isSuccess()) {
throw new RuntimeException("Error creating encrypted subfolder!");
}
}
} else {
// revert to sane state in case of any error
Log_OC.e(TAG, remotePath + " hasn't been created");
}

return result;
} catch (Exception e) {
// TODO remove folder
if (token != null) {
final var unlockFolderResult = EncryptionUtils.unlockFolder(parent, client, token);

if (!EncryptionUtils.unlockFolder(parent, client, token).isSuccess()) {
throw new RuntimeException("Could not clean up after failing folder creation!", e);
if (!unlockFolderResult.isSuccess()) {
throw new RuntimeException("Could not unlock folder!");
} else {
token = null;
}
}

// remove folder
if (encryptedRemotePath != null) {
RemoteOperationResult removeResult = new RemoveRemoteEncryptedFileOperation(encryptedRemotePath,
final var removeResult = new RemoveRemoteEncryptedFileOperation(encryptedRemotePath,
user,
context,
filename,
Expand All @@ -367,16 +360,13 @@ private RemoteOperationResult encryptedCreateV2(OCFile parent, OwnCloudClient cl
}
}

// TODO E2E: do better
return new RemoteOperationResult(e);
return new RemoteOperationResult<>(e);
} finally {
// unlock folder
if (token != null) {
RemoteOperationResult unlockFolderResult = EncryptionUtils.unlockFolder(parent, client, token);
final var unlockFolderResult = EncryptionUtils.unlockFolder(parent, client, token);

if (!unlockFolderResult.isSuccess()) {
// TODO E2E: do better
throw new RuntimeException("Could not unlock folder!");
Log_OC.d(TAG, "Could not unlock folder!");
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/java/com/owncloud/android/operations/RefreshFolderOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,8 @@ private void synchronizeData(List<Object> folderAndFiles) {

if (CapabilityUtils.getCapability(mContext).getEndToEndEncryptionApiVersion().compareTo(E2EVersion.V2_0) >= 0) {
if (encryptedAncestor && object == null) {
throw new IllegalStateException("metadata is null!");
Log_OC.d(TAG,"metadata is null!");
return;
}
}

Expand Down
1 change: 1 addition & 0 deletions app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,7 @@ public void onRemoteOperationFinish(RemoteOperation operation, RemoteOperationRe
onSynchronizeFileOperationFinish((SynchronizeFileOperation) operation, result);
} else if (operation instanceof CreateFolderOperation) {
onCreateFolderOperationFinish((CreateFolderOperation) operation, result);
syncAndUpdateFolder(true);
} else if (operation instanceof MoveFileOperation) {
onMoveFileOperationFinish((MoveFileOperation) operation, result);
} else if (operation instanceof CopyFileOperation) {
Expand Down
5 changes: 0 additions & 5 deletions ...main/java/com/owncloud/android/ui/dialog/setupEncryption/SetupEncryptionDialogFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,6 @@ class SetupEncryptionDialogFragment : DialogFragment(), Injectable {

if (isCertificateValid == false) {
Log_OC.d(TAG, "Could not save certificate, certificate is not valid")
return null
}

if (arbitraryDataProvider == null) {
return null
}

arbitraryDataProvider?.storeOrUpdateKeyValue(
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1820,7 +1820,7 @@ private void encryptFolder(OCFile folder,
Log_OC.d(TAG, "encrypt folder " + folder.getRemoteId());
User user = accountManager.getUser();
OwnCloudClient client = clientFactory.create(user);
RemoteOperationResult remoteOperationResult = new ToggleEncryptionRemoteOperation(localId,
final var remoteOperationResult = new ToggleEncryptionRemoteOperation(localId,
remotePath,
shouldBeEncrypted)
.execute(client);
Expand Down
14 changes: 9 additions & 5 deletions app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.gson.GsonBuilder;
import com.google.gson.reflect.TypeToken;
import com.nextcloud.client.account.User;
import com.nextcloud.common.SessionTimeOut;
import com.owncloud.android.R;
import com.owncloud.android.datamodel.ArbitraryDataProvider;
import com.owncloud.android.datamodel.ArbitraryDataProviderImpl;
Expand Down Expand Up @@ -408,9 +409,9 @@ public static DecryptedFolderMetadataFileV1 decryptFolderMetaData(EncryptedFolde
downloadFolderMetadata(OCFile folder,
OwnCloudClient client,
Context context,
User user
) {
RemoteOperationResult<MetadataResponse> getMetadataOperationResult = new GetMetadataRemoteOperation(folder.getLocalId())
User user) {

RemoteOperationResult<MetadataResponse> getMetadataOperationResult = new GetMetadataRemoteOperation(folder.getLocalId(), longSessionTimeOut)
.execute(client);

if (!getMetadataOperationResult.isSuccess()) {
Expand Down Expand Up @@ -1320,7 +1321,8 @@ public static String lockFolder(ServerFileInterface parentFile, OwnCloudClient c
public static String lockFolder(ServerFileInterface parentFile, OwnCloudClient client, long counter) throws UploadException {
// Lock folder
LockFileRemoteOperation lockFileOperation = new LockFileRemoteOperation(parentFile.getLocalId(),
counter);
counter,
longSessionTimeOut);
RemoteOperationResult<String> lockFileOperationResult = lockFileOperation.execute(client);

if (lockFileOperationResult.isSuccess() &&
Expand Down Expand Up @@ -1507,9 +1509,11 @@ public static void uploadMetadata(ServerFileInterface parentFile,
}
}

private static SessionTimeOut longSessionTimeOut = new SessionTimeOut(60000, 60000);

public static RemoteOperationResult<Void> unlockFolder(ServerFileInterface parentFolder, OwnCloudClient client, String token) {
if (token != null) {
return new UnlockFileRemoteOperation(parentFolder.getLocalId(), token).execute(client);
return new UnlockFileRemoteOperation(parentFolder.getLocalId(), token, longSessionTimeOut).execute(client);
} else {
return new RemoteOperationResult<>(new Exception("No token available"));
}
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/
buildscript {
ext {
androidLibraryVersion ="a4d86ef9d1"
androidLibraryVersion ="5f21c760d4"
androidPluginVersion = '8.5.2'
androidxMediaVersion = '1.4.0'
androidxTestVersion = "1.6.1"
Expand Down
5 changes: 4 additions & 1 deletion gradle/verification-metadata.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@
<trusted-key id="BC87A3FD0A54480F0BADBEBD21939FF0CA2A6567" group="commons-codec"/>
<trusted-key id="BCC135FC7ED8214F823D73E97FE9900F412D622E" group="com.google.flatbuffers" name="flatbuffers-java" version="1.12.0"/>
<trusted-key id="BD5C053B4D49EED4ECDBB48C04C39AD208277F6A" group="io.github.elye" name="loaderviewlibrary" version="3.0.0"/>
<trusted-key id="BDB5FA4FE719D787FB3D3197F6D4A1D411E9D1AE" group="com.google.guava"/>
<trusted-key id="BDB5FA4FE719D787FB3D3197F6D4A1D411E9D1AE">
<trusting group="com.google.guava"/>
<trusting group="com.google.j2objc" name="j2objc-annotations" version="3.0.0"/>
</trusted-key>
<trusted-key id="BFD244AF9E85F6ABDCA2B65CDE61FB98F06CE8AE" group="org.json4s"/>
<trusted-key id="C0612048F3393B80B22639B4F067A2FD751AE3E4" group="io.github.davidburstrom.contester" name="contester-breakpoint" version="0.2.0"/>
<trusted-key id="C1CBA75EC9BD0BAF8061935459E05CE618187ED4" group="org.xerial" name="sqlite-jdbc" version="3.41.2.2"/>
Expand Down