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

Docker signature transport uses reserved URI characters #187

Closed
aweiteka opened this issue Dec 19, 2016 · 13 comments
Closed

Docker signature transport uses reserved URI characters #187

aweiteka opened this issue Dec 19, 2016 · 13 comments

Comments

@aweiteka
Copy link
Contributor

Per implementation (also doc #120) the manifest digest reference for the "docker" transport type is HASH_FUNCTION:HASH_VALUE, using a colon (:) delimiter. The "at" symbol (@) is also used the delimit image and manifest digest (again, see #120). Per URI: Generic Syntax these are reserved delimiting characters.

Impact
The colon in the URI is blocking uploading of signatures to JFrog's Artifactory repository management platform, returning 500 error:

"Invalid path. ':' is not a valid name character: atomic-sigstore/aweiteka/true@sha256:f292b8573b1679a512f575d7bc2441815b7528eb114217781199e9106e742e21/"

Percent-encoding the path (%3A) does not help. Artifactory has not responded to this related issue

@runcom
Copy link
Member

runcom commented Dec 19, 2016

/cc @mtrmac

@aweiteka
Copy link
Contributor Author

related issue #120

@mtrmac
Copy link
Collaborator

mtrmac commented Jan 3, 2017

Ouch.

Anyway, to be more specific, quoting that RFC:

These characters are called
"reserved" because they may (or may not) be defined as delimiters by
the generic syntax, by each scheme-specific syntax, or by the
implementation-specific syntax of a URI's dereferencing algorithm.
If data for a URI component would conflict with a reserved
character's purpose as a delimiter, then the conflicting data must be
percent-encoded before the URI is formed.

(emphasis mine)

Then, looking at http URI ( https://tools.ietf.org/html/rfc7230#section-2.7.1 ), we are back in https://tools.ietf.org/html/rfc3986#section-3.3 for path-abempty, and ultimately pchar, which explicitly contains :, and then

instead, each syntax rule lists the characters
allowed within that component (i.e., not delimiting it)

so : is allowed in principle, with

, and any of
those characters that are also in the reserved set are "reserved" for
use as subcomponent delimiters within the component.

which… changes nothing?

And then

URI producing applications should percent-encode data octets that
correspond to characters in the reserved set unless these characters
are specifically allowed by the URI scheme to represent data in that
component.

The HTTP specification says nothing in that respect, but

If a reserved character is found in a URI component and
no delimiting role is known for that character, then it must be
interpreted as representing the data octet corresponding to that
character's encoding in US-ASCII.

if we include : in the URL, it “must” be interpreted as a : byte.


Anyway, with

Percent-encoding the path (%3A) does not help.

the issue of : being reserved or not by the URI spec is moot, any percent-encoded byte is allowed in the URI; AFAICS the issue is simply that Artifactory refuses a :, however represented.

(That’s not to say that this isn’t an issue and we can just define it away… but if we care about what Artifactory accepts, then we need to know what it refuses so that any possible replacement to the scheme isn’t rejected again.)

@rhatdan
Copy link
Member

rhatdan commented Jan 3, 2017

I believe we care what Artifactory accepts. Should we open a conversation with them?

@aweiteka
Copy link
Contributor Author

aweiteka commented Jan 3, 2017

@mtrmac
Copy link
Collaborator

mtrmac commented Jan 4, 2017

I guess this could be because of https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx , the impossibility to use colons in file names on Windows? That does seem to be important for cross-platform portability.

@rhatdan
Copy link
Member

rhatdan commented Jan 12, 2017

So from my reading these characters are out.

< (less than)
> (greater than)
: (colon)
" (double quote)
/ (forward slash)
\ (backslash)
| (vertical bar or pipe)
? (question mark)
* (asterisk)

@rhatdan
Copy link
Member

rhatdan commented Jan 12, 2017

Suggestions:

!
#
&
+
-
_
@
~

@aweiteka
Copy link
Contributor Author

# is a fragment delimiter, example.com#some-anchor
& is a query key=value delimiter, example.com?foo=bar&bar=foo
@ is an email address delimiter, [email protected]
~ is a user delimiter, example.com/~aweiteka

@mtrmac
Copy link
Collaborator

mtrmac commented Jan 12, 2017

Based on https://github.com/docker/distribution/blob/master/reference/reference.go ,

  • / delimits components
  • _, . and - are allowed in reference components
  • +, _, . and - are allowed in digest algorithm
  • : and @ are used in the tag@algo:digest syntax

(This duplicates some of the exclusions mentioned above, repeating to explicitly associate them with the semantics / rationale.)

@rhatdan
Copy link
Member

rhatdan commented Jan 12, 2017

Does that leave from my original list.

!
~

giuseppe pushed a commit to giuseppe/image that referenced this issue Jan 24, 2017
Update for mtrmac/image:api-changes
mtrmac added a commit to mtrmac/image that referenced this issue Mar 4, 2017
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac
Copy link
Collaborator

mtrmac commented Mar 4, 2017

So the URI reserved characters are really a red herring; containers/image does already percent-encode them in the path when sending the request, and HTTP servers obviously percent-decode. This can be demonstrated by applying #201 and configuring e.g.

docker:
    docker.io/library:
        sigstore: https://example.com/a:b/A%3AB/c[d/e]f/g@h

in a registries.d/something.yaml, which results in e.g.

GET /a:b/A:B/c%5Bd/e%5Df/g@h/docker.io/library/busybox@sha256:817a12c32a39bbe394944ba49de563e085f1d3c5266eb8e9723256bc4448680e/signature-1 HTTP/1.1

(note how both : and @ are unquoted, and how Go’s net/url parsing+quoting canonicalizes %3A to :; apparently the correct reading of RFC 3986 is that when : and @ are explicitly listed in the pchar rule,

URI producing applications should percent-encode data octets that
correspond to characters in the reserved set unless these characters
are specifically allowed by the URI scheme to represent data in that
component.

“specifically allowed’ characters are not percent-encoded)

So, the binding constraints are only 1) Windows pathname restrictions, 2) characters which already have defined meanings in docker digest references, prohibiting ", *, +, -, ., :, <, >, ?, \, _, |.

