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.

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

#2396 Fix: choose correct keys for message encryption #2413

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions FlowCrypt/Controllers/Compose/ComposeViewDecorator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ struct ComposeViewDecorator {
let recipientIdleState: RecipientState = .idle(idleStateContext)
let recipientKeyFoundState: RecipientState = .keyFound(keyFoundStateContext)
let recipientKeyExpiredState: RecipientState = .keyExpired(keyExpiredStateContext)
let recipientKeyNotUsableForEncryptionState: RecipientState = .keyNotUsableForEncryption(keyNotUsableForEncryptionStateContext)
let recipientKeyNotUsableForSigningState: RecipientState = .keyNotUsableForSigning(keyNotUsableForSigningStateContext)
let recipientKeyRevokedState: RecipientState = .keyRevoked(keyRevokedStateContext)
let recipientKeyNotFoundState: RecipientState = .keyNotFound(keyNotFoundStateContext)
let recipientInvalidEmailState: RecipientState = .invalidEmail(invalidEmailStateContext)
Expand Down Expand Up @@ -262,6 +264,26 @@ extension ComposeViewDecorator {
)
}

private static var keyNotUsableForEncryptionStateContext: RecipientStateContext {
RecipientStateContext(
backgroundColor: .darkYellowColor,
borderColor: .borderColor,
textColor: .white,
image: nil,
accessibilityIdentifier: "yellow"
)
}

private static var keyNotUsableForSigningStateContext: RecipientStateContext {
RecipientStateContext(
backgroundColor: .darkYellowColor,
borderColor: .borderColor,
textColor: .white,
image: nil,
accessibilityIdentifier: "yellow"
)
}

