Skip to content

[Collections] Tighten import conditions #3296

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

Merged
merged 3 commits into from
Feb 25, 2021
Merged

[Collections] Tighten import conditions #3296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52559f8
[Collections] Tighten import conditions
yim-lee Feb 20, 2021
497edc2
platform check
yim-lee Feb 20, 2021
c3089f9
Slightly nicer way to skip tests
yim-lee Feb 24, 2021
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
2 changes: 2 additions & 0 deletions Sources/PackageCollections/API.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,6 @@ public enum PackageCollectionError: Equatable, Error {
case invalidSignature

case missingSignature

case unsupportedPlatform
}
91 changes: 68 additions & 23 deletions Sources/PackageCollections/PackageCollections.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ import TSCBasic

// TODO: is there a better name? this conflicts with the module name which is okay in this case but not ideal in Swift
public struct PackageCollections: PackageCollectionsProtocol {
// Check JSONPackageCollectionProvider.isSignatureCheckSupported before updating or removing this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to exclude the entire PackageCollections module in unsupported platforms. not sure if easier / harder but if easy may hep to keep the code simpler and move the complexity there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @neonichu may know of a way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle, we could use conditional dependencies, might need some reorganization because I think right now some of the collections targets are also directly included in products.

#if os(macOS) || os(Linux) || os(Windows)
static let isSupportedPlatform = true
#else
static let isSupportedPlatform = false
#endif

private let configuration: Configuration
private let diagnosticsEngine: DiagnosticsEngine?
private let storageContainer: (storage: Storage, owned: Bool)
Expand Down Expand Up @@ -64,6 +71,10 @@ public struct PackageCollections: PackageCollectionsProtocol {

public func listCollections(identifiers: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<[PackageCollectionsModel.Collection], Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.list { result in
switch result {
case .failure(let error):
Expand Down Expand Up @@ -115,6 +126,10 @@ public struct PackageCollections: PackageCollectionsProtocol {
}

public func refreshCollections(callback: @escaping (Result<[PackageCollectionsModel.CollectionSource], Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.list { result in
switch result {
case .failure(let error):
Expand All @@ -137,17 +152,23 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

public func refreshCollection(
_ source: PackageCollectionsModel.CollectionSource,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void
) {
public func refreshCollection(_ source: PackageCollectionsModel.CollectionSource,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil, callback: callback)
}

public func addCollection(_ source: PackageCollectionsModel.CollectionSource,
order: Int? = nil,
trustConfirmationProvider: ((PackageCollectionsModel.Collection, @escaping (Bool) -> Void) -> Void)? = nil,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

if let errors = source.validate()?.errors() {
return callback(.failure(MultipleErrors(errors)))
}
Expand Down Expand Up @@ -182,6 +203,10 @@ public struct PackageCollections: PackageCollectionsProtocol {

public func removeCollection(_ source: PackageCollectionsModel.CollectionSource,
callback: @escaping (Result<Void, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.remove(source: source) { result in
switch result {
case .failure(let error):
Expand All @@ -195,11 +220,19 @@ public struct PackageCollections: PackageCollectionsProtocol {
public func moveCollection(_ source: PackageCollectionsModel.CollectionSource,
to order: Int,
callback: @escaping (Result<Void, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.move(source: source, to: order, callback: callback)
}

public func updateCollection(_ source: PackageCollectionsModel.CollectionSource,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.update(source: source) { result in
switch result {
case .failure(let error):
Expand All @@ -215,6 +248,10 @@ public struct PackageCollections: PackageCollectionsProtocol {
// If not found locally (storage), the collection will be fetched from the source.
public func getCollection(_ source: PackageCollectionsModel.CollectionSource,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

if let errors = source.validate()?.errors() {
return callback(.failure(MultipleErrors(errors)))
}
Expand All @@ -234,11 +271,13 @@ public struct PackageCollections: PackageCollectionsProtocol {

// MARK: - Packages

public func findPackages(
_ query: String,
collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult, Error>) -> Void
) {
public func findPackages(_ query: String,
collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.storage.sources.list { result in
switch result {
case .failure(let error):
Expand All @@ -257,6 +296,10 @@ public struct PackageCollections: PackageCollectionsProtocol {

public func getPackageMetadata(_ reference: PackageReference,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

// first find in storage
self.findPackage(identifier: reference.identity) { result in
switch result {
Expand Down Expand Up @@ -288,10 +331,12 @@ public struct PackageCollections: PackageCollectionsProtocol {

// MARK: - Targets

public func listTargets(
collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.TargetListResult, Error>) -> Void
) {
public func listTargets(collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.TargetListResult, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

self.listCollections(identifiers: collections) { result in
switch result {
case .failure(let error):
Expand All @@ -303,12 +348,14 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

public func findTargets(
_ query: String,
searchType: PackageCollectionsModel.TargetSearchType? = nil,
collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.TargetSearchResult, Error>) -> Void
) {
public func findTargets(_ query: String,
searchType: PackageCollectionsModel.TargetSearchType? = nil,
collections: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
callback: @escaping (Result<PackageCollectionsModel.TargetSearchResult, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

let searchType = searchType ?? .exactMatch

self.storage.sources.list { result in
Expand Down Expand Up @@ -393,10 +440,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

func findPackage(
identifier: PackageIdentity,
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void
) {
func findPackage(identifier: PackageIdentity,
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void) {
self.storage.sources.list { result in
switch result {
case .failure(let error):
Expand Down
12 changes: 12 additions & 0 deletions Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
// FIXME: remove
static let enableSignatureCheck = ProcessInfo.processInfo.environment["ENABLE_COLLECTION_SIGNATURE_CHECK"] != nil

// TODO: This can be removed when the `Security` framework APIs that the `PackageCollectionsSigning`
// module depends on are available on all Apple platforms.
#if os(macOS) || os(Linux) || os(Windows)
static let isSignatureCheckSupported = true
#else
static let isSignatureCheckSupported = false
#endif

private let configuration: Configuration
private let diagnosticsEngine: DiagnosticsEngine
private let httpClient: HTTPClient
Expand Down Expand Up @@ -136,6 +144,10 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
// Don't validate signature; set isVerified=false
callback(self.makeCollection(from: signedCollection.collection, source: source, signature: Model.SignatureData(from: signedCollection.signature, isVerified: false)))
} else {
if !Self.isSignatureCheckSupported {
fatalError("Unsupported platform")
}

// Check the signature
self.signatureValidator.validate(signedCollection: signedCollection, certPolicyKey: certPolicyKey) { result in
switch result {
Expand Down
6 changes: 3 additions & 3 deletions Sources/PackageCollectionsSigning/Certificate/Certificate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@

import struct Foundation.Data

#if canImport(Security)
#if os(macOS)
import Security
#endif

#if canImport(Security)
#if os(macOS)
typealias Certificate = CoreCertificate
#else
typealias Certificate = BoringSSLCertificate
#endif

// MARK: - Certificate implementation using the Security framework

#if canImport(Security)
#if os(macOS)
struct CoreCertificate {
let underlying: SecCertificate

Expand Down
12 changes: 6 additions & 6 deletions Sources/PackageCollectionsSigning/Certificate/CertificatePolicy.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import struct Foundation.URL

import TSCBasic

#if canImport(Security)
#if os(macOS)
import Security
#endif

Expand Down Expand Up @@ -53,7 +53,7 @@ extension CertificatePolicy {
return wrappedCallback(.failure(CertificatePolicyError.emptyCertChain))
}

#if canImport(Security)
#if os(macOS)
let policy = SecPolicyCreateBasicX509()
let revocationPolicy = SecPolicyCreateRevocation(kSecRevocationOCSPMethod)

Expand Down Expand Up @@ -99,7 +99,7 @@ extension CertificatePolicy {

extension CertificatePolicy {
func hasExtension(oid: String, in certificate: Certificate) throws -> Bool {
#if canImport(Security)
#if os(macOS)
guard let dict = SecCertificateCopyValues(certificate.underlying, [oid as CFString] as CFArray, nil) as? [CFString: Any] else {
throw CertificatePolicyError.extensionFailure
}
Expand All @@ -110,7 +110,7 @@ extension CertificatePolicy {
}

func hasExtendedKeyUsage(_ usage: CertificateExtendedKeyUsage, in certificate: Certificate) throws -> Bool {
#if canImport(Security)
#if os(macOS)
guard let dict = SecCertificateCopyValues(certificate.underlying, [kSecOIDExtendedKeyUsage] as CFArray, nil) as? [CFString: Any] else {
throw CertificatePolicyError.extensionFailure
}
Expand All @@ -127,7 +127,7 @@ extension CertificatePolicy {
/// Checks that the certificate supports OCSP. This **must** be done before calling `verify` to ensure
/// the necessary properties are in place to trigger revocation check.
func supportsOCSP(certificate: Certificate) throws -> Bool {
#if canImport(Security)
#if os(macOS)
// Check that certificate has "Certificate Authority Information Access" extension and includes OCSP as access method.
// The actual revocation check will be done by the Security framework in `verify`.
guard let dict = SecCertificateCopyValues(certificate.underlying, [kSecOIDAuthorityInfoAccess] as CFArray, nil) as? [CFString: Any] else { // ignore error
Expand All @@ -147,7 +147,7 @@ extension CertificatePolicy {
enum CertificateExtendedKeyUsage {
case codeSigning

#if canImport(Security)
#if os(macOS)
var data: Data {
switch self {
case .codeSigning:
Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageCollectionsSigning/Key/Key+EC.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct ECPrivateKey: PrivateKey {
init<Data>(pem data: Data) throws where Data: DataProtocol {
let pem = String(decoding: data, as: UTF8.self)
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
if #available(macOS 11, *) {
if #available(macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0, *) {
self.underlying = try CryptoECPrivateKey(pemRepresentation: pem)
} else {
let pemDocument = try ASN1.PEMDocument(pemString: pem)
Expand All @@ -45,7 +45,7 @@ struct ECPublicKey: PublicKey {
init<Data>(pem data: Data) throws where Data: DataProtocol {
let pem = String(decoding: data, as: UTF8.self)
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
if #available(macOS 11, *) {
if #available(macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0, *) {
self.underlying = try CryptoECPublicKey(pemRepresentation: pem)
} else {
let pemDocument = try ASN1.PEMDocument(pemString: pem)
Expand Down
6 changes: 3 additions & 3 deletions Sources/PackageCollectionsSigning/Key/Key+RSA.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

import Foundation

#if canImport(Security)
#if os(macOS)
import Security
#endif

#if canImport(Security)
#if os(macOS)
typealias RSAPublicKey = CoreRSAPublicKey
typealias RSAPrivateKey = CoreRSAPrivateKey
#else
Expand All @@ -24,7 +24,7 @@ typealias RSAPrivateKey = BoringSSLRSAPrivateKey

// MARK: - RSA key implementations using the Security framework

#if canImport(Security)
#if os(macOS)
struct CoreRSAPrivateKey: PrivateKey {
let underlying: SecKey

Expand Down
4 changes: 2 additions & 2 deletions Sources/PackageCollectionsSigning/Signing/Signing+RSAKey.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@

import struct Foundation.Data

#if canImport(Security)
#if os(macOS)
import Security
#endif

// MARK: - MessageSigner and MessageValidator conformance using the Security framework

#if canImport(Security)
#if os(macOS)
extension CoreRSAPrivateKey {
func sign(message: Data) throws -> Data {
var error: Unmanaged<CFError>?
Expand Down
Loading