Repository: osbuild/osbuild-composer
Pull request #59 (2019-10-29 15:58:17)

Plain text passwords
Not sure if I should not just switch to use crypt_gensalt for generating salt. I had wrote my own salt generation function before I decided to embed the c code.


Patch:
diff --git a/internal/blueprint/customizations.go b/internal/blueprint/customizations.go
index f5f58eab7..7497c202d 100644
--- a/internal/blueprint/customizations.go
+++ b/internal/blueprint/customizations.go
@@ -1,6 +1,7 @@
 package blueprint
 
 import (
+	"github.com/osbuild/osbuild-composer/internal/crypt"
 	"github.com/osbuild/osbuild-composer/internal/pipeline"
 	"strconv"
 	"strings"
@@ -161,8 +162,16 @@ func (c *Customizations) customizeUserAndSSHKey(p *pipeline.Pipeline) error {
 
 	users := make(map[string]pipeline.UsersStageOptionsUser)
 	for _, user := range c.User {
-		// TODO: only hashed password are currently supported as an input
-		// plain-text passwords should be also supported due to parity with lorax-composer
+
+		if user.Password != nil && !crypt.PasswordIsCrypted(*user.Password) {
+			cryptedPassword, err := crypt.CryptSHA512(*user.Password)
+			if err != nil {
+				return err
+			}
+
+			user.Password = &cryptedPassword
+		}
+
 		userData := pipeline.UsersStageOptionsUser{
 			Groups:      user.Groups,
 			Description: user.Description,
diff --git a/internal/crypt/crypt.go b/internal/crypt/crypt.go
new file mode 100644
index 000000000..48adce4a7
--- /dev/null
+++ b/internal/crypt/crypt.go
@@ -0,0 +1,49 @@
+package crypt
+
+import (
+	"crypto/rand"
+	"math/big"
+	"strings"
+)
+
+func CryptSHA512(phrase string) (string, error) {
+	const SHA512SaltLength = 16
+
+	salt, err := genSalt(SHA512SaltLength)
+
+	if err != nil {
+		return "", nil
+	}
+
+	hashSettings := "$6$" + salt
+	return crypt(phrase, hashSettings)
+}
+
+func genSalt(length int) (string, error) {
+	saltChars := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789./"
+
+	b := make([]byte, length)
+
+	for i := range b {
+		runeIndex, err := rand.Int(rand.Reader, big.NewInt(int64(len(saltChars))))
+		if err != nil {
+			return "", err
+		}
+		b[i] = saltChars[runeIndex.Int64()]
+	}
+
+	return string(b), nil
+}
+
+func PasswordIsCrypted(s string) bool {
+	// taken from lorax src: src/pylorax/api/compose.py:533
+	prefixes := [...]string{"$2b$", "$6$", "$5$"}
+
+	for _, prefix := range prefixes {
+		if strings.HasPrefix(s, prefix) {
+			return true
+		}
+	}
+
+	return false
+}
diff --git a/internal/crypt/crypt_impl.go b/internal/crypt/crypt_impl.go
new file mode 100644
index 000000000..088cf4462
--- /dev/null
+++ b/internal/crypt/crypt_impl.go
@@ -0,0 +1,85 @@
+// Copied from https://github.com/amoghe/go-crypt/blob/b3e291286513a0c993f7c4dd7060d327d2d56143/crypt_r.go
+// Original sources are under MIT license:
+// The MIT License (MIT)
+//
+// Copyright (c) 2015 Akshay Moghe
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+//
+// Package crypt provides wrappers around functions available in crypt.h
+//
+// It wraps around the GNU specific extension (crypt_r) when it is available
+// (i.e. where GOOS=linux). This makes the go function reentrant (and thus
+// callable from concurrent goroutines).
+package crypt
+
+import (
+	"unsafe"
+)
+
+/*
+	#cgo LDFLAGS: -lcrypt
+
+	// this is needed for Ubuntu
+	#define _GNU_SOURCE
+
+	#include <stdlib.h>
+	#include <string.h>
+	#include <crypt.h>
+
+	char *gnu_ext_crypt(char *pass, char *salt) {
+		char *enc = NULL;
+		char *ret = NULL;
+		struct crypt_data data;
+		data.initialized = 0;
+
+		enc = crypt_r(pass, salt, &data);
+		if(enc == NULL) {
+			return NULL;
+		}
+
+		ret = (char *)malloc((strlen(enc)+1) * sizeof(char)); // for trailing null
+		strcpy(ret, enc);
+		ret[strlen(enc)]= '\0';
+
+	 return ret;
+	}
+*/
+import "C"
+
+// Crypt provides a wrapper around the glibc crypt_r() function.
+// For the meaning of the arguments, refer to the package README.
+func crypt(pass, salt string) (string, error) {
+	c_pass := C.CString(pass)
+	defer C.free(unsafe.Pointer(c_pass))
+
+	c_salt := C.CString(salt)
+	defer C.free(unsafe.Pointer(c_salt))
+
+	c_enc, err := C.gnu_ext_crypt(c_pass, c_salt)
+	if c_enc == nil {
+		return "", err
+	}
+	defer C.free(unsafe.Pointer(c_enc))
+
+	// Return nil error if the string is non-nil.
+	// As per the errno.h manpage, functions are allowed to set errno
+	// on success. Caller should ignore errno on success.
+	return C.GoString(c_enc), nil
+}


Test patch:
diff --git a/internal/crypt/crypt_test.go b/internal/crypt/crypt_test.go
new file mode 100644
index 000000000..4a29404b8
--- /dev/null
+++ b/internal/crypt/crypt_test.go
@@ -0,0 +1,43 @@
+package crypt
+
+import (
+	"testing"
+)
+
+func Test_crypt_PasswordIsCrypted(t *testing.T) {
+
+	tests := []struct {
+		name     string
+		password string
+		want     bool
+	}{
+		{
+			name:     "bcrypt",
+			password: "$2b$04$123465789012345678901uac5A8egfBuZVHMrDZsQzR96IqNBivCy",
+			want:     true,
+		}, {
+			name:     "sha256",
+			password: "$5$1234567890123456$v.2bOKKLlpmUSKn0rxJmgnh.e3wOKivAVNZmNrOsoA3",
+			want:     true,
+		}, {
+			name:     "sha512",
+			password: "$6$1234567890123456$d.pgKQFaiD8bRiExg5NesbGR/3u51YvxeYaQXPzx4C6oSYREw8VoReiuYZjx0V9OhGVTZFqhc6emAxT1RC5BV.",
+			want:     true,
+		}, {
+			name:     "scrypt",
+			password: "$7$123456789012345", //not actual hash output from scrypt
+			want:     false,
+		}, {
+			name:     "plain",
+			password: "password",
+			want:     false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := PasswordIsCrypted(test.password); got != test.want {
+				t.Errorf("PasswordIsCrypted() =%v, want %v", got, test.want)
+			}
+		})
+	}
+}
diff --git a/internal/weldr/api_test.go b/internal/weldr/api_test.go
index 783a736b8..c786a1a26 100644
--- a/internal/weldr/api_test.go
+++ b/internal/weldr/api_test.go
@@ -78,7 +78,37 @@ func sendHTTP(api *weldr.API, external bool, method, path, body string) *http.Re
 	}
 }
 
-func testRoute(t *testing.T, api *weldr.API, external bool, method, path, body string, expectedStatus int, expectedJSON string) {
+// this function serves to drop fields that shouldn't be tested from the unmarshalled json objects
+func dropFields(obj interface{}, fields ...string) {
+	switch v := obj.(type) {
+	// if the interface type is a map attempt to delete the fields
+	case map[string]interface{}:
+		for i, field := range fields {
+			if _, ok := v[field]; ok {
+				delete(v, field)
+				// if the field is found remove it from the fields slice
+				if len(fields) < i-1 {
+					fields = append(fields[:i], fields[i+1:]...)
+				} else {
+					fields = fields[:i]
+				}
+			}
+		}
+		// call dropFields on the remaining elements since they may contain a map containing the field
+		for _, val := range v {
+			dropFields(val, fields...)
+		}
+	// if the type is a list of interfaces call dropFields on each interface
+	case []interface{}:
+		for _, element := range v {
+			dropFields(element, fields...)
+		}
+	default:
+		return
+	}
+}
+
+func testRoute(t *testing.T, api *weldr.API, external bool, method, path, body string, expectedStatus int, expectedJSON string, ignoreFields ...string) {
 	resp := sendHTTP(api, external, method, path, body)
 	if resp == nil {
 		t.Skip("This test is for internal testing only")
@@ -119,6 +149,9 @@ func testRoute(t *testing.T, api *weldr.API, external bool, method, path, body s
 		return
 	}
 
+	dropFields(reply, ignoreFields...)
+	dropFields(expected, ignoreFields...)
+
 	if !reflect.DeepEqual(reply, expected) {
 		t.Errorf("%s: reply != expected:\n   reply: %s\nexpected: %s", path, strings.TrimSpace(string(replyJSON)), expectedJSON)
 		return
@@ -273,15 +306,16 @@ func TestCompose(t *testing.T) {
 		Body           string
 		ExpectedStatus int
 		ExpectedJSON   string
+		IgnoreFields   []string
 	}{
-		{true, "POST", "/api/v0/compose", `{"blueprint_name": "http-server","compose_type": "tar","branch": "master"}`, http.StatusBadRequest, `{"status":false,"errors":[{"id":"UnknownBlueprint","msg":"Unknown blueprint name: http-server"}]}`},
-		{false, "POST", "/api/v0/compose", `{"blueprint_name": "test","compose_type": "tar","branch": "master"}`, http.StatusOK, `*`},
+		{true, "POST", "/api/v0/compose", `{"blueprint_name": "http-server","compose_type": "tar","branch": "master"}`, http.StatusBadRequest, `{"status":false,"errors":[{"id":"UnknownBlueprint","msg":"Unknown blueprint name: http-server"}]}`, []string{"build_id"}},
+		{false, "POST", "/api/v0/compose", `{"blueprint_name": "test","compose_type": "tar","branch": "master"}`, http.StatusOK, `{"status": true}`, []string{"build_id"}},
 	}
 
 	for _, c := range cases {
 		api := weldr.New(repo, packages, nil, store.New(nil))
 		sendHTTP(api, c.External, "POST", "/api/v0/blueprints/new", `{"name":"test","description":"Test","packages":[{"name":"httpd","version":"2.4.*"}],"version":"0.0.0"}`)
-		testRoute(t, api, c.External, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON)
+		testRoute(t, api, c.External, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON, c.IgnoreFields...)
 		sendHTTP(api, c.External, "DELETE", "/api/v0/blueprints/delete/test", ``)
 	}
 }
@@ -293,8 +327,9 @@ func TestComposeQueue(t *testing.T) {
 		Body           string
 		ExpectedStatus int
 		ExpectedJSON   string
+		IgnoreFields   []string
 	}{
-		{"GET", "/api/v0/compose/queue", ``, http.StatusOK, `*`}, // TODO: we need a way to verify we actually get the expected json here
+		{"GET", "/api/v0/compose/queue", ``, http.StatusOK, `{"new":[{"blueprint":"test","version":"0.0.0","compose_type":"tar","image_size":0,"queue_status":"WAITING"}],"run":[{"blueprint":"test","version":"0.0.0","compose_type":"tar","image_size":0,"queue_status":"RUNNING"}]}`, []string{"id", "job_created", "job_started"}},
 	}
 
 	if len(os.Getenv("OSBUILD_COMPOSER_TEST_EXTERNAL")) > 0 {
@@ -318,7 +353,8 @@ func TestComposeQueue(t *testing.T) {
 		sendHTTP(api, false, "POST", "/api/v0/compose", `{"blueprint_name": "test","compose_type": "tar","branch": "master"}`)
 		job = s.PopCompose()
 		s.UpdateCompose(job.ComposeID, "FAILED")
-		testRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON)
+
+		testRoute(t, api, false, c.Method, c.Path, c.Body, c.ExpectedStatus, c.ExpectedJSON, c.IgnoreFields...)
 		sendHTTP(api, false, "DELETE", "/api/v0/blueprints/delete/test", ``)
 	}
 }