private static var keyExpiredStateContext: RecipientStateContext {
RecipientStateContext(
backgroundColor: .warningColor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ extension ComposeViewController {
return decorator.recipientKeyFoundState
case .expired:
return decorator.recipientKeyExpiredState
case .notUsableForEncryption:
return decorator.recipientKeyNotUsableForEncryptionState
case .notUsableForSigning:
return decorator.recipientKeyNotUsableForSigningState
case .revoked:
return decorator.recipientKeyRevokedState
case .empty:
Expand Down Expand Up @@ -163,7 +167,7 @@ extension ComposeViewController {
switch recipient.state {
case .idle:
handleRecipientSelection(with: indexPath, type: type)
case .keyFound, .keyExpired, .keyRevoked, .keyNotFound, .invalidEmail:
case .keyFound, .keyExpired, .keyRevoked, .keyNotFound, .invalidEmail, .keyNotUsableForEncryption, .keyNotUsableForSigning:
break
case let .error(_, isRetryError):
if isRetryError {
Expand Down
4 changes: 4 additions & 0 deletions FlowCrypt/Extensions/UIColorExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ public extension UIColor {
)
}

static var darkYellowColor: UIColor {
UIColor(r: 191, g: 181, b: 0)
}

static var warningColor: UIColor {
UIColor(r: 194, g: 126, b: 35)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ final class EncryptedStorage: EncryptedStorageType {
case version13
case version14
case version15
case version16

var version: SchemaVersion {
switch self {
Expand Down Expand Up @@ -78,14 +79,16 @@ final class EncryptedStorage: EncryptedStorageType {
return SchemaVersion(appVersion: "1.2.3", dbSchemaVersion: 14)
case .version15:
return SchemaVersion(appVersion: "1.2.3", dbSchemaVersion: 15)
case .version16:
return SchemaVersion(appVersion: "1.2.3", dbSchemaVersion: 16)
}
}
}

private lazy var migrationLogger = Logger.nested(in: Self.self, with: .migration)
private lazy var logger = Logger.nested(Self.self)

private let currentSchema: EncryptedStorageSchema = .version15
private let currentSchema: EncryptedStorageSchema = .version16
private let supportedSchemas = EncryptedStorageSchema.allCases

private let storageEncryptionKey: Data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ enum MessageValidationError: Error, CustomStringConvertible, Equatable {
case noPubRecipients
case revokedKeyRecipients
case expiredKeyRecipients
case notUsableForEncryptionKeyRecipients
case invalidEmailRecipient
case internalError(String)

Expand All @@ -44,6 +45,8 @@ enum MessageValidationError: Error, CustomStringConvertible, Equatable {
return "compose_recipient_revoked".localized
case .expiredKeyRecipients:
return "compose_recipient_expired".localized
case .notUsableForEncryptionKeyRecipients:
return "compose_recipient_unusuable_for_encryption".localized
case .invalidEmailRecipient:
return "compose_recipient_invalid_email".localized
case let .internalError(message):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ final class ComposeMessageHelper {
throw MessageValidationError.noPubRecipients
}

guard !contains(keyState: .notUsableForEncryption) else {
throw MessageValidationError.notUsableForEncryptionKeyRecipients
}
guard !contains(keyState: .expired) else {
throw MessageValidationError.expiredKeyRecipients
}
Expand Down
21 changes: 18 additions & 3 deletions FlowCrypt/Models/Contact Models/PubKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ struct PubKey {
let algo: KeyAlgo?
/// is key revoked
let isRevoked: Bool
/// key usable for encryption
let usableForEncryption: Bool
/// key usable for signing
let usableForSigning: Bool
/// user emails
let emails: [String]
}
Expand All @@ -38,8 +42,15 @@ extension PubKey {
var fingerprint: String? { fingerprints.first }

var keyState: PubKeyState {
guard !isRevoked else { return .revoked }

if isRevoked {
return .revoked
}
if !usableForEncryption {
return .notUsableForEncryption
}
if !usableForSigning {
return .notUsableForSigning
}
guard let expiresOn,
expiresOn.timeIntervalSinceNow.sign == .minus
else { return .active }
Expand All @@ -65,6 +76,8 @@ extension PubKey {
created: Date(timeIntervalSince1970: Double(keyDetails.created)),
algo: keyDetails.algo,
isRevoked: keyDetails.revoked,
usableForEncryption: keyDetails.usableForEncryption,
usableForSigning: keyDetails.usableForSigning,
emails: keyDetails.pgpUserEmails
)
}
Expand All @@ -82,7 +95,9 @@ extension PubKey {
self.created = object.created

self.algo = nil
self.isRevoked = false
self.isRevoked = object.isRevoked
self.usableForEncryption = object.usableForEncryption
self.usableForSigning = object.usableForSigning
self.emails = []
}
}
Expand Down
2 changes: 1 addition & 1 deletion FlowCrypt/Models/Contact Models/PubKeyState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
import Foundation

enum PubKeyState {
case active, expired, revoked, empty
case active, expired, revoked, empty, notUsableForEncryption, notUsableForSigning
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ extension RecipientWithSortedPubKeys {
guard !key1.isRevoked else { return false }
// check if key2 is revoked
guard !key2.isRevoked else { return true }
// check if key1 is usable for encryption
guard key1.usableForEncryption else { return false }
// check if key2 is usable for encryption
guard key2.usableForEncryption else { return true }
// check if key1 is usable for signing
guard key1.usableForSigning else { return false }
// check if key2 is usable for signing
guard key2.usableForSigning else { return true }
// check if key1 never expires
guard let expire1 = key1.expiresOn else { return true }
// check if key2 never expires
Expand Down
15 changes: 13 additions & 2 deletions FlowCrypt/Models/Realm Models/PubKeyRealmObject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ final class PubKeyRealmObject: Object {
@Persisted var fingerprints: List<String>
@Persisted var created: Date?
@Persisted var isRevoked = false
@Persisted var usableForEncryption = false
@Persisted var usableForSigning = false
}

extension PubKeyRealmObject {
Expand All @@ -32,7 +34,9 @@ extension PubKeyRealmObject {
longids: [String] = [],
fingerprints: [String] = [],
created: Date? = nil,
isRevoked: Bool) throws {
isRevoked: Bool,
usableForEncryption: Bool,
usableForSigning: Bool) throws {
self.init()

self.armored = armored
Expand All @@ -41,6 +45,8 @@ extension PubKeyRealmObject {
self.expiresOn = expiresOn
self.created = created
self.isRevoked = isRevoked
self.usableForEncryption = usableForEncryption
self.usableForSigning = usableForSigning

self.longids.append(objectsIn: longids)
self.fingerprints.append(objectsIn: fingerprints)
Expand All @@ -63,7 +69,9 @@ extension PubKeyRealmObject {
longids: key.longids,
fingerprints: key.fingerprints,
created: key.created,
isRevoked: key.isRevoked
isRevoked: key.isRevoked,
usableForEncryption: key.usableForEncryption,
usableForSigning: key.usableForSigning
)
}
}
Expand All @@ -75,6 +83,9 @@ extension PubKeyRealmObject {
self.lastChecked = key.lastChecked
self.expiresOn = key.expiresOn
self.created = key.created
self.isRevoked = key.isRevoked
self.usableForEncryption = key.usableForEncryption
self.usableForSigning = key.usableForSigning

let longids = List<String>()
longids.append(objectsIn: key.longids)
Expand Down
1 change: 1 addition & 0 deletions FlowCrypt/Resources/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
"compose_recipient_no_pub" = "One or more of your recipients are missing a public key (marked in gray).\n\nPlease ask them to share it with you, or ask them to also set up FlowCrypt.";
"compose_recipient_revoked" = "One or more of your recipients have revoked public keys (marked in red).\n\nPlease ask them to send you a new public key. If this is an enterprise installation, please ask your systems admin.";
"compose_recipient_expired" = "One or more of your recipients have expired public keys (marked in orange).\n\nPlease ask them to send you updated public key. If this is an enterprise installation, please ask your systems admin.";
"compose_recipient_unusuable_for_encryption" = "One or more of your recipients have sign-only public keys (marked in yellow).\n\nPlease ask them to send you updated public key. If this is an enterprise installation, please ask your systems admin.";
"compose_recipient_invalid_email" = "One or more of your recipients have invalid email address (marked in red)";
"compose_password_weak" = "Password didn't comply with company policy, which requires at least:\n\n- one uppercase\n- one lowercase\n- one number\n- one special character eg &/#\"-'_%-@,;:!*()\n- 8 characters length\n\nPlease update the password and re-send.";
"compose_password_passphrase_error" = "Please do not use your private key pass phrase as a password for this message.\n\nYou should come up with some other unique password that you can share with recipient.";
Expand Down
2 changes: 1 addition & 1 deletion FlowCryptUI/Cell Nodes/RecipientEmailNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ final class RecipientEmailNode: CellNode {
switch input.recipient.state {
case .idle: self.animateImageRotation()
case .error: self.animateImageScaling()
case .keyFound, .keyExpired, .keyRevoked, .keyNotFound, .invalidEmail:
case .keyFound, .keyExpired, .keyRevoked, .keyNotFound, .invalidEmail, .keyNotUsableForEncryption, .keyNotUsableForSigning:
break
}
}
Expand Down
12 changes: 12 additions & 0 deletions FlowCryptUI/Cell Nodes/RecipientEmailsCellNodeInput.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public extension RecipientEmailsCellNode {
case idle(StateContext)
case keyFound(StateContext)
case keyExpired(StateContext)
case keyNotUsableForEncryption(StateContext)
case keyNotUsableForSigning(StateContext)
case keyRevoked(StateContext)
case keyNotFound(StateContext)
case invalidEmail(StateContext)
Expand All @@ -47,6 +49,8 @@ public extension RecipientEmailsCellNode {
case let .idle(context),
let .keyFound(context),
let .keyExpired(context),
let .keyNotUsableForEncryption(context),
let .keyNotUsableForSigning(context),
let .keyRevoked(context),
let .keyNotFound(context),
let .invalidEmail(context),
Expand Down Expand Up @@ -97,6 +101,12 @@ public extension RecipientEmailsCellNode {
case var .keyExpired(context):
context.isSelected = newValue
self = .keyExpired(context)
case var .keyNotUsableForEncryption(context):
context.isSelected = newValue
self = .keyNotUsableForEncryption(context)
case var .keyNotUsableForSigning(context):
context.isSelected = newValue
self = .keyNotUsableForSigning(context)
case var .keyRevoked(context):
context.isSelected = newValue
self = .keyRevoked(context)
Expand All @@ -118,6 +128,8 @@ public extension RecipientEmailsCellNode {
case .idle: return "idle"
case .keyFound: return "keyFound"
case .keyExpired: return "keyExpired"
case .keyNotUsableForEncryption: return "keyNotUsableForEncryption"
case .keyNotUsableForSigning: return "keyNotUsableForSigning"
case .keyRevoked: return "keyRevoked"
case .keyNotFound: return "keyNotFound"
case .invalidEmail: return "invalidEmail"
Expand Down
23 changes: 22 additions & 1 deletion appium/api-mocks/mock-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export type MockUser = {
pub?: string;
pubNew?: string;
pubOther?: string;
pubSignOnly?: string;
};

export type MockUserAlias = {
Expand All @@ -26,7 +27,8 @@ export type MockUserName =
| 'expired'
| 'revoked'
| 'sunit'
| 'ioan';
| 'ioan'
| 'signOnlyKey';

export const MockUserList: Record<MockUserName, MockUser> = {
e2e: {
Expand Down Expand Up @@ -349,6 +351,25 @@ i0Bd7uutnnkRdCDPTvY5ub4ZDrHGAQCFIYc+Mp6zZdR1s/3kIpjrcg6mOtmj
7Xox/a0FLLQsCQ==
=+AGT
-----END PGP PUBLIC KEY BLOCK-----`,
pubSignOnly: `-----BEGIN PGP PUBLIC KEY BLOCK-----

mQENBGU/ys4BCADvxv0RSWGalR4/xD+axR4tLlgUxaZWYg6Yd6s8xAWsZ/ygZCAW
RaLplLa33uqbtuK2KLZMk8VdtmkmlAN3L9qMLwG9eVPlqjVAROsbJNTil5jBKeTI
OaK98Lx2WML/J7lZHZZmlDWQsWBZUDKagQKn2uqVhI/A+uzDy0SLBZAqhgBBM44P
iwJiIx0TJJ/CBEjZDJ8yzg+Aa0oEGh9uULwwA21wES9t8UZ/s0FxyPbpL3Th1ovN
Cs3cpLPqkbOY6REMMAb5pdDLM0KtXjTST34bHNvFdDLhldY69XL1x7D4wvVvUFpc
u7wrwnTqPgb8SC+GF3SH3nVJjQQ5Qnolc6VlABEBAAG0JUZsb3dDcnlwdCBSb2Jv
dCA8cm9ib3RAZmxvd2NyeXB0LmNvbT6JAVQEEwEIAD4WIQTT3thevVcV8N5aA7W0
KflQf/+crQUCZT/KzgIbAwUJB4YfXAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAK
CRC0KflQf/+crS8JCAC44zw7JkyYPjBW17NDdq+VeiD8qasO6qOzmt+y1wI0a/Ve
cpk4jGZO5XoWGgRrDuppS4bPumXcMzQVO6z0YKain51j51Rah4WC0eS6fJU6+vLZ
foKw1nKVgfp4ezwCZp3fL2/J66F3lqpM5Shj4u3kd9qeumh/CqJzgE1ld0Urk/lc
9fkM4BBBf5enQ4mT0wHZfi/nawg7zfy7moJNqRkGp+TNmK3N4I0RMA+fPl4OPdv7
H6xmypVPdCZmZCDBeGr3n9cR/qz5OEvrDUb0fTdvpf2NGYW0Mco5bprtOIvtRhje
/s+m3hBbMBOaM6v+JFPtm8SjqjvLMuYVnvnXwyFp
=kQbU
-----END PGP PUBLIC KEY BLOCK-----
`,
},
expired: {
email: '[email protected]',
Expand Down
7 changes: 7 additions & 0 deletions appium/tests/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@ export const CommonData = {
'Please ask them to share it with you, or ask them to also set up FlowCrypt.',
wrongPassPhrase:
'Error\n' + 'Could not compose message\n' + '\n' + 'This pass phrase did not match your signing private key.',
notUsableEncryptionPublicKey:
'Error\n' +
'Could not compose message\n' +
'\n' +
'One or more of your recipients have sign-only public keys (marked in yellow).\n' +
'\n' +
'Please ask them to send you updated public key. If this is an enterprise installation, please ask your systems admin.',
expiredPublicKey:
'Error\n' +
'Could not compose message\n' +
Expand Down
Loading
Loading