Skip to content

Incorporate feedback from 2nd review of SE-0292 #1319

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 10 commits into from
May 27, 2021
Merged

Incorporate feedback from 2nd review of SE-0292 #1319

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1947dce
Fix regular expression for package scopes
mattt Apr 5, 2021
5b568fa
Package names are compared using NFKC + CaseFolding
mattt Apr 5, 2021
4f019d3
The default, unscoped registry is denoted by [default]
mattt Apr 5, 2021
526fe33
Mention .netrc file as mitigation for hardcoded credentials
mattt Apr 5, 2021
79d1377
Remove trailing whitespace
mattt Apr 5, 2021
372c605
Fix typo name -> scope
mattt Apr 5, 2021
ac89166
Add link to draft registry proposal in @apple/swift-package-manager
mattt May 4, 2021
d44d47a
Update Information disclosure section
mattt May 18, 2021
ed0c440
Add section discussing authentication
mattt May 20, 2021
c03e612
Use console syntax for terminal code blocks
mattt May 20, 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
178 changes: 115 additions & 63 deletions proposals/0292-package-registry-service.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ and downloading the source archive for a release:
| `GET` | `/{scope}/{name}/{version}.zip` | Download source archive for a package release |
| `GET` | `/identifiers{?url}` | Lookup package identifiers registered for a URL |

