Skip to content

Expected signing entity verification #6359

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 1 commit into from
Apr 3, 2023
Merged

Expected signing entity verification #6359

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
2 changes: 1 addition & 1 deletion Sources/PackageModel/RegistryReleaseMetadata.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public struct RegistryReleaseMetadata {
}
}

public enum SigningEntity: Codable {
public enum SigningEntity: Codable, Equatable {
case recognized(type: String, commonName: String?, organization: String?, identity: String?)
case unrecognized(commonName: String?, organization: String?)
}
Expand Down
9 changes: 7 additions & 2 deletions Sources/SPMTestSupport/MockPackage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public struct MockPackage {
platforms: [PlatformDescription] = [],
identity: String,
alternativeURLs: [String]? = .none,
metadata: RegistryReleaseMetadata? = .none,
targets: [MockTarget],
products: [MockProduct],
dependencies: [MockDependency] = [],
Expand All @@ -85,7 +86,11 @@ public struct MockPackage {
) {
self.name = name
self.platforms = platforms
self.location = .registry(identity: .plain(identity), alternativeURLs: alternativeURLs?.compactMap{ URL(string: $0) })
self.location = .registry(
identity: .plain(identity),
alternativeURLs: alternativeURLs?.compactMap{ URL(string: $0) },
metadata: metadata
)
self.targets = targets
self.products = products
self.dependencies = dependencies
Expand All @@ -110,6 +115,6 @@ public struct MockPackage {
public enum Location {
case fileSystem(path: RelativePath)
case sourceControl(url: URL)
case registry(identity: PackageIdentity, alternativeURLs: [URL]?)
case registry(identity: PackageIdentity, alternativeURLs: [URL]?, metadata: RegistryReleaseMetadata?)
}
}
13 changes: 11 additions & 2 deletions Sources/SPMTestSupport/MockWorkspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,15 @@ public final class MockWorkspace {
} else {
packagePath = basePath.appending(components: "sourceControl", url.absoluteString.spm_mangledToC99ExtendedIdentifier())
}
case .registry(let identity, _):
case .registry(let identity, _, let metadata):
packagePath = basePath.appending(components: "registry", identity.description.spm_mangledToC99ExtendedIdentifier())

// Write registry release metadata if the mock package provided it.
if let metadata = metadata {
try self.fileSystem.createDirectory(packagePath, recursive: true)
let path = packagePath.appending(component: RegistryReleaseMetadataStorage.fileName)
try RegistryReleaseMetadataStorage.save(metadata, to: path, fileSystem: self.fileSystem)
}
}

let packageLocation: String
Expand All @@ -177,7 +184,7 @@ public final class MockWorkspace {
packageLocation = url.absoluteString
packageKind = .remoteSourceControl(url)
sourceControlSpecifier = RepositorySpecifier(url: url)
case (_, .registry(let identity, let alternativeURLs)):
case (_, .registry(let identity, let alternativeURLs, _)):
packageLocation = identity.description
packageKind = .registry(identity)
registryIdentity = identity
Expand Down Expand Up @@ -437,6 +444,7 @@ public final class MockWorkspace {
roots: [String] = [],
dependencies: [PackageDependency] = [],
forceResolvedVersions: Bool = false,
expectedSigningEntities: [PackageIdentity: RegistryReleaseMetadata.SigningEntity] = [:],
_ result: (PackageGraph, [Basics.Diagnostic]) throws -> Void
) throws {
let observability = ObservabilitySystem.makeForTesting()
Expand All @@ -448,6 +456,7 @@ public final class MockWorkspace {
let graph = try workspace.loadPackageGraph(
rootInput: rootInput,
forceResolvedVersions: forceResolvedVersions,
expectedSigningEntities: expectedSigningEntities,
observabilityScope: observability.topScope
)
try result(graph, observability.diagnostics)
Expand Down
54 changes: 53 additions & 1 deletion Sources/Workspace/Workspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,7 @@ extension Workspace {
forceResolvedVersions: Bool = false,
customXCTestMinimumDeploymentTargets: [PackageModel.Platform: PlatformVersion]? = .none,
testEntryPointPath: AbsolutePath? = nil,
expectedSigningEntities: [PackageIdentity: RegistryReleaseMetadata.SigningEntity] = [:],
observabilityScope: ObservabilityScope
) throws -> PackageGraph {
// reload state in case it was modified externally (eg by another process) before reloading the graph
Expand Down Expand Up @@ -1104,7 +1105,7 @@ extension Workspace {
}

// Load the graph.
return try PackageGraph.load(
let packageGraph = try PackageGraph.load(
root: manifests.root,
identityResolver: self.identityResolver,
additionalFileRules: self.configuration.additionalFileRules,
Expand All @@ -1119,6 +1120,57 @@ extension Workspace {
fileSystem: fileSystem,
observabilityScope: observabilityScope
)

try expectedSigningEntities.forEach { identity, expectedSigningEntity in
if let package = packageGraph.packages.first(where: { $0.identity == identity }) {
if let actualSigningEntity = package.registryMetadata?.signature?.signedBy {
if actualSigningEntity != expectedSigningEntity {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should the comparison be more limited?

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you have in mind? e.g., must be .recognized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, just posing the question

throw SigningError.mismatchedSigningEntity(
package: identity,
expected: expectedSigningEntity,
actual: actualSigningEntity
)
}
} else {
throw SigningError.unsigned(package: identity, expected: expectedSigningEntity)
}
} else {
if let mirror = self.mirrors.mirror(for: identity.description) {
let mirroredIdentity = PackageIdentity.plain(mirror)
if mirroredIdentity.isRegistry {
if let package = packageGraph.packages.first(where: { $0.identity == mirroredIdentity }) {
if let actualSigningEntity = package.registryMetadata?.signature?.signedBy {
if actualSigningEntity != expectedSigningEntity {
throw SigningError.mismatchedSigningEntity(
package: identity,
expected: expectedSigningEntity,
actual: actualSigningEntity
)
}
} else {
throw SigningError.unsigned(package: identity, expected: expectedSigningEntity)
}
} else {
// Unsure if this case is reachable in practice.
throw SigningError.expectedIdentityNotFound(package: identity)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how we get into this scenario, so this isn't covered by any tests right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this more of an invalid state error?

}
} else {
throw SigningError.expectedSignedMirroredToSourceControl(package: identity, expected: expectedSigningEntity)
}
} else {
throw SigningError.expectedIdentityNotFound(package: identity)
}
}
}

return packageGraph
}

public enum SigningError: Swift.Error {
case expectedIdentityNotFound(package: PackageIdentity)
case expectedSignedMirroredToSourceControl(package: PackageIdentity, expected: RegistryReleaseMetadata.SigningEntity)
case mismatchedSigningEntity(package: PackageIdentity, expected: RegistryReleaseMetadata.SigningEntity, actual: RegistryReleaseMetadata.SigningEntity)
case unsigned(package: PackageIdentity, expected: RegistryReleaseMetadata.SigningEntity)
}

@discardableResult
Expand Down
Loading