From a5be7b677e63523e7fea1c41f12a19d5c1bba8b4 Mon Sep 17 00:00:00 2001 From: Davide N Date: Tue, 25 Mar 2025 15:10:45 +0100 Subject: [PATCH 01/14] make the signaturePubKey overwritable --- conn.go | 157 +++++++++++++++++++++-------------------- globals/globals.go | 4 +- main.go | 20 ++++-- main_test.go | 10 +-- tools/download_test.go | 6 +- tools/tools.go | 5 +- utilities/utilities.go | 39 ++++++---- v2/http.go | 5 +- v2/pkgs/tools.go | 18 +++-- v2/pkgs/tools_test.go | 10 +-- 10 files changed, 157 insertions(+), 117 deletions(-) diff --git a/conn.go b/conn.go index b6e03268c..a9d8f64dd 100644 --- a/conn.go +++ b/conn.go @@ -19,6 +19,7 @@ package main import ( "bytes" + "crypto/rsa" "encoding/json" "errors" "fmt" @@ -79,111 +80,113 @@ type Upload struct { var uploadStatusStr = "ProgrammerStatus" -func uploadHandler(c *gin.Context) { - data := new(Upload) - if err := c.BindJSON(data); err != nil { - c.String(http.StatusBadRequest, fmt.Sprintf("err with the payload. %v", err.Error())) - return - } - - log.Printf("%+v %+v %+v %+v %+v %+v", data.Port, data.Board, data.Rewrite, data.Commandline, data.Extra, data.Filename) - - if data.Port == "" { - c.String(http.StatusBadRequest, "port is required") - return - } - - if data.Board == "" { - c.String(http.StatusBadRequest, "board is required") - log.Error("board is required") - return - } - - if !data.Extra.Network { - if data.Signature == "" { - c.String(http.StatusBadRequest, "signature is required") +func uploadHandler(pubKey *rsa.PublicKey) func(*gin.Context) { + return func(c *gin.Context) { + data := new(Upload) + if err := c.BindJSON(data); err != nil { + c.String(http.StatusBadRequest, fmt.Sprintf("err with the payload. %v", err.Error())) return } - if data.Commandline == "" { - c.String(http.StatusBadRequest, "commandline is required for local board") + log.Printf("%+v %+v %+v %+v %+v %+v", data.Port, data.Board, data.Rewrite, data.Commandline, data.Extra, data.Filename) + + if data.Port == "" { + c.String(http.StatusBadRequest, "port is required") return } - err := utilities.VerifyInput(data.Commandline, data.Signature) - - if err != nil { - c.String(http.StatusBadRequest, "signature is invalid") + if data.Board == "" { + c.String(http.StatusBadRequest, "board is required") + log.Error("board is required") return } - } - buffer := bytes.NewBuffer(data.Hex) + if !data.Extra.Network { + if data.Signature == "" { + c.String(http.StatusBadRequest, "signature is required") + return + } - filePath, err := utilities.SaveFileonTempDir(data.Filename, buffer) - if err != nil { - c.String(http.StatusBadRequest, err.Error()) - return - } + if data.Commandline == "" { + c.String(http.StatusBadRequest, "commandline is required for local board") + return + } - tmpdir, err := os.MkdirTemp("", "extrafiles") - if err != nil { - c.String(http.StatusBadRequest, err.Error()) - return - } + err := utilities.VerifyInput(data.Commandline, data.Signature, pubKey) - for _, extraFile := range data.ExtraFiles { - path, err := utilities.SafeJoin(tmpdir, extraFile.Filename) - if err != nil { - c.String(http.StatusBadRequest, err.Error()) - return + if err != nil { + c.String(http.StatusBadRequest, "signature is invalid") + return + } } - log.Printf("Saving %s on %s", extraFile.Filename, path) - err = os.MkdirAll(filepath.Dir(path), 0744) - if err != nil { - c.String(http.StatusBadRequest, err.Error()) - return - } + buffer := bytes.NewBuffer(data.Hex) - err = os.WriteFile(path, extraFile.Hex, 0644) + filePath, err := utilities.SaveFileonTempDir(data.Filename, buffer) if err != nil { c.String(http.StatusBadRequest, err.Error()) return } - } - if data.Rewrite != "" { - data.Board = data.Rewrite - } - - go func() { - // Resolve commandline - commandline, err := upload.PartiallyResolve(data.Board, filePath, tmpdir, data.Commandline, data.Extra, Tools) + tmpdir, err := os.MkdirTemp("", "extrafiles") if err != nil { - send(map[string]string{uploadStatusStr: "Error", "Msg": err.Error()}) + c.String(http.StatusBadRequest, err.Error()) return } - l := PLogger{Verbose: true} - - // Upload - if data.Extra.Network { - err = errors.New("network upload is not supported anymore, pease use OTA instead") - } else { - send(map[string]string{uploadStatusStr: "Starting", "Cmd": "Serial"}) - err = upload.Serial(data.Port, commandline, data.Extra, l) + for _, extraFile := range data.ExtraFiles { + path, err := utilities.SafeJoin(tmpdir, extraFile.Filename) + if err != nil { + c.String(http.StatusBadRequest, err.Error()) + return + } + log.Printf("Saving %s on %s", extraFile.Filename, path) + + err = os.MkdirAll(filepath.Dir(path), 0744) + if err != nil { + c.String(http.StatusBadRequest, err.Error()) + return + } + + err = os.WriteFile(path, extraFile.Hex, 0644) + if err != nil { + c.String(http.StatusBadRequest, err.Error()) + return + } } - // Handle result - if err != nil { - send(map[string]string{uploadStatusStr: "Error", "Msg": err.Error()}) - return + if data.Rewrite != "" { + data.Board = data.Rewrite } - send(map[string]string{uploadStatusStr: "Done", "Flash": "Ok"}) - }() - c.String(http.StatusAccepted, "") + go func() { + // Resolve commandline + commandline, err := upload.PartiallyResolve(data.Board, filePath, tmpdir, data.Commandline, data.Extra, Tools) + if err != nil { + send(map[string]string{uploadStatusStr: "Error", "Msg": err.Error()}) + return + } + + l := PLogger{Verbose: true} + + // Upload + if data.Extra.Network { + err = errors.New("network upload is not supported anymore, pease use OTA instead") + } else { + send(map[string]string{uploadStatusStr: "Starting", "Cmd": "Serial"}) + err = upload.Serial(data.Port, commandline, data.Extra, l) + } + + // Handle result + if err != nil { + send(map[string]string{uploadStatusStr: "Error", "Msg": err.Error()}) + return + } + send(map[string]string{uploadStatusStr: "Done", "Flash": "Ok"}) + }() + + c.String(http.StatusAccepted, "") + } } // PLogger sends the info from the upload to the websocket diff --git a/globals/globals.go b/globals/globals.go index d7cb09a17..8da580fd5 100644 --- a/globals/globals.go +++ b/globals/globals.go @@ -17,6 +17,6 @@ package globals // DefaultIndexURL is the default index url var ( - // SignatureKey is the public key used to verify commands and url sent by the builder - SignatureKey = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvc0yZr1yUSen7qmE3cxF\nIE12rCksDnqR+Hp7o0nGi9123eCSFcJ7CkIRC8F+8JMhgI3zNqn4cUEn47I3RKD1\nZChPUCMiJCvbLbloxfdJrUi7gcSgUXrlKQStOKF5Iz7xv1M4XOP3JtjXLGo3EnJ1\npFgdWTOyoSrA8/w1rck4c/ISXZSinVAggPxmLwVEAAln6Itj6giIZHKvA2fL2o8z\nCeK057Lu8X6u2CG8tRWSQzVoKIQw/PKK6CNXCAy8vo4EkXudRutnEYHEJlPkVgPn\n2qP06GI+I+9zKE37iqj0k1/wFaCVXHXIvn06YrmjQw6I0dDj/60Wvi500FuRVpn9\ntwIDAQAB\n-----END PUBLIC KEY-----" + // ArduinoSignaturePubKey is the public key used to verify commands and url sent by the builder + ArduinoSignaturePubKey = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvc0yZr1yUSen7qmE3cxF\nIE12rCksDnqR+Hp7o0nGi9123eCSFcJ7CkIRC8F+8JMhgI3zNqn4cUEn47I3RKD1\nZChPUCMiJCvbLbloxfdJrUi7gcSgUXrlKQStOKF5Iz7xv1M4XOP3JtjXLGo3EnJ1\npFgdWTOyoSrA8/w1rck4c/ISXZSinVAggPxmLwVEAAln6Itj6giIZHKvA2fL2o8z\nCeK057Lu8X6u2CG8tRWSQzVoKIQw/PKK6CNXCAy8vo4EkXudRutnEYHEJlPkVgPn\n2qP06GI+I+9zKE37iqj0k1/wFaCVXHXIvn06YrmjQw6I0dDj/60Wvi500FuRVpn9\ntwIDAQAB\n-----END PUBLIC KEY-----" ) diff --git a/main.go b/main.go index 1ca857b02..c6ef81e64 100755 --- a/main.go +++ b/main.go @@ -22,6 +22,7 @@ import ( _ "embed" "encoding/json" "flag" + "fmt" "html/template" "io" "os" @@ -81,7 +82,7 @@ var ( logDump = iniConf.String("log", "off", "off = (default)") origins = iniConf.String("origins", "", "Allowed origin list for CORS") portsFilterRegexp = iniConf.String("regex", "usb|acm|com", "Regular expression to filter serial port list") - signatureKey = iniConf.String("signatureKey", globals.SignatureKey, "Pem-encoded public key to verify signed commandlines") + signatureKey = iniConf.String("signatureKey", globals.ArduinoSignaturePubKey, "Pem-encoded public key to verify signed commandlines") updateURL = iniConf.String("updateUrl", "", "") verbose = iniConf.Bool("v", true, "show debug logging") crashreport = iniConf.Bool("crashreport", false, "enable crashreport logging") @@ -278,9 +279,20 @@ func loop() { } } + if signatureKey == nil { + log.Panicf("signature public key cannot be nil") + } + if len(*signatureKey) == 0 { + log.Panicf("signature public key cannot be empty") + } + signaturePubKey, err := utilities.ParseRsaPublicKey(*signatureKey) + if err != nil { + log.Panicf("cannot parse signature key: %s", err) + } + // Instantiate Index and Tools Index = index.Init(*indexURL, config.GetDataDir()) - Tools = tools.New(config.GetDataDir(), Index, logger) + Tools = tools.New(config.GetDataDir(), Index, logger, signaturePubKey) // see if we are supposed to wait 5 seconds if *isLaunchSelf { @@ -454,7 +466,7 @@ func loop() { r.LoadHTMLFiles("templates/nofirefox.html") r.GET("/", homeHandler) - r.POST("/upload", uploadHandler) + r.POST("/upload", uploadHandler(signaturePubKey)) r.GET("/socket.io/", socketHandler) r.POST("/socket.io/", socketHandler) r.Handle("WS", "/socket.io/", socketHandler) @@ -464,7 +476,7 @@ func loop() { r.POST("/update", updateHandler) // Mount goa handlers - goa := v2.Server(config.GetDataDir().String(), Index) + goa := v2.Server(config.GetDataDir().String(), Index, signaturePubKey) r.Any("/v2/*path", gin.WrapH(goa)) go func() { diff --git a/main_test.go b/main_test.go index d6f23fcec..afb5f9576 100644 --- a/main_test.go +++ b/main_test.go @@ -30,8 +30,10 @@ import ( "github.com/arduino/arduino-create-agent/config" "github.com/arduino/arduino-create-agent/gen/tools" + "github.com/arduino/arduino-create-agent/globals" "github.com/arduino/arduino-create-agent/index" "github.com/arduino/arduino-create-agent/upload" + "github.com/arduino/arduino-create-agent/utilities" v2 "github.com/arduino/arduino-create-agent/v2" "github.com/gin-gonic/gin" "github.com/stretchr/testify/require" @@ -54,7 +56,7 @@ func TestValidSignatureKey(t *testing.T) { func TestUploadHandlerAgainstEvilFileNames(t *testing.T) { r := gin.New() - r.POST("/", uploadHandler) + r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey))) ts := httptest.NewServer(r) uploadEvilFileName := Upload{ @@ -90,7 +92,7 @@ func TestUploadHandlerAgainstEvilFileNames(t *testing.T) { func TestUploadHandlerAgainstBase64WithoutPaddingMustFail(t *testing.T) { r := gin.New() - r.POST("/", uploadHandler) + r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey))) ts := httptest.NewServer(r) defer ts.Close() @@ -119,7 +121,7 @@ func TestInstallToolV2(t *testing.T) { Index := index.Init(indexURL, config.GetDataDir()) r := gin.New() - goa := v2.Server(config.GetDataDir().String(), Index) + goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) r.Any("/v2/*path", gin.WrapH(goa)) ts := httptest.NewServer(r) @@ -213,7 +215,7 @@ func TestInstalledHead(t *testing.T) { Index := index.Init(indexURL, config.GetDataDir()) r := gin.New() - goa := v2.Server(config.GetDataDir().String(), Index) + goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) r.Any("/v2/*path", gin.WrapH(goa)) ts := httptest.NewServer(r) diff --git a/tools/download_test.go b/tools/download_test.go index 7cf2fab0d..bd4812ec0 100644 --- a/tools/download_test.go +++ b/tools/download_test.go @@ -21,7 +21,9 @@ import ( "testing" "time" + "github.com/arduino/arduino-create-agent/globals" "github.com/arduino/arduino-create-agent/index" + "github.com/arduino/arduino-create-agent/utilities" "github.com/arduino/arduino-create-agent/v2/pkgs" "github.com/arduino/go-paths-helper" "github.com/stretchr/testify/require" @@ -128,7 +130,7 @@ func TestDownload(t *testing.T) { IndexFile: *paths.New("testdata", "test_tool_index.json"), LastRefresh: time.Now(), } - testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }) + testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) for _, tc := range testCases { t.Run(tc.name+"-"+tc.version, func(t *testing.T) { @@ -175,7 +177,7 @@ func TestCorruptedInstalled(t *testing.T) { defer fileJSON.Close() _, err = fileJSON.Write([]byte("Hello")) require.NoError(t, err) - testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }) + testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) // Download the tool err = testTools.Download("arduino-test", "avrdude", "6.3.0-arduino17", "keep") require.NoError(t, err) diff --git a/tools/tools.go b/tools/tools.go index 5cecc5089..f371126b5 100644 --- a/tools/tools.go +++ b/tools/tools.go @@ -16,6 +16,7 @@ package tools import ( + "crypto/rsa" "encoding/json" "path/filepath" "strings" @@ -55,14 +56,14 @@ type Tools struct { // The New functions accept the directory to use to host the tools, // an index (used to download the tools), // and a logger to log the operations -func New(directory *paths.Path, index *index.Resource, logger func(msg string)) *Tools { +func New(directory *paths.Path, index *index.Resource, logger func(msg string), signPubKey *rsa.PublicKey) *Tools { t := &Tools{ directory: directory, index: index, logger: logger, installed: map[string]string{}, mutex: sync.RWMutex{}, - tools: pkgs.New(index, directory.String(), "replace"), + tools: pkgs.New(index, directory.String(), "replace", signPubKey), } _ = t.readMap() return t diff --git a/utilities/utilities.go b/utilities/utilities.go index 5979732d4..df71ca2ee 100644 --- a/utilities/utilities.go +++ b/utilities/utilities.go @@ -30,8 +30,6 @@ import ( "os/exec" "path/filepath" "strings" - - "github.com/arduino/arduino-create-agent/globals" ) // SaveFileonTempDir creates a temp directory and saves the file data as the @@ -131,23 +129,38 @@ func SafeJoin(parent, subdir string) (string, error) { return res, nil } -// VerifyInput will verify an input against a signature +// VerifyInput will verify an input against a signature using the public key. // A valid signature is indicated by returning a nil error. -func VerifyInput(input string, signature string) error { +func VerifyInput(input string, signature string, pubKey *rsa.PublicKey) error { sign, _ := hex.DecodeString(signature) - block, _ := pem.Decode([]byte(globals.SignatureKey)) + h := sha256.New() + h.Write([]byte(input)) + d := h.Sum(nil) + return rsa.VerifyPKCS1v15(pubKey, crypto.SHA256, d, sign) +} + +// ParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. +// Returns an error if the key is invalid. +func ParseRsaPublicKey(key string) (*rsa.PublicKey, error) { + block, _ := pem.Decode([]byte(key)) if block == nil { - return errors.New("invalid key") + return nil, errors.New("invalid key") + } + + parsedKey, err := x509.ParsePKIXPublicKey(block.Bytes) + if err != nil { + return nil, err } - key, err := x509.ParsePKIXPublicKey(block.Bytes) + + return parsedKey.(*rsa.PublicKey), nil +} + +func MustParseRsaPublicKey(key string) *rsa.PublicKey { + parsedKey, err := ParseRsaPublicKey(key) if err != nil { - return err + panic(err) } - rsaKey := key.(*rsa.PublicKey) - h := sha256.New() - h.Write([]byte(input)) - d := h.Sum(nil) - return rsa.VerifyPKCS1v15(rsaKey, crypto.SHA256, d, sign) + return parsedKey } // UserPrompt executes an osascript and returns the pressed button diff --git a/v2/http.go b/v2/http.go index 390ec3989..2b47b93a8 100644 --- a/v2/http.go +++ b/v2/http.go @@ -17,6 +17,7 @@ package v2 import ( "context" + "crypto/rsa" "encoding/json" "net/http" @@ -31,7 +32,7 @@ import ( ) // Server is the actual server -func Server(directory string, index *index.Resource) http.Handler { +func Server(directory string, index *index.Resource, pubKey *rsa.PublicKey) http.Handler { mux := goahttp.NewMuxer() // Instantiate logger @@ -40,7 +41,7 @@ func Server(directory string, index *index.Resource) http.Handler { logAdapter := LogAdapter{Logger: logger} // Mount tools - toolsSvc := pkgs.New(index, directory, "replace") + toolsSvc := pkgs.New(index, directory, "replace", pubKey) toolsEndpoints := toolssvc.NewEndpoints(toolsSvc) toolsServer := toolssvr.New(toolsEndpoints, mux, CustomRequestDecoder, goahttp.ResponseEncoder, errorHandler(logger), nil) toolssvr.Mount(mux, toolsServer) diff --git a/v2/pkgs/tools.go b/v2/pkgs/tools.go index f09dc3f0a..877e8aa09 100644 --- a/v2/pkgs/tools.go +++ b/v2/pkgs/tools.go @@ -18,6 +18,7 @@ package pkgs import ( "bytes" "context" + "crypto/rsa" "crypto/sha256" "encoding/hex" "encoding/json" @@ -63,18 +64,21 @@ type Tools struct { behaviour string installed map[string]string mutex sync.RWMutex + + verifySignaturePubKey *rsa.PublicKey // public key used to verify the signature when a tools is installed } // New will return a Tool object, allowing the caller to execute operations on it. // The New function will accept an index as parameter (used to download the indexes) // and a folder used to download the indexes -func New(index *index.Resource, folder, behaviour string) *Tools { +func New(index *index.Resource, folder, behaviour string, verifySignaturePubKey *rsa.PublicKey) *Tools { t := &Tools{ - index: index, - folder: folder, - behaviour: behaviour, - installed: map[string]string{}, - mutex: sync.RWMutex{}, + index: index, + folder: folder, + behaviour: behaviour, + installed: map[string]string{}, + mutex: sync.RWMutex{}, + verifySignaturePubKey: verifySignaturePubKey, } t.readInstalled() return t @@ -166,7 +170,7 @@ func (t *Tools) Install(ctx context.Context, payload *tools.ToolPayload) (*tools //if URL is defined and is signed we verify the signature and override the name, payload, version parameters if payload.URL != nil && payload.Signature != nil && payload.Checksum != nil { - err := utilities.VerifyInput(*payload.URL, *payload.Signature) + err := utilities.VerifyInput(*payload.URL, *payload.Signature, t.verifySignaturePubKey) // TODO pass the public key if err != nil { return nil, err } diff --git a/v2/pkgs/tools_test.go b/v2/pkgs/tools_test.go index edd575fc8..ff0376ac4 100644 --- a/v2/pkgs/tools_test.go +++ b/v2/pkgs/tools_test.go @@ -25,7 +25,9 @@ import ( "github.com/arduino/arduino-create-agent/config" "github.com/arduino/arduino-create-agent/gen/tools" + "github.com/arduino/arduino-create-agent/globals" "github.com/arduino/arduino-create-agent/index" + "github.com/arduino/arduino-create-agent/utilities" "github.com/arduino/arduino-create-agent/v2/pkgs" "github.com/arduino/go-paths-helper" "github.com/stretchr/testify/require" @@ -45,7 +47,7 @@ func TestTools(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace") + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) ctx := context.Background() @@ -126,7 +128,7 @@ func TestEvilFilename(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace") + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) ctx := context.Background() @@ -195,7 +197,7 @@ func TestInstalledHead(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace") + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) ctx := context.Background() @@ -216,7 +218,7 @@ func TestInstall(t *testing.T) { LastRefresh: time.Now(), } - tool := pkgs.New(testIndex, tmp, "replace") + tool := pkgs.New(testIndex, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) ctx := context.Background() From dec684b0f712ca923f9ede3f5bc188a85d15ef89 Mon Sep 17 00:00:00 2001 From: Davide N Date: Tue, 25 Mar 2025 16:06:34 +0100 Subject: [PATCH 02/14] Add debug logging for signature key parsing and handle newline escape sequences --- main.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index c6ef81e64..fe50aeaad 100755 --- a/main.go +++ b/main.go @@ -285,9 +285,12 @@ func loop() { if len(*signatureKey) == 0 { log.Panicf("signature public key cannot be empty") } - signaturePubKey, err := utilities.ParseRsaPublicKey(*signatureKey) + fmt.Println("Signature key:", *signatureKey) + + // when a public key is read fro the .ini file, the '\n' are escape with an additional '\', we need to replace them with '\n' + signaturePubKey, err := utilities.ParseRsaPublicKey(strings.ReplaceAll(*signatureKey, "\\n", "\n")) if err != nil { - log.Panicf("cannot parse signature key: %s", err) + log.Panicf("cannot parse signature key '%s'. %s", *signatureKey, err) } // Instantiate Index and Tools @@ -539,6 +542,8 @@ func parseIni(filename string) (args []string, err error) { return nil, err } + fmt.Println("Sections:", cfg.Sections()) + for _, section := range cfg.Sections() { for key, val := range section.KeysHash() { // Ignore launchself From 5681091401c1713261c46007c7e03f2836649983 Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 10:11:38 +0100 Subject: [PATCH 03/14] Add MustParseRsaPublicKey function for parsing PEM formatted public keys --- utilities/utilities.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utilities/utilities.go b/utilities/utilities.go index df71ca2ee..561f56c05 100644 --- a/utilities/utilities.go +++ b/utilities/utilities.go @@ -155,6 +155,8 @@ func ParseRsaPublicKey(key string) (*rsa.PublicKey, error) { return parsedKey.(*rsa.PublicKey), nil } +// MustParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. +// Panics if the key is invalid. func MustParseRsaPublicKey(key string) *rsa.PublicKey { parsedKey, err := ParseRsaPublicKey(key) if err != nil { From 497a640e0f8858effaccec125beeaebe732ec64c Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 10:16:11 +0100 Subject: [PATCH 04/14] Remove debug print statement for signature key and fix comment typo in key parsing logic --- main.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main.go b/main.go index fe50aeaad..7081a1777 100755 --- a/main.go +++ b/main.go @@ -285,9 +285,7 @@ func loop() { if len(*signatureKey) == 0 { log.Panicf("signature public key cannot be empty") } - fmt.Println("Signature key:", *signatureKey) - - // when a public key is read fro the .ini file, the '\n' are escape with an additional '\', we need to replace them with '\n' + // when a public key is read from the .ini file, the '\n' are escape with an additional '\', we need to replace them with '\n' signaturePubKey, err := utilities.ParseRsaPublicKey(strings.ReplaceAll(*signatureKey, "\\n", "\n")) if err != nil { log.Panicf("cannot parse signature key '%s'. %s", *signatureKey, err) From 07117501a11c5837711f8666d1e839c53adf6c5a Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 10:25:22 +0100 Subject: [PATCH 05/14] Add error logging for command verification and update comment for public key usage --- conn.go | 1 + v2/pkgs/tools.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index a9d8f64dd..8c71c54c4 100644 --- a/conn.go +++ b/conn.go @@ -115,6 +115,7 @@ func uploadHandler(pubKey *rsa.PublicKey) func(*gin.Context) { err := utilities.VerifyInput(data.Commandline, data.Signature, pubKey) if err != nil { + log.WithField("err", err).Error("Error verifying the command") c.String(http.StatusBadRequest, "signature is invalid") return } diff --git a/v2/pkgs/tools.go b/v2/pkgs/tools.go index 877e8aa09..1635417d9 100644 --- a/v2/pkgs/tools.go +++ b/v2/pkgs/tools.go @@ -65,7 +65,7 @@ type Tools struct { installed map[string]string mutex sync.RWMutex - verifySignaturePubKey *rsa.PublicKey // public key used to verify the signature when a tools is installed + verifySignaturePubKey *rsa.PublicKey // public key used to verify the signature of a command sent to the boards } // New will return a Tool object, allowing the caller to execute operations on it. @@ -170,7 +170,7 @@ func (t *Tools) Install(ctx context.Context, payload *tools.ToolPayload) (*tools //if URL is defined and is signed we verify the signature and override the name, payload, version parameters if payload.URL != nil && payload.Signature != nil && payload.Checksum != nil { - err := utilities.VerifyInput(*payload.URL, *payload.Signature, t.verifySignaturePubKey) // TODO pass the public key + err := utilities.VerifyInput(*payload.URL, *payload.Signature, t.verifySignaturePubKey) if err != nil { return nil, err } From be54150c089cd1a067793665158a7d31a34cbf9b Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 12:00:02 +0100 Subject: [PATCH 06/14] trigger From bfcd135688ea04af295b85f272810332a7e2232f Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 12:25:10 +0100 Subject: [PATCH 07/14] fix(release.yml) install go step for gon --- .github/workflows/release.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3b2a210d9..6275922c3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -259,6 +259,11 @@ jobs: environment: production steps: + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: Download artifact uses: actions/download-artifact@v4 with: From 8a60882ebb4cfe5e27919a424bf9dc4f9a5b6f26 Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 14:45:14 +0100 Subject: [PATCH 08/14] refactor(main.go) remove unnecessary print statement in parseIni function --- main.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/main.go b/main.go index 7081a1777..b746a5758 100755 --- a/main.go +++ b/main.go @@ -22,7 +22,6 @@ import ( _ "embed" "encoding/json" "flag" - "fmt" "html/template" "io" "os" @@ -540,8 +539,6 @@ func parseIni(filename string) (args []string, err error) { return nil, err } - fmt.Println("Sections:", cfg.Sections()) - for _, section := range cfg.Sections() { for key, val := range section.KeysHash() { // Ignore launchself From 9601bc0e89b4e5243059f9c2220a744122cf8827 Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 16:36:55 +0100 Subject: [PATCH 09/14] implement suggestions --- globals/globals.go | 11 +++++++++-- main.go | 3 +-- main_test.go | 8 ++++---- tools/download_test.go | 4 ++-- utilities/utilities.go | 6 +++--- v2/pkgs/tools_test.go | 8 ++++---- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/globals/globals.go b/globals/globals.go index 8da580fd5..ac4c14666 100644 --- a/globals/globals.go +++ b/globals/globals.go @@ -15,8 +15,15 @@ package globals -// DefaultIndexURL is the default index url var ( // ArduinoSignaturePubKey is the public key used to verify commands and url sent by the builder - ArduinoSignaturePubKey = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvc0yZr1yUSen7qmE3cxF\nIE12rCksDnqR+Hp7o0nGi9123eCSFcJ7CkIRC8F+8JMhgI3zNqn4cUEn47I3RKD1\nZChPUCMiJCvbLbloxfdJrUi7gcSgUXrlKQStOKF5Iz7xv1M4XOP3JtjXLGo3EnJ1\npFgdWTOyoSrA8/w1rck4c/ISXZSinVAggPxmLwVEAAln6Itj6giIZHKvA2fL2o8z\nCeK057Lu8X6u2CG8tRWSQzVoKIQw/PKK6CNXCAy8vo4EkXudRutnEYHEJlPkVgPn\n2qP06GI+I+9zKE37iqj0k1/wFaCVXHXIvn06YrmjQw6I0dDj/60Wvi500FuRVpn9\ntwIDAQAB\n-----END PUBLIC KEY-----" + ArduinoSignaturePubKey = `-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvc0yZr1yUSen7qmE3cxF +IE12rCksDnqR+Hp7o0nGi9123eCSFcJ7CkIRC8F+8JMhgI3zNqn4cUEn47I3RKD1 +ZChPUCMiJCvbLbloxfdJrUi7gcSgUXrlKQStOKF5Iz7xv1M4XOP3JtjXLGo3EnJ1 +pFgdWTOyoSrA8/w1rck4c/ISXZSinVAggPxmLwVEAAln6Itj6giIZHKvA2fL2o8z +CeK057Lu8X6u2CG8tRWSQzVoKIQw/PKK6CNXCAy8vo4EkXudRutnEYHEJlPkVgPn +2qP06GI+I+9zKE37iqj0k1/wFaCVXHXIvn06YrmjQw6I0dDj/60Wvi500FuRVpn9 +twIDAQAB +-----END PUBLIC KEY-----` ) diff --git a/main.go b/main.go index b746a5758..d00b4f8ec 100755 --- a/main.go +++ b/main.go @@ -284,8 +284,7 @@ func loop() { if len(*signatureKey) == 0 { log.Panicf("signature public key cannot be empty") } - // when a public key is read from the .ini file, the '\n' are escape with an additional '\', we need to replace them with '\n' - signaturePubKey, err := utilities.ParseRsaPublicKey(strings.ReplaceAll(*signatureKey, "\\n", "\n")) + signaturePubKey, err := utilities.ParseRsaPublicKey([]byte(*signatureKey)) if err != nil { log.Panicf("cannot parse signature key '%s'. %s", *signatureKey, err) } diff --git a/main_test.go b/main_test.go index afb5f9576..1387fd221 100644 --- a/main_test.go +++ b/main_test.go @@ -56,7 +56,7 @@ func TestValidSignatureKey(t *testing.T) { func TestUploadHandlerAgainstEvilFileNames(t *testing.T) { r := gin.New() - r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey))) + r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey)))) ts := httptest.NewServer(r) uploadEvilFileName := Upload{ @@ -92,7 +92,7 @@ func TestUploadHandlerAgainstEvilFileNames(t *testing.T) { func TestUploadHandlerAgainstBase64WithoutPaddingMustFail(t *testing.T) { r := gin.New() - r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey))) + r.POST("/", uploadHandler(utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey)))) ts := httptest.NewServer(r) defer ts.Close() @@ -121,7 +121,7 @@ func TestInstallToolV2(t *testing.T) { Index := index.Init(indexURL, config.GetDataDir()) r := gin.New() - goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) r.Any("/v2/*path", gin.WrapH(goa)) ts := httptest.NewServer(r) @@ -215,7 +215,7 @@ func TestInstalledHead(t *testing.T) { Index := index.Init(indexURL, config.GetDataDir()) r := gin.New() - goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + goa := v2.Server(config.GetDataDir().String(), Index, utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) r.Any("/v2/*path", gin.WrapH(goa)) ts := httptest.NewServer(r) diff --git a/tools/download_test.go b/tools/download_test.go index bd4812ec0..96a105fd7 100644 --- a/tools/download_test.go +++ b/tools/download_test.go @@ -130,7 +130,7 @@ func TestDownload(t *testing.T) { IndexFile: *paths.New("testdata", "test_tool_index.json"), LastRefresh: time.Now(), } - testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) for _, tc := range testCases { t.Run(tc.name+"-"+tc.version, func(t *testing.T) { @@ -177,7 +177,7 @@ func TestCorruptedInstalled(t *testing.T) { defer fileJSON.Close() _, err = fileJSON.Write([]byte("Hello")) require.NoError(t, err) - testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + testTools := New(tempDirPath, &testIndex, func(msg string) { t.Log(msg) }, utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) // Download the tool err = testTools.Download("arduino-test", "avrdude", "6.3.0-arduino17", "keep") require.NoError(t, err) diff --git a/utilities/utilities.go b/utilities/utilities.go index 561f56c05..55927932d 100644 --- a/utilities/utilities.go +++ b/utilities/utilities.go @@ -141,8 +141,8 @@ func VerifyInput(input string, signature string, pubKey *rsa.PublicKey) error { // ParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. // Returns an error if the key is invalid. -func ParseRsaPublicKey(key string) (*rsa.PublicKey, error) { - block, _ := pem.Decode([]byte(key)) +func ParseRsaPublicKey(key []byte) (*rsa.PublicKey, error) { + block, _ := pem.Decode(key) if block == nil { return nil, errors.New("invalid key") } @@ -157,7 +157,7 @@ func ParseRsaPublicKey(key string) (*rsa.PublicKey, error) { // MustParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. // Panics if the key is invalid. -func MustParseRsaPublicKey(key string) *rsa.PublicKey { +func MustParseRsaPublicKey(key []byte) *rsa.PublicKey { parsedKey, err := ParseRsaPublicKey(key) if err != nil { panic(err) diff --git a/v2/pkgs/tools_test.go b/v2/pkgs/tools_test.go index ff0376ac4..7bf0ff0e3 100644 --- a/v2/pkgs/tools_test.go +++ b/v2/pkgs/tools_test.go @@ -47,7 +47,7 @@ func TestTools(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) ctx := context.Background() @@ -128,7 +128,7 @@ func TestEvilFilename(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) ctx := context.Background() @@ -197,7 +197,7 @@ func TestInstalledHead(t *testing.T) { // Instantiate Index Index := index.Init(indexURL, config.GetDataDir()) - service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + service := pkgs.New(Index, tmp, "replace", utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) ctx := context.Background() @@ -218,7 +218,7 @@ func TestInstall(t *testing.T) { LastRefresh: time.Now(), } - tool := pkgs.New(testIndex, tmp, "replace", utilities.MustParseRsaPublicKey(globals.ArduinoSignaturePubKey)) + tool := pkgs.New(testIndex, tmp, "replace", utilities.MustParseRsaPublicKey([]byte(globals.ArduinoSignaturePubKey))) ctx := context.Background() From 0f7a4fbdc589a3ce443f4847571c1b2f1f996274 Mon Sep 17 00:00:00 2001 From: Davide Date: Wed, 26 Mar 2025 17:34:18 +0100 Subject: [PATCH 10/14] Update v2/pkgs/tools.go Co-authored-by: Luca Rinaldi --- v2/pkgs/tools.go | 1 - 1 file changed, 1 deletion(-) diff --git a/v2/pkgs/tools.go b/v2/pkgs/tools.go index 1635417d9..4c051c4a0 100644 --- a/v2/pkgs/tools.go +++ b/v2/pkgs/tools.go @@ -64,7 +64,6 @@ type Tools struct { behaviour string installed map[string]string mutex sync.RWMutex - verifySignaturePubKey *rsa.PublicKey // public key used to verify the signature of a command sent to the boards } From d9348ca12f4562c7b4898e35e523482feb271328 Mon Sep 17 00:00:00 2001 From: Davide Date: Wed, 26 Mar 2025 17:34:30 +0100 Subject: [PATCH 11/14] Update utilities/utilities.go Co-authored-by: Luca Rinaldi --- utilities/utilities.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/utilities/utilities.go b/utilities/utilities.go index 55927932d..c844afc73 100644 --- a/utilities/utilities.go +++ b/utilities/utilities.go @@ -152,7 +152,11 @@ func ParseRsaPublicKey(key []byte) (*rsa.PublicKey, error) { return nil, err } - return parsedKey.(*rsa.PublicKey), nil + publicKey, ok := value.(*rsa.PublicKey) + if !ok { + return nil,fmt.Errorf("not an rsa key") + } +return publicKey, nil } // MustParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. From 27e570e7247d2f58a8b8d44bd8ba972b10f6103f Mon Sep 17 00:00:00 2001 From: Davide Date: Wed, 26 Mar 2025 17:35:25 +0100 Subject: [PATCH 12/14] Update main.go Co-authored-by: Luca Rinaldi --- main.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index d00b4f8ec..41f824b1b 100755 --- a/main.go +++ b/main.go @@ -278,11 +278,8 @@ func loop() { } } - if signatureKey == nil { - log.Panicf("signature public key cannot be nil") - } - if len(*signatureKey) == 0 { - log.Panicf("signature public key cannot be empty") + if signatureKey == nil || len(*signatureKey) == 0 { + log.Panicf("signature public key should be set") } signaturePubKey, err := utilities.ParseRsaPublicKey([]byte(*signatureKey)) if err != nil { From 27b7ce77a82f1a9c52d516679d3f0e7fb1f21513 Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 17:37:26 +0100 Subject: [PATCH 13/14] refactor(utilities): improve formatting in ParseRsaPublicKey function --- utilities/utilities.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/utilities/utilities.go b/utilities/utilities.go index c844afc73..662672da7 100644 --- a/utilities/utilities.go +++ b/utilities/utilities.go @@ -152,11 +152,11 @@ func ParseRsaPublicKey(key []byte) (*rsa.PublicKey, error) { return nil, err } - publicKey, ok := value.(*rsa.PublicKey) - if !ok { - return nil,fmt.Errorf("not an rsa key") - } -return publicKey, nil + publicKey, ok := parsedKey.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("not an rsa key") + } + return publicKey, nil } // MustParseRsaPublicKey parses a public key in PEM format and returns the rsa.PublicKey object. From 2237ff0345c8af6974cfaf86b4a81d65097c878c Mon Sep 17 00:00:00 2001 From: Davide N Date: Wed, 26 Mar 2025 17:50:10 +0100 Subject: [PATCH 14/14] style(tools): align field declarations for improved readability --- v2/pkgs/tools.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/v2/pkgs/tools.go b/v2/pkgs/tools.go index 4c051c4a0..7f34e3d08 100644 --- a/v2/pkgs/tools.go +++ b/v2/pkgs/tools.go @@ -59,11 +59,11 @@ var ( // // It requires an Index Resource to search for tools type Tools struct { - index *index.Resource - folder string - behaviour string - installed map[string]string - mutex sync.RWMutex + index *index.Resource + folder string + behaviour string + installed map[string]string + mutex sync.RWMutex verifySignaturePubKey *rsa.PublicKey // public key used to verify the signature of a command sent to the boards }