Cleaned up; Patched identity point & invalid curve attacks..

- Update README.md - Set read-only and write-only channels. - Reorganized code. - Set Content-Type and HSTS on index - House keeping in ecdh.go and symcrypt.go - Fixed bug; user changes weren't being saved.
2026-04-28 11:57:14 +00:00 · 2015-04-11 01:25:50 -07:00
parent 2f4e7fed67
commit e90713a370
6 changed files with 58 additions and 47 deletions
--- a/README.md
+++ b/README.md
@@ -35,7 +35,7 @@ secure) way is to skip the
 [Certificate Authority](https://en.wikipedia.org/wiki/Certificate_authority#Issuing_a_certificate)
 verification and generate a self-signed TLS certificate. Read this
 [detailed guide](http://www.akadia.com/services/ssh_test_certificate.html)
-or, alternatively, follow these unsecure commands:
+or, alternatively, follow these insecure commands:

    $ mkdir cert
    $ chmod 700 cert
@@ -152,11 +152,11 @@ server.

 Example query:

-    $ echo "Why is a raven like a writing desk?"|python -c "print raw_input().encode('base64')"
-    V2h5IGlzIGEgcmF2ZW4gbGlrZSBhIHdyaXRpbmcgZGVzaz8=
+    $ echo "Why is a raven like a writing desk?" | openssl base64
+    V2h5IGlzIGEgcmF2ZW4gbGlrZSBhIHdyaXRpbmcgZGVzaz8K

    $ curl --cacert cert/server.crt https://localhost:8080/encrypt  \
-            -d '{"Name":"Alice","Password":"Lewis","Minimum":2, "Owners":["Alice","Bill","Cat","Dodo"],"Data":"V2h5IGlzIGEgcmF2ZW4gbGlrZSBhIHdyaXRpbmcgZGVzaz8="}'
+            -d '{"Name":"Alice","Password":"Lewis","Minimum":2, "Owners":["Alice","Bill","Cat","Dodo"],"Data":"V2h5IGlzIGEgcmF2ZW4gbGlrZSBhIHdyaXRpbmcgZGVzaz8K"}'
    {"Status":"ok","Response":"eyJWZXJzaW9uIj...NSSllzPSJ9"}

 The data expansion is not tied to the size of the input.
@@ -165,13 +165,14 @@ The data expansion is not tied to the size of the input.

 Decrypt allows an admin to decrypt a piece of data. As long as
 "Minimum" number users from the set of "Owners" have delegated their
-keys to the server, the clear data will be returned.
+keys to the server, a base64 encoded object with the clear data and the
+set of "Owners" whose private keys were used is returned.

 Example query:

    $ curl --cacert cert/server.crt https://localhost:8080/decrypt  \
-            -d {"Name":"Alice","Password":"Lewis","Data":"eyJWZXJzaW9uIj...NSSllzPSJ9"}
-    {"Status":"ok","Response":"V2h5IGlzIGEgcmF2ZW4gbGlrZSBhIHdyaXRpbmcgZGVzaz8="}
+            -d '{"Name":"Alice","Password":"Lewis","Data":"eyJWZXJzaW9uIj...NSSllzPSJ9"}'
+    {"Status":"ok","Response":"eyJEYXRhI...FuMiJdfQ=="}

 If there aren't enough keys delegated you'll see:

--- a/ecdh/ecdh.go
+++ b/ecdh/ecdh.go
@@ -19,13 +19,6 @@ import (

 var Curve = elliptic.P256

-func zero(in []byte) {
-	inLen := len(in)
-	for i := 0; i < inLen; i++ {
-		in[i] ^= in[i]
-	}
-}
-
 // Encrypt secures and authenticates its input using the public key
 // using ECDHE with AES-128-CBC-HMAC-SHA1.
 func Encrypt(pub *ecdsa.PublicKey, in []byte) (out []byte, err error) {
@@ -63,7 +56,7 @@ func Encrypt(pub *ecdsa.PublicKey, in []byte) (out []byte, err error) {
 	return
 }

-// Decrypt authentications and recovers the original message from
+// Decrypt authenticates and recovers the original message from
 // its input using the private key and the ephemeral key included in
 // the message.
 func Decrypt(priv *ecdsa.PrivateKey, in []byte) (out []byte, err error) {
@@ -75,7 +68,8 @@ func Decrypt(priv *ecdsa.PrivateKey, in []byte) (out []byte, err error) {
 	}

 	x, y := elliptic.Unmarshal(Curve(), ephPub)
-	if x == nil {
+	ok := Curve().IsOnCurve(x, y) // Rejects the identity point too.
+	if x == nil || !ok {
 		return nil, errors.New("Invalid public key")
 	}

--- a/index.html
+++ b/index.html
@@ -9,12 +9,11 @@
 	<script src="//cdnjs.cloudflare.com/ajax/libs/jquery/2.0.3/jquery.min.js"></script>
 	<script src="//netdna.bootstrapcdn.com/bootstrap/3.0.2/js/bootstrap.min.js"></script>
 	<style type="text/css">
-		body{padding-top: 50px;}
 		.footer{ border-top: 1px solid #ccc; margin-top: 50px; padding: 20px 0;}
 	</style>
 </head>
 <body>
-	<nav class="navbar navbar-default navbar-fixed-top" role="banner">
+	<nav class="navbar navbar-default" role="banner">
 		<div class="container">
 			<div class="navbar-header">
 				<a href="/" class="navbar-brand">Red October</a>
@@ -77,6 +76,7 @@
 					<button type="submit" class="btn btn-primary">Delegate</button>
 				</form>
 			</div>
+
 		</section>

 		<hr />
@@ -125,7 +125,7 @@
 				</div>
 			</div>

-			<div id="create" class="col-md-6">
+			<div class="col-md-6">
 				<h3 id="create">Create</h3>

 				<form id="user-create" class="ro-user-create" role="form" action="/delegate" method="post">
@@ -225,17 +225,17 @@

 					<div class="form-group row">
 						<div class="col-md-6">
-							<label for="encrypt-user-admin">Admin User</label>
+							<label for="encrypt-user-admin">User name</label>
 							<input type="text" name="Name" class="form-control" id="encrypt-user-admin" placeholder="User name" required />
 						</div>
 						<div class="col-md-6">
-							<label for="encrypt-user-pass">Admin Password</label>
+							<label for="encrypt-user-pass">Password</label>
 							<input type="password" name="Password" class="form-control" id="encrypt-user-pass" placeholder="Password" required />
 						</div>
 					</div>
 					<div class="form-group row">
 						<div class="col-md-6">
-							<label for="encrypt-minimum">Minimum number of user for access</label>
+							<label for="encrypt-minimum">Minimum number of users for access</label>
 							<input type="number" name="Minimum" class="form-control" id="encrypt-minimum" placeholder="2" required />
 						</div>
 						<div class="col-md-6">
@@ -264,11 +264,11 @@

 					<div class="form-group row">
 						<div class="col-md-6">
-							<label for="decrypt-user-admin">Admin User</label>
+							<label for="decrypt-user-admin">User name</label>
 							<input type="text" name="Name" class="form-control" id="decrypt-user-admin" placeholder="User name" required />
 						</div>
 						<div class="col-md-6">
-							<label for="decrypt-user-pass">Admin Password</label>
+							<label for="decrypt-user-pass">Password</label>
 							<input type="password" name="Password" class="form-control" id="decrypt-user-pass" placeholder="Password" required />
 						</div>
 					</div>
@@ -357,7 +357,7 @@
 							var li = $('<li />', {'class': 'list-group-item'}).appendTo(loc);
 							if( user.Uses ){ li.append( $('<span />', {'class': 'badge'}).text(user.Uses+' uses remaining') ); }
 							li.append( $('<h5 />', {'class': 'list-group-item-heading'}).text(key || 'Unknown') );
-							li.append( $('<p />', {'class': 'list-group-item-text'}).html('Type: '+user.Type+ (user.Expiry ? '<br />Expiry: '+user.Expiry : '')) );
+							li.append( $('<p />', {'class': 'list-group-item-text'}).html('Type: '+user.Type+ (user.Expiry ? '<br />Expiry: '+user.Expiry : '')+ (user.Users ? '<br />Users: '+user.Users.join(', ') : '')+ (user.Labels ? '<br />Labels: '+user.Labels.join(', ') : '')) );

 							if( user.Admin ){
 								li.find('h5').append(' (admin)');
--- a/passvault/passvault.go
+++ b/passvault/passvault.go
@@ -457,7 +457,7 @@ func ChangePassword(name, password, newPassword string) (err error) {
 func DeleteRecord(name string) error {
 	if _, ok := GetRecord(name); ok {
 		delete(records.Passwords, name)
-		return nil
+		return WriteRecordsToDisk()
 	}

 	return errors.New("Record missing")
@@ -468,7 +468,7 @@ func RevokeRecord(name string) error {
 	if rec, ok := GetRecord(name); ok {
 		rec.Admin = false
 		SetRecord(rec, name)
-		return nil
+		return WriteRecordsToDisk()
 	}

 	return errors.New("Record missing")
@@ -479,7 +479,7 @@ func MakeAdmin(name string) error {
 	if rec, ok := GetRecord(name); ok {
 		rec.Admin = true
 		SetRecord(rec, name)
-		return nil
+		return WriteRecordsToDisk()
 	}

 	return errors.New("Record missing")
--- a/redoctober.go
+++ b/redoctober.go
@@ -41,7 +41,7 @@ type userRequest struct {
 	// keys of the functions map above
 	in []byte // Arbitrary input data (depends on the core.*
 	// function called)
-	resp chan []byte // Channel down which a response is sent (the
+	resp chan<- []byte // Channel down which a response is sent (the
 	// data sent will depend on the core.* function
 	// called to handle this request)
 }
@@ -50,7 +50,7 @@ type userRequest struct {
 // one of the functions named in the functions map above. It reads the
 // request and sends it to the goroutine started in main() below for
 // processing and then waits for the response.
-func queueRequest(process chan userRequest, requestType string, w http.ResponseWriter, r *http.Request) {
+func queueRequest(process chan<- userRequest, requestType string, w http.ResponseWriter, r *http.Request) {
 	body, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -61,6 +61,10 @@ func queueRequest(process chan userRequest, requestType string, w http.ResponseW
 	process <- userRequest{rt: requestType, in: body, resp: response}

 	if resp, ok := <-response; ok {
+		header := w.Header()
+		header.Set("Content-Type", "application/json")
+		header.Set("Strict-Transport-Security", "max-age=86400; includeSubDomains; preload")
+
 		w.Write(resp)
 	} else {
 		http.Error(w, "Unknown request", http.StatusInternalServerError)
@@ -74,12 +78,7 @@ func queueRequest(process chan userRequest, requestType string, w http.ResponseW
 //
 // Returns a valid http.Server handling redoctober JSON requests (and
 // its associated listener) or an error
-func NewServer(process chan userRequest, staticPath, addr, certPath, keyPath, caPath string) (*http.Server, *net.Listener, error) {
-	mux := http.NewServeMux()
-	srv := http.Server{
-		Addr:    addr,
-		Handler: mux,
-	}
+func NewServer(process chan<- userRequest, staticPath, addr, certPath, keyPath, caPath string) (*http.Server, *net.Listener, error) {
 	cert, err := tls.LoadX509KeyPair(certPath, keyPath)
 	if err != nil {
 		return nil, nil, fmt.Errorf("Error loading certificate (%s, %s): %s", certPath, keyPath, err)
@@ -90,27 +89,31 @@ func NewServer(process chan userRequest, staticPath, addr, certPath, keyPath, ca
 		Rand:         rand.Reader,
 		PreferServerCipherSuites: true,
 		SessionTicketsDisabled:   true,
+		MinVersion: tls.VersionTLS10,
 	}

 	// If a caPath has been specified then a local CA is being used
 	// and not the system configuration.

 	if caPath != "" {
-		rootPool := x509.NewCertPool()
 		pemCert, err := ioutil.ReadFile(caPath)
 		if err != nil {
 			return nil, nil, fmt.Errorf("Error reading %s: %s\n", caPath, err)
 		}
-		derCert, pemCert := pem.Decode(pemCert)
+
+		derCert, _ := pem.Decode(pemCert)
 		if derCert == nil {
-			return nil, nil, fmt.Errorf("Error decoding CA certificate: %s\n", err)
+			return nil, nil, fmt.Errorf("No PEM data was found in the CA certificate file\n")
 		}
+
 		cert, err := x509.ParseCertificate(derCert.Bytes)
 		if err != nil {
 			return nil, nil, fmt.Errorf("Error parsing CA certificate: %s\n", err)
 		}

+		rootPool := x509.NewCertPool()
 		rootPool.AddCert(cert)
+
 		config.ClientAuth = tls.RequireAndVerifyClientCert
 		config.ClientCAs = rootPool
 	}
@@ -122,10 +125,12 @@ func NewServer(process chan userRequest, staticPath, addr, certPath, keyPath, ca

 	lstnr := tls.NewListener(conn, &config)

+	mux := http.NewServeMux()
+
 	// queue up post URIs
 	for current := range functions {
 		// copy this so reference does not get overwritten
-		var requestType = current
+		requestType := current
 		mux.HandleFunc(requestType, func(w http.ResponseWriter, r *http.Request) {
 			queueRequest(process, requestType, w, r)
 		})
@@ -136,6 +141,11 @@ func NewServer(process chan userRequest, staticPath, addr, certPath, keyPath, ca
 	mux.HandleFunc("/index", idxHandler.handle)
 	mux.HandleFunc("/", idxHandler.handle)

+	srv := http.Server{
+		Addr:    addr,
+		Handler: mux,
+	}
+
 	return &srv, &lstnr, nil
 }

@@ -156,6 +166,12 @@ func (this *indexHandler) handle(w http.ResponseWriter, r *http.Request) {
 	} else {
 		body = bytes.NewReader(indexHtml)
 	}
+
+	header := w.Header()
+	header.Set("Content-Type", "text/html")
+	header.Set("Strict-Transport-Security", "max-age=86400; includeSubDomains; preload")
+	// If the server isn't HTTPS worthy, the HSTS header won't be honored.
+
 	http.ServeContent(w, r, "index.html", time.Now(), body)
 }

@@ -240,12 +256,11 @@ var indexHtml = []byte(`<!DOCTYPE html>
 	<script src="//cdnjs.cloudflare.com/ajax/libs/jquery/2.0.3/jquery.min.js"></script>
 	<script src="//netdna.bootstrapcdn.com/bootstrap/3.0.2/js/bootstrap.min.js"></script>
 	<style type="text/css">
-		body{padding-top: 50px;}
 		.footer{ border-top: 1px solid #ccc; margin-top: 50px; padding: 20px 0;}
 	</style>
 </head>
 <body>
-	<nav class="navbar navbar-default navbar-fixed-top" role="banner">
+	<nav class="navbar navbar-default" role="banner">
 		<div class="container">
 			<div class="navbar-header">
 				<a href="/" class="navbar-brand">Red October</a>
@@ -356,7 +371,7 @@ var indexHtml = []byte(`<!DOCTYPE html>
 				</div>
 			</div>

-			<div id="create" class="col-md-6">
+			<div class="col-md-6">
 				<h3 id="create">Create</h3>

 				<form id="user-create" class="ro-user-create" role="form" action="/delegate" method="post">
@@ -466,7 +481,7 @@ var indexHtml = []byte(`<!DOCTYPE html>
 					</div>
 					<div class="form-group row">
 						<div class="col-md-6">
-							<label for="encrypt-minimum">Minimum number of user for access</label>
+							<label for="encrypt-minimum">Minimum number of users for access</label>
 							<input type="number" name="Minimum" class="form-control" id="encrypt-minimum" placeholder="2" required />
 						</div>
 						<div class="col-md-6">
@@ -589,6 +604,7 @@ var indexHtml = []byte(`<!DOCTYPE html>
 							if( user.Uses ){ li.append( $('<span />', {'class': 'badge'}).text(user.Uses+' uses remaining') ); }
 							li.append( $('<h5 />', {'class': 'list-group-item-heading'}).text(key || 'Unknown') );
 							li.append( $('<p />', {'class': 'list-group-item-text'}).html('Type: '+user.Type+ (user.Expiry ? '<br />Expiry: '+user.Expiry : '')+ (user.Users ? '<br />Users: '+user.Users.join(', ') : '')+ (user.Labels ? '<br />Labels: '+user.Labels.join(', ') : '')) );
+
 							if( user.Admin ){
 								li.find('h5').append(' (admin)');
 							}
@@ -717,4 +733,5 @@ var indexHtml = []byte(`<!DOCTYPE html>
 			});
 		});
 	</script>
-</body>`)
+</body>
+</html>`)
--- a/symcrypt/symcrypt.go
+++ b/symcrypt/symcrypt.go
@@ -5,7 +5,6 @@ import (
 	"crypto/aes"
 	"crypto/cipher"
 	"crypto/rand"
-	"io"
 )

 // DecryptCBC decrypt bytes using a key and IV with AES in CBC mode.
@@ -41,6 +40,6 @@ func EncryptCBC(data, iv, key []byte) (encryptedData []byte, err error) {
 // MakeRandom is a helper that makes a new buffer full of random data.
 func MakeRandom(length int) ([]byte, error) {
 	bytes := make([]byte, length)
-	_, err := io.ReadFull(rand.Reader, bytes)
+	_, err := rand.Read(bytes)
 	return bytes, err
 }