#3179 [1.27.0] git with credentials token don't work
Closed: Fixed by tkopecek. Opened by frank-mdc.

With 1.27.0 git with credentials was added, but when it contains an "/" char then it fails.
Sample:
koji build --scratch FOO-CHAIN "git+https://USERR:TOKEN@GITSERVER/PATH_to_RPEO"
this will result in:
3722 build (FOO-CHAIN, /REST_OF_THE_TOKEN@GITSERVER/PATH_TO_REPO): free


Metadata Update from @tkopecek:
- Custom field Size adjusted to None
- Issue set to the milestone: 1.28
- Issue tagged with: bug

This is problem, because we are using urllib.parse.urlparse for parse of url. In urlparse, there is netloc part, which has char's rules:
Characters in the netloc attribute that decompose under NFKC normalization (as used by the IDNA encoding) into any of /, ?, #, @, or : will raise a ValueError. If the URL is decomposed before parsing, no error will be raised.

So, @mikem , @tkopecek do we want to fix this test case or is it correct as non-valid?

https://docs.python.org/3/library/urllib.parse.html

When I try to encode the "/" to %2F then the path part of the scm will shown correct, but it fails with:
3744 build (FOO-CHAIN, PATH_to_RPEO): free
GenericError: username:password format not supported

Yes, because Koji isn't support username:password format now.
Supported format example:
"https://rootpassword@GITSERVER/PATH_to_REPO"

Supported format example:
"https://rootpassword@GITSERVER/PATH_to_REPO"

Correction, only a user value is expected, not a user:pw combo nor a plain password. According to the SCM class,

        The expected url format is:
        scheme://[user@]host/path/to/repo?path/to/module#revision_or_tag_identifier

When the url is parsed and a user is found, the error you're seeing is explicitly raised if it contains a colon.

    elif ':' in user:
        raise koji.GenericError('username:password format not supported: %s' % user)

When #2868 was filed, the issue description only mentioned passing a user through, not a user:pw.

As Tomas noted in that issue, the previous support for passed users was explicitly no-password:

we can't pass password from user to builder (it would be visible in too many places), so only administrator can put credentials on every builder (PR #1640). So, mostly only repos accessible without authentication makes sense for end-user. So, we're stripping username intentionally there.

The trick here is that Koji cannot know the difference between "a real password" and "a token with read only permissions".

The scm url parameter is part of the task parameters, which are:

  • visible in the web ui
  • accessible via the api
  • stored in the db in plaintext
  • possibly logged

Furthermore the scm url is also added the the build entry for non-scratch builds. The point is this value is highly visible.

While you may only intend to use this feature with low-value passwords, removing this error would allow another user to do the same with an actual password (either through naivete or accident).

Do you control your git server? Is there the possibility of any sort of host-based access?

Or have you explored using the scm_credentials_dir builder config option that Tomas alluded to? (see #1640)

Aside: fwiw, it appears that the named tuple returned by urllib.parse.urlparse includes the parsed components of netloc (username, password, hostname, and port), even in py2.7 (though I'm not sure it did when the SCM class was originally written). So, independent of the password issue, we might consider dropping our own code for parsing netloc in favor of just checking those fields. This avoids the encoding issue.

Or have you explored using the scm_credentials_dir builder config option that Tomas alluded to? (see #1640)

I can't find any documentation how to use it. Of curse this can be an alternative.

I can't find any documentation how to use it.

Hmm, there isn't really any besides the comments in #1640 and the brief release note.
https://docs.pagure.org/koji/release_notes/release_notes_1.20/

However, after looking at this feature a little more I don't believe it will help here. All it does is bind mount the specified dir as /credentials for buildSRPMFromSCM tasks. The intent is to allow for the code run inside the buildroot to expand the sources (make sources or whatever is configured for the scm insource_cmd) to use those credentials when fetching tarballs or similar.

However, koji itself never looks inside that directory, so this feature can't get any sort of password into the kojid code that does the initial scm checkout.

We're kind of up against a couple deep design issues here:

  1. Koji wants all builds to be generated from clearly specified inputs that are recorded forever
  2. Koji has (mostly) transparent data. The only secrets in the db are those for Koji's own authentication.

Solving this correctly will require some pretty significant work.

However, for the sake of pragmatism and getting this working for you, I think I'd be ok with the following quick workaround.

  • we fix the netloc parsing code to utilize the extended fields in the urlparse return, as described above
  • we add a new kojid configuration option, allow_password_in_scm_url (default False) that controls whether or not kojid errors when it finds such a password
  • we document this option as a workaround, clearly indicating that its use is not recommended for security reasons

Metadata Update from @jcupova:
- Issue tagged with: testing-ready

Metadata Update from @jobrauer:
- Issue tagged with: testing-done

Commit e029d4c4 fixes this issue

Commit 725c157f fixes this issue

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/koji/koji/issues/3179

Please continue any further discussion there.

Metadata
Related Pull Requests