A formal specification for the package registry interface
is provided alongside this proposal.
A formal specification for the package registry interface is provided
[alongside this proposal](https://github.com/apple/swift-package-manager/blob/main/Documentation/RegistryDraft.md).
In addition,
an OpenAPI (v3) document
and a reference implementation written in Swift
Expand All @@ -145,11 +145,11 @@ A package scope consists of
alphanumeric characters and hyphens.
Hyphens may not occur at the beginning or end,
nor consecutively within a scope.
The maximum length of a package name is 39 characters.
The maximum length of a package scope is 39 characters.
A valid package scope matches the following regular expression pattern:

```regexp
\A[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,39}\z
\A[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,38}\z
```

A package's *name* uniquely identifies a package in a scope.
Expand All @@ -163,16 +163,9 @@ A valid package name matches the following regular expression pattern:
> For more information,
> see [Unicode Identifier and Pattern Syntax][UAX31].

Package scopes are case-insensitive
(for example, `mona` ≍ `MONA`).
Package names are
case-insensitive,
diacritic-insensitive
(for example, `Å` ≍ `A`), and
width-insensitive
(for example, `Ａ` ≍ `A`).
Package names are compared using
[Normalization Form Compatible Composition (NFKC)][UAX15].
[Normalization Form Compatible Composition (NFKC)][UAX15]
with locale-independent case folding.

#### New `PackageDescription` API

Expand Down Expand Up @@ -242,7 +235,7 @@ that is, the `package` parameter in `.product(name:package)` method calls.

```diff
targets: [
.target(name: "MyLibrary",
.target(name: "MyLibrary",
dependencies: [
.product(name: "LinkedList",
- package: "LinkedList")
Expand Down Expand Up @@ -306,7 +299,7 @@ Swift Package Manager compares the integrity checksum provided by the server
against any existing checksum for that release in the `Package.resolved` file
as well as the integrity checksum reported by the `compute-checksum` subcommand:

```terminal
```console
$ swift package compute-checksum LinkedList-1.2.0.zip
1feec3d8d144814e99e694cd1d785928878d8d6892c4e59d12569e179252c535
```
Expand Down Expand Up @@ -339,7 +332,7 @@ a package's contents may have changed at some point.
Swift Package Manager will refuse to download dependencies
if there's a mismatch in integrity checksums.

```terminal
```console
$ swift build
error: checksum of downloaded source archive of dependency 'mona.LinkedList' (c2b934fe66e55747d912f1cfd03150883c4f037370c40ca2ad4203805db79457) does not match checksum specified by the manifest (ed008d5af44c1d0ea0e3668033cae9b695235f18b1a99240b7cf0f3d9559a30d)
```
Expand Down Expand Up @@ -373,7 +366,7 @@ in the root directory of a package
to generate a source archive for the current working tree.
For example:

```terminal
```console
$ tree -a -L 1
LinkedList
├── .git
Expand All @@ -399,7 +392,7 @@ the name of the package with a `.zip` extension
(for example, "LinkedList.zip").
You can override this behavior with the `--output` option:

```terminal
```console
$ git checkout 1.2.0
$ swift package archive-source --output="LinkedList-1.2.0.zip"
# Created LinkedList-1.2.0.zip
Expand All @@ -410,7 +403,7 @@ The `archive-source` subcommand has the equivalent behavior of
Therefore, the following command produces
equivalent output to the previous example:

```terminal
```console
$ git archive --format zip --output LinkedList-1.2.0.zip 1.2.0
```

Expand Down Expand Up @@ -450,29 +443,33 @@ SYNOPSIS
OPTIONS:
--global Apply settings to all projects for this user
--scope Associate the registry with a given scope
--login Specify a user name for the remote machine
--password Supply a password for the remote machine
```

Running the `package-registry set` subcommand
in the root directory of a package
creates or updates the `.swiftpm/config/registries.json` file
with a new top-level `registries` key
that's associated with an object containing the specified registry URLs.
The default, unscoped registry is associated with the key `[default]`.
Any scoped registries are keyed by their case-folded name.

For example,
a build server that doesn't allow external network connections
may configure a registry URL to resolve dependencies
using an internal registry service.

```terminal
```console
$ swift package-registry set https://internal.example.com/
$ cat .swiftpm/config/registries.json
```

```json
{
"registries": {
"default": {
"url": "https://internal.example.com"
"[default]": {
"url": "https://internal.example.com"
}
},
"version": 1
Expand All @@ -485,7 +482,7 @@ Swift Package Manager commands like
`swift package resolve` and `swift package update`
fail with an error.

```terminal
```console
$ swift package resolve
error: cannot resolve dependency 'mona.LinkedList' without a configured registry
```
Expand All @@ -500,14 +497,14 @@ a user might resolve all packages with the package scope `example`
(such as `example.PriorityQueue`)
to a private registry.

```terminal
```console
$ swift package-registry set https://internal.example.com/ --scope example
$ cat .swiftpm/config/registries.json
```

```json
{
"registries": {
"registries": {
"example": {
"url": "https://internal.example.com"
}
Expand Down Expand Up @@ -557,8 +554,8 @@ consider the following global and local registry configuration files:
```jsonc
// Global configuration (~/.swiftpm/config/registries.json)
{
"registries": {
"default": {
"registries": {
"[default]": {
Copy link
Contributor

Choose a reason for hiding this comment

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

does square brackets have precedence in other such system? in Swift we usually use underbar for making "special" things

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 chose [default], because it was visually distinctive. However, the only requirement is for it to not be a valid scope name, so _default could work just as well. At that point, it's a subjective choice.

As far as precedence, Swift uses square brackets in closures to annotate capture semantics ([weak self]), whereas underscore-prefixed variables are an unenforced convention. Another option to consider might be _. I don't feel too strongly either which way.

Copy link
Contributor

@tomerd tomerd May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer we go with underbar _ or dot ., tho the latter may be too subtle. @neonichu @abertelrud @yim-lee @weissi @Lukasa opinions?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on _

Copy link
Contributor

Choose a reason for hiding this comment

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

I think _ is usually associated with private or "not to be used" things in the Apple developer community, so seems like we should avoid that.

Copy link

Choose a reason for hiding this comment

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

Agree with @neonichu , _ means "don't use, the ACL on this decl is more permissive than it should ideally be" in most of Swift.

Happy with [default], <default>, (default), .default, ... Don't think I have a strong preference. We could also go with "" to mean default but that's probably a bit opaque.

Copy link
Contributor

Choose a reason for hiding this comment

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

Happy with any of @weissi's options there, I don't object to the _ but I understand the desire not to use it.

"url": "https://global.example.com"
},
"foo": {
Expand All @@ -570,7 +567,7 @@ consider the following global and local registry configuration files:

// Local configuration (.swiftpm/config/registries.json)
{
"registries": {
"registries": {
"foo": {
"url": "https://local.example.com"
}
Expand All @@ -596,14 +593,65 @@ in descending order of precedence:
* Any local configuration (`./.swiftpm/config/registries.json`)
* Any global configuration file (`~/.swiftpm/config/registries.json`)

#### Specifying credentials for a custom registry

Some servers may require a username and password.
The user can provide credentials when setting a custom registry
by passing the `--login` and `--password` options.

When credentials are provided,
the corresponding object in the `registries.json` file
includes a `login` key with the passed value.
If the project's `.netrc` file has an existing entry
for a given machine and login,
it's updated with the new password;
otherwise, a new entry is added.
If no `.netrc` file exists,
a new one is created and populated with the new entry.

```console
$ swift package-registry set https://internal.example.com/ \
--login jappleseed --password alpine

$ cat .netrc
machine internal.example.com
login jappleseed
password alpine

$ cat .swiftpm/config/registries.json

{
"registries": {
"[default]": {
"url": "https://internal.example.com"
"login": "jappleseed"
}
},
"version": 1
}
```

If the user passes the `--login` and `--password` options
to the `set` subcommand along with the `--global` option,
the user-level `.netrc` file is updated instead.
When Swift Package Manager connects to a custom registry,
it first consults the project's `.netrc` file, if one exists.
If no entry is found for the custom registry,
Swift Package Manager then consults the user-level `.netrc` file, if one exists.

If the provided credentials are missing or invalid,
Swift Package Manager commands like
`swift package resolve` and `swift package update`
fail with an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Lukasa @mmarston ptal ^^

Copy link

Choose a reason for hiding this comment

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

This UX, the ability to pass --login and --password, is what I was looking for, so this looks good to me. I have a few questions about the behavior.

  1. Does this support using different credentials for different registries served from the same host name (at different paths)? I'm inferring that the answer is yes since the login field is added to registries.json, which means that the .netrc could have more than one entry for the same machine with different login and the Swift Package Manager would select the one with the matching login.
  2. Is the user allowed to omit --password when passing --login? (For example, to specify which login to use if the user already has credentials in their user-level .netrc).
  3. What credentials will the Swift Package Manager use if the registry redirects to a different host? For example, the a private registry may require credentials and then it may redirect the client to a pre-signed S3 URL. From a security perspective the client shouldn't include the credentials when following a redirect to a different host. Will the client attempt to look up the host of the redirect location in a .netrc file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Does this support using different credentials for different registries served from the same host name (at different paths)?

Yes. A different login / password combination can be specified to add a separate entry in the .netrc file.

  1. Is the user allowed to omit --password when passing --login

Yes.

  1. What credentials will the Swift Package Manager use if the registry redirects to a different host?

I'll defer to @tomerd about this specific implementation detail, but my expectation as a user and registry operator would be for any provided credentials (like in a pre-signed S3 URL) to be respected. If that request fails with a 401, I think there's a good case for Swift Package Manager to retry with any available .netrc credentials.

Copy link
Contributor

Choose a reason for hiding this comment

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

this all makes sense to me

Copy link
Contributor

Choose a reason for hiding this comment

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

This approach seems reasonable, provided that SwiftPM's default .gitignore file ignores the project .netrc file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Lukasa I just submitted a PR with that change here: swiftlang/swift-package-manager#3511

### Changes to config subcommand

#### Set-mirror option for package identifiers

A user can currently specify an alternate location for a package
by setting a [dependency mirror][SE-0219] for that package's URL.

```terminal
```console
$ swift package config set-mirror \
--original-url https:///github.com/mona/linkedlist \
--mirror-url https:///github.com/octocorp/swiftlinkedlist
Expand Down Expand Up @@ -717,7 +765,7 @@ Although the impact of such an attack is potentially high,
the risk is largely mitigated by the use of cryptographic checksums
to verify the integrity of downloaded source archives.

```terminal
```console
$ echo "$(swift package compute-checksum LinkedList-1.2.0.zip) *LinkedList-1.2.0.zip" | \
shasum -a 256 -c -
LinkedList-1.2.0.zip: OK
Expand Down Expand Up @@ -754,7 +802,7 @@ If the history of a project is available
and the commit used to generate the source archive is signed with [GPG],
the cryptographic signature may be used to verify the authenticity.

```terminal
```console
$ git rev-parse HEAD
b7c37c81f164e5dce0f64e3d75c79a48fb1fe00b3

Expand All @@ -776,41 +824,30 @@ can both provide similar non-repudiation guarantees.

### Information disclosure

A user may inadvertently reveal the existence of a private registry
or expose hardcoded credentials
by checking in their project's `.swiftpm/config` directory.

An attacker could scrape public code repositories for `.swiftpm/config` files
and attempt to reuse those credentials to impersonate the user.
A user may inadvertently expose credentials
by checking in their project's configuration files.
An attacker could scrape public code repositories for configuration files
and attempt to reuse credentials to impersonate the user.

The risk of leaking credentials can be mitigated by
storing them in a `.netrc` file located outside the project directory
(typically in the user's home directory).
However,
a user may run `swift package` subcommands with the `--netrc-file` option
to configure the location of their project's `.netrc` file.
To mitigate the risk of a user inadvertently
adding a local `.netrc` file to version control,
Swift Package Manager could add an entry to the `.gitignore` file template
for new projects created with `swift package init`.

```json
{
"registries": {
"default": {
"url": "https://<USERNAME>:<TOKEN>@swift.pkg.github.com/<OWNER>/"
}
},
"version": 1
}

```

This kind of attack can be mitigated on an individual basis
by adding `.swiftpm/config` to a project's `.gitignore` file.
The risk could be mitigated for all users
if Swift Package Manager included a `.gitignore` file
in its new project template.
Code hosting providers can also help minimize this risk
by [detecting secrets][secret scanning]
that are committed to public repositories.

Credentials may also be unintentionally disclosed
by Swift Package Manager or other tools in logging statements.
Care should be taken to redact the user info component of URLs
when displaying feedback to the user
(for example,
the URL `https://<USERNAME>:<TOKEN>@swift.pkg.github.com`
is logged as `https://***@swift.pkg.github.com`).
Care should be taken to redact usernames and passwords
when displaying feedback to the user.

### Denial of service

Expand All @@ -820,10 +857,25 @@ that declare one or more custom registries
and launch a denial-of-service attack
in an attempt to reduce the availability of those resources.

```json
{
"registries": {
"[default]": {
"url": "https://private.example.com"
}
},
"version": 1
}

```

The likelihood of this attack is generally low
but could be used in a targeted way
against resources known to be important or expensive to distribute.

This kind of attack can be mitigated on an individual basis
by adding `.swiftpm/config` to a project's `.gitignore` file.

### Escalation of privilege

Even authentic packages from trusted creators can contain malicious code.
Expand Down Expand Up @@ -1047,7 +1099,7 @@ let package = Package(
could be extended to add dependencies using scoped identifiers
in addition to URLs.

```terminal
```console
$ swift package add-dependency mona.LinkedList
# Installed LinkedList 1.2.0
```
Expand All @@ -1062,7 +1114,7 @@ Swift Package Manager could add tooling
to help package maintainers adopt registry-supported identifiers
in their projects.

```terminal
```console
$ swift package-registry migrate
```

Expand Down Expand Up @@ -1096,7 +1148,7 @@ Swift Package Manager could communicate this information to users
when installing or updating dependencies
or as part of a new `swift package audit` subcommand.

```terminal
```console
$ swift package audit
┌───────────────┬────────────────────────────────────────────────┐
│ High │ Regular Expression Denial of Service │
Expand All @@ -1120,7 +1172,7 @@ The package registry API could be extended to add a search endpoint
to allow users to search for packages by name, keywords, or other criteria.
This endpoint could be used by clients like Swift Package Manager.

```terminal
```console
$ swift package search LinkedList
LinkedList (github.com/mona/LinkedList) - One thing links to another.

Expand Down