diff --git a/dev/devcam/test.go b/dev/devcam/test.go index 3f956dc46..013b487cf 100644 --- a/dev/devcam/test.go +++ b/dev/devcam/test.go @@ -131,7 +131,9 @@ func (c *testCmd) buildSelf() error { func (c *testCmd) runTests(args []string) error { targs := []string{"test"} if !strings.HasSuffix(c.buildGoPath, "-nosqlite") { - targs = append(targs, "--tags=with_sqlite") + targs = append(targs, "--tags=with_sqlite fake_android") + } else { + targs = append(targs, "--tags=fake_android") } if c.short { targs = append(targs, "-short") diff --git a/pkg/client/android/androidx.go b/pkg/client/android/androidx.go index e715fb3b2..2124257d6 100644 --- a/pkg/client/android/androidx.go +++ b/pkg/client/android/androidx.go @@ -48,14 +48,6 @@ import ( // TODO(mpl): distinguish CAMPUT, CAMGET, etc var androidOutput, _ = strconv.ParseBool(os.Getenv("CAMPUT_ANDROID_OUTPUT")) -// IsChild reports whether this process is running as an Android -// child process and should report its output in the form that the -// Android uploader app expects. -func IsChild() bool { - memOnce.Do(startMemGoroutine) - return androidOutput -} - func PreExit() { if !IsChild() { return @@ -84,11 +76,6 @@ func initOnAndroid() { onAndroidCache = dirExists("/data/data") && dirExists("/system/etc") } -func OnAndroid() bool { - detectOnce.Do(initOnAndroid) - return onAndroidCache -} - var pingRx = regexp.MustCompile(`\((.+?)\)`) type namedInt struct { diff --git a/pkg/client/android/androidx_fake.go b/pkg/client/android/androidx_fake.go new file mode 100644 index 000000000..2d5edb91f --- /dev/null +++ b/pkg/client/android/androidx_fake.go @@ -0,0 +1,39 @@ +// +build fake_android + +/* +Copyright 2015 The Camlistore Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package android + +// If non-nil, user-provided hook used by OnAndroid. +var OnAndroidHook func() bool + +// IsChild reports whether this process is running as an Android +// child process and should report its output in the form that the +// Android uploader app expects. +func IsChild() bool { + if OnAndroidHook != nil { + return OnAndroidHook() + } + return false +} + +func OnAndroid() bool { + if OnAndroidHook != nil { + return OnAndroidHook() + } + return false +} diff --git a/pkg/client/android/androidx_real.go b/pkg/client/android/androidx_real.go new file mode 100644 index 000000000..8a2a1c594 --- /dev/null +++ b/pkg/client/android/androidx_real.go @@ -0,0 +1,32 @@ +// +build !fake_android + +/* +Copyright 2015 The Camlistore Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package android + +// IsChild reports whether this process is running as an Android +// child process and should report its output in the form that the +// Android uploader app expects. +func IsChild() bool { + memOnce.Do(startMemGoroutine) + return androidOutput +} + +func OnAndroid() bool { + detectOnce.Do(initOnAndroid) + return onAndroidCache +} diff --git a/pkg/client/client.go b/pkg/client/client.go index 6c6ea8e77..8d5eec014 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -197,18 +197,14 @@ func (c *Client) TransportForConfig(tc *TransportConfig) http.RoundTripper { if c == nil { return nil } - tlsConfig, err := c.TLSConfig() - if err != nil { - log.Fatalf("Error while configuring TLS for client: %v", err) - } var transport http.RoundTripper proxy := http.ProxyFromEnvironment if tc != nil && tc.Proxy != nil { proxy = tc.Proxy } transport = &http.Transport{ + DialTLS: c.DialTLSFunc(), Dial: c.DialFunc(), - TLSClientConfig: tlsConfig, Proxy: proxy, MaxIdleConnsPerHost: maxParallelHTTP, } @@ -817,7 +813,7 @@ func (c *Client) newRequest(method, url string, body ...io.Reader) *http.Request if len(body) > 1 { panic("too many body arguments") } - req, err := http.NewRequest(method, c.condRewriteURL(url), bodyR) + req, err := http.NewRequest(method, url, bodyR) if err != nil { panic(err.Error()) } @@ -854,85 +850,38 @@ func (c *Client) insecureTLS() bool { return c.useTLS() && c.InsecureTLS } -// selfVerifiedSSL returns whether the client config has fingerprints for -// (self-signed) trusted certificates. -// When true, we run with InsecureSkipVerify and it is our responsibility -// to check the server's cert against our trusted certs. -func (c *Client) selfVerifiedSSL() bool { - return c.useTLS() && len(c.getTrustedCerts()) > 0 -} - -// condRewriteURL changes "https://" to "http://" and adds ":443" to -// the host (if no port was specified) when we are in selfVerifiedSSL -// mode. We need to do that because we do the TLS dialing ourselves, -// and we do not want the http transport layer to redo it. -func (c *Client) condRewriteURL(urlStr string) string { - if c.selfVerifiedSSL() || c.insecureTLS() { - u, err := url.Parse(urlStr) - if err != nil { - return urlStr - } - if strings.HasSuffix(u.Host, ":") { - // Here we compensate for the fact that, as of - // 2015-08-24, when the host part ends with a colon (empty - // port), url.Parse only fails when the hostname is an IPv6 - // address (https://github.com/golang/go/issues/12200). - // We instead choose to be consistent and we refuse to - // rewrite any URL that ends with a colon. - return urlStr - } - if u.Scheme == "https" { - // Keep the port 443 if no explicit port was specified. - _, _, err := net.SplitHostPort(u.Host) - if err == nil { - u.Scheme = "http" - return u.String() - } - addrerr, ok := err.(*net.AddrError) - if ok && addrerr.Err == "missing port in address" { - u.Scheme = "http" - u.Host += ":443" - return u.String() - } - } - } - return urlStr -} - -// TLSConfig returns the correct tls.Config depending on whether -// SSL is required, the client's config has some trusted certs, -// and we're on android. -func (c *Client) TLSConfig() (*tls.Config, error) { - if !c.useTLS() { - return nil, nil - } - trustedCerts := c.getTrustedCerts() - if len(trustedCerts) > 0 { - return &tls.Config{InsecureSkipVerify: true}, nil - } - if !android.OnAndroid() { - return nil, nil - } - return android.TLSConfig() -} - -// DialFunc returns the adequate dial function, depending on -// whether SSL is required, the client's config has some trusted -// certs, and we're on android. -// If the client's config has some trusted certs, the server's -// certificate will be checked against those in the config after -// the TLS handshake. +// DialFunc returns the adequate dial function when we're on android. func (c *Client) DialFunc() func(network, addr string) (net.Conn, error) { - trustedCerts := c.getTrustedCerts() - if !c.useTLS() || (!c.InsecureTLS && len(trustedCerts) == 0) { - // No TLS, or TLS with normal/full verification - if android.IsChild() { - return func(network, addr string) (net.Conn, error) { - return android.Dial(network, addr) - } - } + if c.useTLS() { return nil } + if android.IsChild() { + return func(network, addr string) (net.Conn, error) { + return android.Dial(network, addr) + } + } + return nil +} + +// DialTLSFunc returns the adequate dial function, when using SSL, depending on +// whether we're using insecure TLS (certificate verification is disabled), or we +// have some trusted certs, or we're on android. +// If the client's config has some trusted certs, the server's certificate will +// be checked against those in the config after the TLS handshake. +func (c *Client) DialTLSFunc() func(network, addr string) (net.Conn, error) { + if !c.useTLS() { + return nil + } + trustedCerts := c.getTrustedCerts() + var stdTLS bool + if !c.InsecureTLS && len(trustedCerts) == 0 { + // TLS with normal/full verification + stdTLS = true + if !android.IsChild() { + // Not android, so let the stdlib deal with it + return nil + } + } return func(network, addr string) (net.Conn, error) { var conn *tls.Conn @@ -942,7 +891,16 @@ func (c *Client) DialFunc() func(network, addr string) (net.Conn, error) { if err != nil { return nil, err } - conn = tls.Client(con, &tls.Config{InsecureSkipVerify: true}) + var tlsConfig *tls.Config + if stdTLS { + tlsConfig, err = android.TLSConfig() + if err != nil { + return nil, err + } + } else { + tlsConfig = &tls.Config{InsecureSkipVerify: true} + } + conn = tls.Client(con, tlsConfig) if err = conn.Handshake(); err != nil { return nil, err } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go deleted file mode 100644 index 88679aa45..000000000 --- a/pkg/client/client_test.go +++ /dev/null @@ -1,54 +0,0 @@ -/* -Copyright 2015 The Camlistore Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package client - -import "testing" - -var rewritetests = []struct { - in string - out string -}{ - // Valid URLs change the scheme, and add :433 iff there's no port. - {"https://foo.bar:443", "http://foo.bar:443"}, - {"https://foo.bar", "http://foo.bar:443"}, - {"https://foo.bar/", "http://foo.bar:443/"}, - {"https://foo.bar:443/", "http://foo.bar:443/"}, - {"https://foo.bar:baz/", "http://foo.bar:baz/"}, - {"https://[::0]/", "http://[::0]:443/"}, - {"https://[::0]:82/", "http://[::0]:82/"}, - {"https://[2001:DB8::1]:80/", "http://[2001:DB8::1]:80/"}, - {"https://[2001:DB8:0:1]/", "http://[2001:DB8:0:1]:443/"}, - {"https://192.0.2.3/", "http://192.0.2.3:443/"}, - {"https://192.0.2.3:60/", "http://192.0.2.3:60/"}, - // Invalid URLs stay exactly the same. - {"https://[2001:DB8::1:/", "https://[2001:DB8::1:/"}, - {"https://foo.bar:443:baz/", "https://foo.bar:443:baz/"}, - {"https://foo.bar:/", "https://foo.bar:/"}, - {"https://[2001:DB8::1]:/", "https://[2001:DB8::1]:/"}, -} - -func TestCondRewriteURL(t *testing.T) { - c := &Client{InsecureTLS: true} - c.initTrustedCertsOnce.Do(c.initTrustedCerts) // Initialise an empty list of trusted certs. - c.server = "https://example.com/" - for _, tt := range rewritetests { - s := c.condRewriteURL(tt.in) - if s != tt.out { - t.Errorf("c.condRewriteURL(%q) => %q, want %q", tt.in, s, tt.out) - } - } -} diff --git a/pkg/client/config.go b/pkg/client/config.go index e19527e88..764c67ffa 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -295,8 +295,7 @@ func (c *Client) serverOrDefault() string { } func (c *Client) useTLS() bool { - // TODO(mpl): I think this might be wrong, because sometimes c.server is not the one being used? - return strings.HasPrefix(c.server, "https://") + return strings.HasPrefix(c.discoRoot(), "https://") } // SetupAuth sets the client's authMode. It tries from the environment first if we're on android or in dev mode, and then from the client configuration. diff --git a/pkg/client/transport_test.go b/pkg/client/transport_test.go new file mode 100644 index 000000000..7b803bb60 --- /dev/null +++ b/pkg/client/transport_test.go @@ -0,0 +1,175 @@ +// +build fake_android + +/* +Copyright 2015 The Camlistore Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client + +import ( + "net/http" + "testing" + + "camlistore.org/pkg/client/android" + "camlistore.org/pkg/httputil" +) + +var transportTests = []struct { + // input + server string + onAndroid bool + trustedCerts []string + insecureTLS bool + // ouptput + dialFunc bool // whether the transport's Dial is not nil + dialTLSFunc bool // whether the transport's DialTLS is not nil +}{ + // All http, not android. + { + server: "http://example.com", + onAndroid: false, + trustedCerts: nil, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: false, + }, + { + server: "http://example.com", + onAndroid: false, + trustedCerts: nil, + insecureTLS: true, + dialFunc: false, + dialTLSFunc: false, + }, + { + server: "http://example.com", + onAndroid: false, + trustedCerts: []string{"whatever"}, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: false, + }, + + // All http, on android. + { + server: "http://example.com", + onAndroid: true, + trustedCerts: nil, + insecureTLS: false, + dialFunc: true, + dialTLSFunc: false, + }, + { + server: "http://example.com", + onAndroid: true, + trustedCerts: nil, + insecureTLS: true, + dialFunc: true, + dialTLSFunc: false, + }, + { + server: "http://example.com", + onAndroid: true, + trustedCerts: []string{"whatever"}, + insecureTLS: false, + dialFunc: true, + dialTLSFunc: false, + }, + + // All https, not android. + { + server: "https://example.com", + onAndroid: false, + trustedCerts: nil, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: false, + }, + { + server: "https://example.com", + onAndroid: false, + trustedCerts: nil, + insecureTLS: true, + dialFunc: false, + dialTLSFunc: true, + }, + { + server: "https://example.com", + onAndroid: false, + trustedCerts: []string{"whatever"}, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: true, + }, + + // All https, on android. + { + server: "https://example.com", + onAndroid: true, + trustedCerts: nil, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: true, + }, + { + server: "https://example.com", + onAndroid: true, + trustedCerts: nil, + insecureTLS: true, + dialFunc: false, + dialTLSFunc: true, + }, + { + server: "https://example.com", + onAndroid: true, + trustedCerts: []string{"whatever"}, + insecureTLS: false, + dialFunc: false, + dialTLSFunc: true, + }, +} + +func TestTransportSetup(t *testing.T) { + sayNil := func(isNil bool) string { + if isNil { + return "nil" + } + return "not nil" + } + for tti, tt := range transportTests { + cl := &Client{ + paramsOnly: true, + server: tt.server, + trustedCerts: tt.trustedCerts, + InsecureTLS: tt.insecureTLS, + } + android.OnAndroidHook = func() bool { + return tt.onAndroid + } + rt := cl.TransportForConfig(nil) + var tr *http.Transport + if tt.onAndroid { + tr = rt.(*android.StatsTransport).Rt.(*httputil.StatsTransport).Transport.(*http.Transport) + } else { + tr = rt.(*httputil.StatsTransport).Transport.(*http.Transport) + } + if tt.dialTLSFunc != (tr.DialTLS != nil) { + t.Errorf("test %d for %#v: dialTLSFunc should be %v", tti, tt, sayNil(!tt.dialTLSFunc)) + } + if tt.dialFunc != (tr.Dial != nil) { + t.Errorf("test %d for %#v: dialFunc should be %v", tti, tt, sayNil(!tt.dialFunc)) + } + } +}