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

parse ssh invalid port ":sjatsh" after host #106

Open
sjatsh opened this issue Aug 14, 2019 · 16 comments
Open

parse ssh invalid port ":sjatsh" after host #106

sjatsh opened this issue Aug 14, 2019 · 16 comments

Comments

@sjatsh
Copy link

sjatsh commented Aug 14, 2019

image

image

awoodbeck added a commit to awoodbeck/caddy-git that referenced this issue Aug 14, 2019
@awoodbeck
Copy link

@sjatsh if you want to use these changes before they are merged into this repo, feel free to use my fork: https://github.com/awoodbeck/caddy-git

I've merged these changes into it. Once these changes are merged, I'll resync my fork, which means you'll need to go back to using this repo.

@phpgao
Copy link

phpgao commented Aug 28, 2019

As you can see, url.Parse will return err if no port number is given.

golang/go@3226f2d

@BobCashStory
Copy link

the problem is git don't accept port in the ssh url

@ricardoboss
Copy link

Why is something that crucial not being tested prior to a release?! Can't you just implement a custom parser that checks for this URL style? I'm not familiar with golang but is there a parser that can do this?

@doowzs
Copy link

doowzs commented Sep 9, 2019

@ricardoboss I think lacking a dependency management system like npm or composer of Golang is to blame.

The author of this plugin uses Go 1.12.6 in testing, and the plugin works well in this environment. However, the updated official URL parser comes with later versions of Go, and the tests failed when I changed to Go v1.13. Official Caddy builds seem to use up-to-date Go packages, causing this plugin to break. :(

@awoodbeck
Copy link

This commit changed url.Parse's behavior to satify CVE-2019-14809. Since this middleware relies on url.Parse, it was affected by the change.

We often use SSH URLs with relative paths (e.g., [email protected]:abiosoft/caddy-git.git where "abiosoft/caddy-git.git" is the relative path indicated by the ":") as opposed to full paths (e.g., [email protected]/abiosoft/caddy-git.git indicated by the initial "/"), the first element of the relative path is treated as a named port (e.g. ":abiosoft" in my first example). This fails the URL parsing in Go since url.Parse no longer allows named ports.

My proposed solution is to insert a pseudo port of 0 just before passing the URL onto url.Parse and then removing this pseudo port if parsing succeeds. url.Parse doesn't entirely consider SSH URLs with relative paths, so my proposed change is meant to make this middleware play nice with url.Parse.

@ricardoboss
Copy link

@doowzs thanks for the explanation! This helps me understand what's going on...
@awoodbeck I see. Could be a workaround until there is a better solution.

Thank you guys and @abiosoft for creating and maintining a nice project like this! Keep the good work up 👍

@blankoworld
Copy link

Does anybody have a workaround for current caddyserver git repository? It's pretty uncomfortable to have an entire server down because it uses a lot this git plugin 😞

@awoodbeck
Copy link

awoodbeck commented Sep 11, 2019

@blankoworld Yes. Use https://github.com/awoodbeck/caddy-git temporarily until either this PR gets merged or Abiola comes up with another solution. My fork of this repo includes the changes I'm proposing here. I've been using them for a few weeks now without issue.

@blankoworld
Copy link

blankoworld commented Sep 18, 2019

@blankoworld Yes. Use https://github.com/awoodbeck/caddy-git temporarily until either this PR gets merged or Abiola comes up with another solution. My fork of this repo includes the changes I'm proposing here. I've been using them for a few weeks now without issue.

@awoodbeck Could you please explain how to use the plugin? Should I compile something? Where should I place the result? How to create a caddy binary for that?

It's a little bit cryptic for me :-/

@awoodbeck
Copy link

@blankoworld I'd be happy to. The following is applicable to Caddy version 1. It wouldn't surprise me if these steps change with version 2. I haven't been following its development.

  1. Create a sub directory for your caddy build: mkdir -p $HOME/caddy
  2. Create a file called $HOME/caddy/main.go with the following contents:
package main

import (
    "github.com/caddyserver/caddy/caddy/caddymain"

    // Put all of your plugin imports here.
    _ "github.com/awoodbeck/caddy-git"
)

func main() {
    // Your choice to disable telemetry by uncommenting the following line.
    // caddymain.EnableTelemetry = false
    caddymain.Run()
}
  1. Run the following commands to build caddy:
cd $HOME/caddy
go mod init caddy
go get github.com/caddyserver/caddy/caddy
go build
  1. The resulting $HOME/caddy/caddy binary should now include a working caddy-git plugin. Copy it to wherever you have your old caddy binary, making sure to back up your previous caddy binary first.

Granted, the above steps make some assumptions about your server, but they should get you started. The plugins are compiled into the caddy binary. If you want to add additional plugins at a later time, you need to recompile the caddy binary after adding the plugin's import path to the main.go file.

@blankoworld
Copy link

Thank you @awoodbeck ! What about all other "normal" plugins? How can I activate them?

@awoodbeck
Copy link

You add their import path to the example main.go I posted above. See: https://github.com/caddyserver/caddy/wiki/Plugging-in-Plugins-Yourself

For example, I'm including the caddy-git plugin by adding _ "github.com/awoodbeck/caddy-git" to the import block. The preceding underscore is important because all we care about are the plugin's side effects. I then build the binary. All that's left to do is add the plugin's directive(s) to my Caddyfile. Each plugin usually has documentation on how to use it in the Caddyfile.

@jonasrauber
Copy link

Changing the URLs from [email protected]:username/repo.git to [email protected]:/username/repo.git (adding a slash after the colon) worked for me. No need to recompile the plugin or anything.
Nevertheless, I think it would be good to update the official binaries that include this plugin. People will be very surprised to see caddy not working as described in the docs.

@nurtext
Copy link

nurtext commented Oct 30, 2019

@jonasrauber That's a workaround for now, but not a real solution. I'm with @ricardoboss: Such things need to be tested before release and this needs to be fixed code-wise.

@region23
Copy link

Problem is still exist :(

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

10 participants