That leaves us quite a few options.

To make things simple for naive clients constructing HTTP by pasting strings, we can use one of the “specifically allowed” characters for pchar, i.e. :, @ (explicitly allowed), !, $, &, ', (, ), *, +, ,, ;, = (sub-delims), -, ., _, ~. (with strikethrough for the characters prohibited above).

And to make things simple for us and copy&paste in shell, let’s only consider characters which are not treated by sh specially: %, ,, =, @, ^, {, }, ~ (the last three are treated specially only in specific positions).

The intersection is ,, =, @, ~, and we are already using @. In this set, = seems a fairly obvious choice: …/busybox@sha256=817a12c32a39bbe394944ba49de563e085f1d3c5266eb8e9723256bc4448680e.

@aweiteka
Copy link
Contributor Author

aweiteka commented Mar 6, 2017

I like it. '=' it is.

mtrmac added a commit to mtrmac/image that referenced this issue Mar 17, 2017
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
mtrmac added a commit to mtrmac/image that referenced this issue Mar 28, 2017
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
mtrmac added a commit to mtrmac/image that referenced this issue Mar 29, 2017
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
mtrmac added a commit to mtrmac/image that referenced this issue Mar 30, 2017
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
Jamesjaj2dc6j added a commit to Jamesjaj2dc6j/lucascalsilvaa that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
mrhyperbit23z0d added a commit to mrhyperbit23z0d/thomasdarimont that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
easyriderk0c9g added a commit to easyriderk0c9g/rnin9 that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
oguzturker8ijm8l added a commit to oguzturker8ijm8l/diwir that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
wesnowm added a commit to wesnowm/MatfOOP- that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
gaveen3 added a commit to gaveen3/KimJeongHoon3x that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
gaveen3 added a commit to gaveen3/KimJeongHoon3x that referenced this issue Jun 6, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
Manuel-Suarez-Abascal80n9y added a commit to Manuel-Suarez-Abascal80n9y/knimeu that referenced this issue Oct 7, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
straiforos8bsh5n added a commit to straiforos8bsh5n/tokingsq that referenced this issue Oct 7, 2022
i.e. use …/busybox@sha256=… instead of ../busybox@sha256:… .

See containers/image#187 for more discussion.

Signed-off-by: Miloslav Trmač <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants