-
Notifications
You must be signed in to change notification settings - Fork 134
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.
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
RUMM-1870 Add data encryption interface #797
Merged
Merged
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e07a1b8
RUMM-1870 Add data encryption interface
maxep 1373bc3
RUMM-1870 Update CHANGELOG
maxep 210571d
RUMM-1870 Fix FeaturesCommonDependencies mocks
maxep bb6b228
RUMM-1870 Add encryption in feature tests
maxep 040bf39
RUMM-1870 Improve documentation
maxep 8851d46
RUMM-1870 Report decryption failure
maxep File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* | ||
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
* This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
* Copyright 2019-2022 Datadog, Inc. | ||
*/ | ||
|
||
import Foundation | ||
|
||
/// Interface that allows storing data in encrypted format. Encryption/decryption round should | ||
/// return exactly the same data as it given for the encryption originally (even if decryption | ||
/// happens in another process/app launch). | ||
public protocol DataEncryption { | ||
/// Encrypts given `Data` with user-chosen encryption. | ||
/// | ||
/// - Parameter data: Data to encrypt. | ||
/// - Returns: The encrypted data. | ||
func encrypt(data: Data) throws -> Data | ||
|
||
/// Decrypts given `Data` with user-chosen encryption. | ||
/// | ||
/// Beware that data to decrypt could be encrypted in a previous | ||
/// app launch, so implementation should be aware of the case when decryption could | ||
/// fail (for example, key used for encryption is different from key used for decryption, if | ||
/// they are unique for every app launch). | ||
/// | ||
/// - Parameter data: Data to decrypt. | ||
/// - Returns: The decrypted data. | ||
func decrypt(data: Data) throws -> Data | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,17 +13,17 @@ internal struct DataFormat { | |
/// Suffixes the batch payload read from file. | ||
let suffixData: Data | ||
/// Separates entities written to file. | ||
let separatorData: Data | ||
let separatorByte: UInt8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enforce single byte separator for data splitting during decryption. |
||
|
||
// MARK: - Initialization | ||
|
||
init( | ||
prefix: String, | ||
suffix: String, | ||
separator: String | ||
separator: Character | ||
) { | ||
self.prefixData = prefix.data(using: .utf8)! // swiftlint:disable:this force_unwrapping | ||
self.suffixData = suffix.data(using: .utf8)! // swiftlint:disable:this force_unwrapping | ||
self.separatorData = separator.data(using: .utf8)! // swiftlint:disable:this force_unwrapping | ||
self.separatorByte = separator.asciiValue! // swiftlint:disable:this force_unwrapping | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,42 +12,63 @@ internal final class FileReader: Reader { | |
private let dataFormat: DataFormat | ||
/// Orchestrator producing reference to readable file. | ||
private let orchestrator: FilesOrchestrator | ||
private let encryption: DataEncryption? | ||
private let internalMonitor: InternalMonitor? | ||
|
||
/// Files marked as read. | ||
private var filesRead: [ReadableFile] = [] | ||
private var filesRead: Set<String> = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor refacto |
||
|
||
init( | ||
dataFormat: DataFormat, | ||
orchestrator: FilesOrchestrator, | ||
encryption: DataEncryption? = nil, | ||
internalMonitor: InternalMonitor? = nil | ||
) { | ||
self.dataFormat = dataFormat | ||
self.orchestrator = orchestrator | ||
self.encryption = encryption | ||
self.internalMonitor = internalMonitor | ||
} | ||
|
||
// MARK: - Reading batches | ||
|
||
func readNextBatch() -> Batch? { | ||
if let file = orchestrator.getReadableFile(excludingFilesNamed: Set(filesRead.map { $0.name })) { | ||
do { | ||
let fileData = try file.read() | ||
let batchData = dataFormat.prefixData + fileData + dataFormat.suffixData | ||
return Batch(data: batchData, file: file) | ||
} catch { | ||
internalMonitor?.sdkLogger.error("Failed to read data from file", error: error) | ||
return nil | ||
} | ||
guard let file = orchestrator.getReadableFile(excludingFilesNamed: filesRead) else { | ||
return nil | ||
} | ||
|
||
return nil | ||
do { | ||
let fileData = try decode(data: file.read()) | ||
let batchData = dataFormat.prefixData + fileData + dataFormat.suffixData | ||
return Batch(data: batchData, file: file) | ||
} catch { | ||
internalMonitor?.sdkLogger.error("Failed to read data from file", error: error) | ||
return nil | ||
} | ||
} | ||
|
||
func decode(data: Data) -> Data { | ||
maxep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
guard let encryption = encryption else { | ||
return data | ||
} | ||
|
||
return data | ||
// split data | ||
.split(separator: dataFormat.separatorByte) | ||
// decode base64 - allow failure | ||
.compactMap { Data(base64Encoded: $0) } | ||
// decrypt data - allow failure | ||
.compactMap { try? encryption.decrypt(data: $0) } | ||
maxep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// concat data | ||
.reduce(Data()) { $0 + $1 + [dataFormat.separatorByte] } | ||
// drop last separator | ||
.dropLast() | ||
} | ||
|
||
// MARK: - Accepting batches | ||
|
||
func markBatchAsRead(_ batch: Batch) { | ||
orchestrator.delete(readableFile: batch.file) | ||
filesRead.append(batch.file) | ||
filesRead.insert(batch.file.name) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be part of
Unreleased
section along with "Web-view tracking" which is already part1.10.0-beta1
? I think that something is out-of-sync about theCHANGELOG.md
, no? IMO the1.10.0-beta1
release should be listed here as regular release - otherwise we will lose track on what's inside1.10.0
and what not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently don't add pre-release tag in the changelog, it is part of our release-process. I also think we should add pre-release sections tho! lets chat for updating the process before the next version.