caddy-git icon indicating copy to clipboard operation
caddy-git copied to clipboard

parse ssh invalid port ":sjatsh" after host

Open sjatsh opened this issue 4 years ago • 16 comments

image

image

sjatsh avatar Aug 14 '19 15:08 sjatsh

@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.

awoodbeck avatar Aug 19 '19 11:08 awoodbeck

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

https://github.com/golang/go/commit/3226f2d492963d361af9dfc6714ef141ba606713

phpgao avatar Aug 28 '19 07:08 phpgao

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

BobCashStory avatar Sep 05 '19 16:09 BobCashStory

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?

ricardoboss avatar Sep 09 '19 07:09 ricardoboss

@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. :(

doowzs avatar Sep 09 '19 12:09 doowzs

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.

awoodbeck avatar Sep 09 '19 13:09 awoodbeck

@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 👍

ricardoboss avatar Sep 09 '19 20:09 ricardoboss

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 :disappointed:

blankoworld avatar Sep 11 '19 14:09 blankoworld

@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 avatar Sep 11 '19 14:09 awoodbeck

@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 :-/

blankoworld avatar Sep 18 '19 19:09 blankoworld

@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.

awoodbeck avatar Sep 18 '19 22:09 awoodbeck

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

blankoworld avatar Sep 19 '19 19:09 blankoworld

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.

awoodbeck avatar Sep 20 '19 00:09 awoodbeck

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.

jonasrauber avatar Oct 23 '19 11:10 jonasrauber

@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.

nurtext avatar Oct 30 '19 09:10 nurtext

Problem is still exist :(

region23 avatar Nov 16 '19 06:11 region23