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

implement ssl-skip-verify to forward to self-signed-certificates #29

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
18 changes: 18 additions & 0 deletions .github/workflows/makefile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: Makefile CI

on:
push:
branches: [ master ]
pull_request:
branches: [ master ]

jobs:
build:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2

- name: make all
run: make gets test application
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ src/code.google.com/*
log
log/*
etc
__debug_bin
clammit.exe
clammit.cfg
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ all: gets application
linux:
GOOS=linux GOARCH=amd64 make application

linux-win:
set GOOS=linux && set GOARCH=amd64 && make application

This conversation was marked as resolved.
Show resolved Hide resolved
test: gets
go test ./...

Expand All @@ -27,5 +30,8 @@ application:
gets:
go get

build:
go build

release:
goreleaser release
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ content-memory-threshold | (Optional) Maximum payload size to keep in RAM. Large
log-file | (Optional) The clammit log file, if ommitted will log to stdout
test-pages | (Optional) If true, clammit will also offer up a page to perform test uploads
debug | (Optional) If true, more things will be logged
ssl-skip-verify | (Optional) If true, will skip SSL validation forwarding connection to use self-signed certitifates
Copy link
Contributor

Choose a reason for hiding this comment

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

I would reword this as:

if true, it will not perform TLS certificate verification, allowing connections to servers using self-signed or otherwise untrusted certificates. Use with care, it is always preferable to perform certificate verification.

Copy link
Author

Choose a reason for hiding this comment

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

I like your very precise documentation


The listen address can be a TCP port or Unix socket, e.g.:

Expand Down Expand Up @@ -179,6 +180,8 @@ steps manually if you want.
You will need external access to github and code.google.com to load the
third-party packages that Clammit depends on: [go-clamd][] and [gcfg][].

The release build also need [goreleaser](https://goreleaser.com/install/)

Once you have this, simple run:

```sh
Expand Down Expand Up @@ -248,6 +251,18 @@ testing/ sub-directory.

In the web/ directory is a simple Sinatra server to act as the application (for testing purposes).

install bundler
```
gem install sinatra
gem install bundler
```

run
```
cd web
bundle install
```

## Tests

Run ```make test```
Expand Down
6 changes: 6 additions & 0 deletions clammit.cfg.example
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,9 @@ log-file = log/clammit.log
# the virus scanning
#
#test-pages = true

#
# If true, will skip SSL validation forwarding connection to use self-signed certitifates
# default = false
This conversation was marked as resolved.
Show resolved Hide resolved
#
#ssl-skip-verify = false
12 changes: 10 additions & 2 deletions forwarder/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package forwarder

import (
"crypto/tls"
"io"
"io/ioutil"
"log"
Expand Down Expand Up @@ -48,17 +49,19 @@ type Forwarder struct {
logger *log.Logger
debug bool
contentMemoryThreshold int64
sslSkipVerify bool
}

/*
* Constructs a new forwarder. Pass in the application URL and the interceptor.
*/
func NewForwarder(applicationURL *url.URL, contentMemoryThreshold int64, interceptor Interceptor) *Forwarder {
func NewForwarder(applicationURL *url.URL, contentMemoryThreshold int64, interceptor Interceptor, sslSkipVerify bool) *Forwarder {
return &Forwarder{
applicationURL: applicationURL,
interceptor: interceptor,
logger: log.New(ioutil.Discard, "", 0),
contentMemoryThreshold: contentMemoryThreshold,
sslSkipVerify: sslSkipVerify,
}
}

Expand Down Expand Up @@ -236,6 +239,11 @@ func (f *Forwarder) getClient(req *http.Request) (*http.Client, *url.URL) {
}, url
} else {
f.logger.Printf("Forwarding to %s", applicationURL.String())
return &http.Client{}, url
// allow for
// https://stackoverflow.com/questions/12122159/how-to-do-a-https-request-with-bad-certificate
tr := &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: f.sslSkipVerify},
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks OK as today we only have a single option. For the future, we’ll want to pass the tls config object in the forwarder struct directly, that’ll allow us to drive any tls config options from clammit configuration without adding more fields to the Forwarder struct.

Copy link
Author

Choose a reason for hiding this comment

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

I agree - I was not happy also. But I did not wanted to do a full refactoring ....

}
return &http.Client{Transport: tr}, url
}
}
73 changes: 67 additions & 6 deletions forwarder/forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,45 @@ func TestInterceptor(t *testing.T) {
w.WriteHeader(204)
w.Write([]byte("This is a response"))
return true
}), false)

req, _ := http.NewRequest("POST", "http://localhost:9999/bar?crazy=true", strings.NewReader(bodyText))
w := NewTestResponseWriter()

fw.HandleRequest(w, req)

