Skip to content

Revert "vicadmin redirect/force TLS (#7960)" #8165

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.

Sign up for GitHub

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

Merged
zjs merged 1 commit into from
Aug 7, 2018
Merged

Revert "vicadmin redirect/force TLS (#7960)" #8165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
98 changes: 4 additions & 94 deletions cmd/vicadmin/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import (
"compress/gzip"
"crypto/tls"
"crypto/x509"
"fmt"
"html/template"
"io"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -77,83 +75,6 @@ const (
genericErrorMessage = "Internal Server Error; see /var/log/vic/vicadmin.log for details" // for http errors that shouldn't be displayed in the browser to the user
)

// Conn is a wrapper struct around net.Conn that implements custom functionality (TLS Check)
type Conn struct {
net.Conn
b byte
err error
UncertainTLS bool
}

// Read checks for TLS in the connection and returns number of bytes read
func (c *Conn) Read(b []byte) (int, error) {
// one time check to determine if TLS is in the connection
if c.UncertainTLS {
if len(b) == 0 {
return 0, fmt.Errorf("invalid length of byte array, cannot proceed with TLS check")
}
c.UncertainTLS = false
b[0] = c.b
// if there's more bytes to read
if len(b) > 1 && c.err == nil {
// recurse on next byte
n, e := c.Conn.Read(b[1:])
// close connection if error during reading
if e != nil {
c.Conn.Close()
}
// return total number of bytes read (+ current) and pass error e
return n + 1, e
}
// only one byte read
return 1, c.err
}
// using the default Conn read
return c.Conn.Read(b)
}

// TLSRedirectListener is a wrapper struct around net.Listener that implements custom functionality (TLS Check)
type TLSRedirectListener struct {
net.Listener
addr string
config *tls.Config
}

// Accept overrides the default listener Accept and adds a TLS check
func (l *TLSRedirectListener) Accept() (net.Conn, error) {
// call default net.listener.Accept first
c, err := l.Listener.Accept()
if err != nil {
return nil, err
}
// create slice of size 1 for the first byte
b := make([]byte, 1)
// regular Conn read, b will be updated with byte val
_, err = c.Read(b)
if err != nil {
c.Close()
if err != io.EOF {
return nil, err
}
}
// wrap net.Conn in custom Conn struct
con := &Conn{
Conn: c,
b: b[0],
err: err,
UncertainTLS: true,
}
// First byte is the hex byte 0x16 = 22
// which means that this is a TLS “handshake” record
if b[0] == 22 {
// HTTPS creates Conn from our custom con/TLS config
return tls.Server(con, l.config), nil
}

// regular HTTP connection, return con
return con, nil
}

func (s *server) listen() error {
defer trace.End(trace.Begin(""))

Expand All @@ -174,6 +95,7 @@ func (s *server) listen() error {
}
if err != nil {
log.Errorf("Could not load certificate from config - running without TLS: %s", err)

s.l, err = net.Listen("tcp", s.addr)
return err
}
Expand Down Expand Up @@ -223,7 +145,7 @@ func (s *server) listen() error {
return err
}

s.l = &TLSRedirectListener{Listener: innerListener, config: tlsconfig}
s.l = tls.NewListener(innerListener, tlsconfig)
return nil
}

Expand All @@ -236,22 +158,9 @@ func (s *server) AuthenticatedHandle(link string, h http.Handler) {
s.Authenticated(link, h.ServeHTTP)
}

// HTTPSRedirectHandle Redirects HTTP to HTTPS
func HTTPSRedirectHandle(h http.Handler) http.Handler {
redirectToHTTPS := func(w http.ResponseWriter, r *http.Request) {
// If TLS is nil, request is not HTTPS, so we must redirect
if r.TLS == nil {
http.Redirect(w, r, fmt.Sprintf("https://%s%s", r.Host, r.RequestURI), http.StatusMovedPermanently)
return
}
h.ServeHTTP(w, r)
}
return http.HandlerFunc(redirectToHTTPS)
}

func (s *server) Handle(link string, h http.Handler) {
log.Debugf("%s --- %s", time.Now().String(), link)
s.mux.Handle(link, gorillacontext.ClearHandler(HTTPSRedirectHandle(h)))
s.mux.Handle(link, gorillacontext.ClearHandler(h))
}

// Enforces authentication on route `link` and runs `handler` on successful auth
Expand Down Expand Up @@ -350,6 +259,7 @@ func (s *server) Authenticated(link string, handler func(http.ResponseWriter, *h
s.logoutHandler(w, r)
return
}

// if the date & remote IP on the cookie were valid, then the user is authenticated
log.Infof("User with a valid auth cookie at %s is authenticated.", connectingAddr[0])
handler(w, r)
Expand Down
6 changes: 0 additions & 6 deletions tests/test-cases/Group9-VIC-Admin/9-01-VICAdmin-ShowHTML.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ Get Login Page
${rc} ${output}= Run And Return Rc And Output curl -sk %{VIC-ADMIN}/authentication
Should contain ${output} <title>VCH Admin</title>

Check that HTTP Request Redirects to HTTPS
${rc} ${output}= Run And Return Rc And Output curl -L -sk -vvv http://%{VCH-IP}:2378
Should Contain ${output} SSL connection using TLS
Should Contain ${output} 301 Moved Permanently
Should not contain ${output} Empty reply from server

While Logged Out Fail To Display HTML
${rc} ${output}= Run And Return Rc And Output curl -sk %{VIC-ADMIN}
Should not contain ${output} <title>VIC: %{VCH-NAME}</title>
Expand Down