if w.StatusCode != 204 {
t.Fatalf("Response: StatusCode was %d, expected %d", w.StatusCode, 204)
}
if w.Header().Get("Foo") != "bar" {
t.Fatalf("Response: Header['foo'] not set")
}
if w.Body.String() != "This is a response" {
t.Fatalf("Response: Body is: %s", w.Body.String())
}
}

func TestInterceptorNonValidSll(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
t.Fatal("Request was forwarded but should not have been")
}))
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

bodyText := "This is the request body"

fw := NewForwarder(tsURL, 10000, testInterceptor(func(w http.ResponseWriter, req *http.Request, body io.Reader) bool {
buf := make([]byte, 10000)
if n, err := body.Read(buf); err != nil && err != io.EOF {
t.Fatalf("Got error reading body: %s", err.Error())
} else if string(buf[0:n]) != bodyText {
t.Fatalf("Read body failed: X%vX, expected X%vX", string(buf[0:n]), bodyText)
}
w.Header().Set("foo", "bar")
w.WriteHeader(204)
w.Write([]byte("This is a response"))
return true
}), true)

req, _ := http.NewRequest("POST", "http://localhost:9999/bar?crazy=true", strings.NewReader(bodyText))
w := NewTestResponseWriter()
Expand Down Expand Up @@ -118,7 +156,7 @@ func TestForwarding(t *testing.T) {
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

fw := NewForwarder(tsURL, 10000, nil)
fw := NewForwarder(tsURL, 10000, nil, false)

req, _ := http.NewRequest("POST", "http://localhost:99999/bar?crazy=true", strings.NewReader(requestText))
req.Header.Set("myheader", "headervalue")
Expand Down Expand Up @@ -154,7 +192,7 @@ func TestHostForwarding(t *testing.T) {
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

fw := NewForwarder(tsURL, 10000, nil)
fw := NewForwarder(tsURL, 10000, nil, false)

req, _ := http.NewRequest("POST", "http://localhost:99999/bar?crazy=true", strings.NewReader(requestText))
req.Header.Set("myheader", "headervalue")
Expand All @@ -175,7 +213,7 @@ func TestHostForwarding(t *testing.T) {
defer ts.Close()
tsURL, _ = url.Parse(ts.URL)

fw = NewForwarder(tsURL, 10000, nil)
fw = NewForwarder(tsURL, 10000, nil, false)

req, _ = http.NewRequest("POST", "http://localhost:99999/bar?crazy=true", strings.NewReader(requestText))
req.Host = "testhost"
Expand All @@ -189,7 +227,7 @@ func TestHostForwarding(t *testing.T) {
func TestMultiForwarder(t *testing.T) {
requestText := "This is a request"

fw := NewForwarder(nil, 10000, nil)
fw := NewForwarder(nil, 10000, nil, false)

// Build two backends
backend1ResponseText := "backend1"
Expand Down Expand Up @@ -252,7 +290,7 @@ func TestForwardingWithRedirectPOST(t *testing.T) {
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

fw := NewForwarder(tsURL, 10000, nil)
fw := NewForwarder(tsURL, 10000, nil, false)

req, _ := http.NewRequest("POST", "http://localhost:99999/bar?crazy=true", strings.NewReader(requestText))

Expand All @@ -274,7 +312,7 @@ func TestForwardingWithRedirectGET(t *testing.T) {
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

fw := NewForwarder(tsURL, 10000, nil)
fw := NewForwarder(tsURL, 10000, nil, false)

req, _ := http.NewRequest("GET", "http://localhost:99999/bar?crazy=true", emptyBody())
req.Header.Set("X-Clammit-Backend", tsURL.String())
Expand All @@ -286,3 +324,26 @@ func TestForwardingWithRedirectGET(t *testing.T) {
require.Equal(t, 302, w.StatusCode)
assert.Equal(t, "https://localhost:12345/foobar", w.Header().Get("Location"))
}

func TestForwardingWithRedirectGETAndNoSll(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
defer r.Body.Close()
w.Header().Add("Location", "https://localhost:12345/foobar")
w.WriteHeader(302)
}))
defer ts.Close()
tsURL, _ := url.Parse(ts.URL)

fw := NewForwarder(tsURL, 10000, nil, true)

req, _ := http.NewRequest("GET", "https://localhost:99999/bar?crazy=true", emptyBody())
req.Header.Set("X-Clammit-Backend", tsURL.String())

w := NewTestResponseWriter()

fw.HandleRequest(w, req)

require.Equal(t, 302, w.StatusCode)
assert.Equal(t, "https://localhost:12345/foobar", w.Header().Get("Location"))
}
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ module clammit
go 1.15

require (
github.com/cpuguy83/go-md2man v1.0.10 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
github.com/go-delve/delve v1.6.0 // indirect
github.com/go-delve/delve v1.7.2 // indirect
github.com/stretchr/testify v1.7.0
gopkg.in/gcfg.v1 v1.2.3
gopkg.in/warnings.v0 v0.1.2 // indirect
Expand Down
Loading