changes from code review

Signed-off-by: Matthew R. Kasun <mkasun@nusak.ca>

controllers/server.go

@@ -14,7 +14,6 @@ import (
 	"github.com/gravitl/netmaker/logic"
 	"github.com/gravitl/netmaker/models"
 	"github.com/gravitl/netmaker/netclient/config"
-	"github.com/gravitl/netmaker/netclient/ncutils"
 	"github.com/gravitl/netmaker/servercfg"
 	"github.com/gravitl/netmaker/tls"
 )
@@ -173,9 +172,6 @@ func register(w http.ResponseWriter, r *http.Request) {
 		returnErrorResponse(w, r, errorResponse)
 		return
 	}
-
-	tls.SaveCert("/tmp/sent/", "root.pem", ca)
-	tls.SaveCert("/tmp/sent/", "client.pem", cert)
 	//x509.Certificate.PublicKey is an interface therefore json encoding/decoding result in a string value rather than a []byte
 	//include the actual public key so the certificate can be properly reassembled on the other end.
 	response := config.RegisterResponse{
@@ -212,28 +208,3 @@ func genCerts(clientKey *ed25519.PrivateKey, name *pkix.Name) (*x509.Certificate
 	}
 	return cert, ca, nil
 }
-
-// genOpenSSLCerts generates a client certificate using calls to openssl and returns the certificate and root CA
-func genOpenSSLCerts(key *ed25519.PrivateKey, name *pkix.Name) (*x509.Certificate, *x509.Certificate, error) {
-	if err := tls.SaveKey("/tmp/", "client.key", *key); err != nil {
-		return nil, nil, fmt.Errorf("failed to store client key %w", err)
-	}
-	cmd2 := fmt.Sprintf("openssl req -new -out /tmp/client.csr -key /tmp/client.key -subj /CN=%s", name.CommonName)
-	cmd3 := "openssl x509 -req -in /tmp/client.csr -days 365 -CA /etc/netmaker/root.pem -CAkey /etc/netmaker/root.key -CAcreateserial -out /tmp/client.pem"
-
-	if _, err := ncutils.RunCmd(cmd2, true); err != nil {
-		return nil, nil, fmt.Errorf("client csr error %w", err)
-	}
-	if _, err := ncutils.RunCmd(cmd3, true); err != nil {
-		return nil, nil, fmt.Errorf("client cert error %w", err)
-	}
-	cert, err := tls.ReadCert("/tmp/client.pem")
-	if err != nil {
-		return nil, nil, fmt.Errorf("read client cert error %w", err)
-	}
-	ca, err := tls.ReadCert("/etc/netmaker/root.pem")
-	if err != nil {
-		return nil, nil, fmt.Errorf("read ca cert error %w", err)
-	}
-	return cert, ca, nil
-}

functions/local.go

@@ -7,6 +7,9 @@ import (
 	"github.com/gravitl/netmaker/logic"
 )
 
+// LINUX_APP_DATA_PATH - linux path
+const LINUX_APP_DATA_PATH = "/etc/netmaker"
+
 // FileExists - checks if file exists
 func FileExists(f string) bool {
 	info, err := os.Stat(f)
@@ -49,3 +52,8 @@ func SetDNSDir() error {
 	}
 	return nil
 }
+
+// GetNetmakerPath - gets netmaker path locally
+func GetNetmakerPath() string {
+	return LINUX_APP_DATA_PATH
+}

main.go

@@ -2,6 +2,9 @@ package main
 
 import (
 	"context"
+	"crypto/ed25519"
+	"crypto/rand"
+	"errors"
 	"flag"
 	"fmt"
 	"net"
@@ -11,6 +14,7 @@ import (
 	"strconv"
 	"sync"
 	"syscall"
+	"time"
 
 	"github.com/gravitl/netmaker/auth"
 	"github.com/gravitl/netmaker/config"
@@ -25,6 +29,7 @@ import (
 	"github.com/gravitl/netmaker/netclient/ncutils"
 	"github.com/gravitl/netmaker/servercfg"
 	"github.com/gravitl/netmaker/serverctl"
+	"github.com/gravitl/netmaker/tls"
 	"google.golang.org/grpc"
 )
 
@@ -117,6 +122,7 @@ func initialize() { // Client Mode Prereq Check
 			logger.FatalLog(err.Error())
 		}
 	}
+	genCerts()
 }
 
 func startControllers() {
@@ -235,3 +241,63 @@ func setGarbageCollection() {
 		debug.SetGCPercent(ncutils.DEFAULT_GC_PERCENT)
 	}
 }
+
+func genCerts() error {
+	private, err := tls.ReadKey(functions.GetNetmakerPath())
+	if errors.Is(err, os.ErrNotExist) {
+		_, *private, err = ed25519.GenerateKey(rand.Reader)
+		if err != nil {
+			return err
+		}
+		if err := tls.SaveKey(functions.GetNetmakerPath(), "/root.key", *private); err != nil {
+			return err
+		}
+	} else if err != nil {
+		return err
+	}
+	ca, err := tls.ReadCert(functions.GetNetmakerPath() + "/root.pem")
+	//if cert doesn't exist or will expire within 10 days --- but can't do this as clients won't be able to connect
+	//if errors.Is(err, os.ErrNotExist) || cert.NotAfter.Before(time.Now().Add(time.Hour*24*10)) {
+	if errors.Is(err, os.ErrNotExist) {
+		caName := tls.NewName("CA Root", "US", "Gravitl")
+		csr, err := tls.NewCSR(*private, caName)
+		if err != nil {
+			return err
+		}
+		rootCA, err := tls.SelfSignedCA(*private, csr, tls.CERTIFICATE_VALIDITY)
+		if err != nil {
+			return err
+		}
+		if err := tls.SaveCert(functions.GetNetmakerPath(), "/root.pem", rootCA); err != nil {
+			return err
+		}
+	} else if err != nil {
+		return err
+	}
+	cert, err := tls.ReadCert(functions.GetNetmakerPath() + "/server.pem")
+	if errors.Is(err, os.ErrNotExist) || cert.NotAfter.Before(time.Now().Add(time.Hour*24*10)) {
+		//gen new key
+		_, key, err := ed25519.GenerateKey(rand.Reader)
+		if err != nil {
+			return err
+		}
+		serverName := tls.NewCName(servercfg.GetServer())
+		csr, err := tls.NewCSR(key, serverName)
+		if err != nil {
+			return err
+		}
+		cert, err := tls.NewEndEntityCert(*private, csr, ca, tls.CERTIFICATE_VALIDITY)
+		if err != nil {
+			return err
+		}
+		if err := tls.SaveKey(functions.GetNetmakerPath(), "/server.key", key); err != nil {
+			return err
+		}
+		if err := tls.SaveCert(functions.GetNetmakerPath(), "/server.pem", cert); err != nil {
+			return err
+		}
+	} else if err != nil {
+		return err
+	}
+	return nil
+}

netclient/cli_options/cmds.go

@@ -30,7 +30,7 @@ func GetCommands(cliFlags []cli.Flag) []*cli.Command {
 					err = errors.New("no server address provided")
 					return err
 				}
-				err = command.Join(&cfg, pvtKey)
+				err = command.Register(&cfg, pvtKey)
 				return err
 			},
 		},
@@ -105,18 +105,6 @@ func GetCommands(cliFlags []cli.Flag) []*cli.Command {
 				return err
 			},
 		},
-		{
-			Name:  "register",
-			Usage: "register with netmaker",
-			Flags: cliFlags,
-			Action: func(c *cli.Context) error {
-				cfg, _, err := config.GetCLIConfig(c)
-				if err != nil {
-					return err
-				}
-				return command.Register(&cfg)
-			},
-		},
 	}
 }
 

netclient/command/commands.go

@@ -155,6 +155,6 @@ func Daemon() error {
 	return err
 }
 
-func Register(cfg *config.ClientConfig) error {
+func Register(cfg *config.ClientConfig, key string) error {
-	return functions.Register(cfg)
+	return functions.Register(cfg, key)
 }

netclient/functions/common.go

@@ -23,6 +23,9 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
+// LINUX_APP_DATA_PATH - linux path
+const LINUX_APP_DATA_PATH = "/etc/netmaker"
+
 // ListPorts - lists ports of WireGuard devices
 func ListPorts() error {
 	wgclient, err := wgctrl.New()
@@ -321,3 +324,8 @@ func WipeLocal(network string) error {
 	}
 	return err
 }
+
+// GetNetmakerPath - gets netmaker path locally
+func GetNetmakerPath() string {
+	return LINUX_APP_DATA_PATH
+}

netclient/functions/daemon.go

@@ -186,7 +186,6 @@ func setupMQTTSub(server string) mqtt.Client {
 	opts := mqtt.NewClientOptions()
 	opts.AddBroker("ssl://" + server + ":8883") // TODO get the appropriate port of the comms mq server
 	opts.TLSConfig = NewTLSConfig(nil, server)
-	opts.ClientID = ncutils.MakeRandomString(23) // helps avoid id duplication on broker
 	opts.SetDefaultPublishHandler(All)
 	opts.SetAutoReconnect(true)
 	opts.SetConnectRetry(true)
@@ -328,7 +327,6 @@ func setupMQTT(cfg *config.ClientConfig, publish bool) mqtt.Client {
 	server := cfg.Server.Server
 	opts.AddBroker("ssl://" + server + ":8883") // TODO get the appropriate port of the comms mq server
 	opts.TLSConfig = NewTLSConfig(cfg, "")
-	opts.ClientID = ncutils.MakeRandomString(23) // helps avoid id duplication on broker
 	opts.SetDefaultPublishHandler(All)
 	opts.SetAutoReconnect(true)
 	opts.SetConnectRetry(true)

netclient/functions/mqpublish.go

@@ -3,7 +3,9 @@ package functions
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"os"
 	"sync"
 	"time"
 
@@ -11,6 +13,7 @@ import (
 	"github.com/gravitl/netmaker/netclient/auth"
 	"github.com/gravitl/netmaker/netclient/config"
 	"github.com/gravitl/netmaker/netclient/ncutils"
+	"github.com/gravitl/netmaker/tls"
 )
 
 // Checkin  -- go routine that checks for public or local ip changes, publishes changes
@@ -75,6 +78,7 @@ func Checkin(ctx context.Context, wg *sync.WaitGroup, currentComms map[string]st
 				} else {
 					Hello(&nodeCfg)
 				}
+				checkCertExpiry(&nodeCfg)
 			}
 		}
 	}
@@ -135,3 +139,19 @@ func publish(nodeCfg *config.ClientConfig, dest string, msg []byte, qos byte) er
 	}
 	return nil
 }
+
+func checkCertExpiry(cfg *config.ClientConfig) error {
+	cert, err := tls.ReadCert(ncutils.GetNetclientServerPath(cfg.Server.Server) + "/client.pem")
+	//if cert doesn't exist or will expire within 10 days
+	if errors.Is(err, os.ErrNotExist) || cert.NotAfter.Before(time.Now().Add(time.Hour*24*10)) {
+		key, err := tls.ReadKey(ncutils.GetNetclientPath() + "/client.key")
+		if err != nil {
+			return err
+		}
+		return RegisterWithServer(key, cfg)
+	}
+	if err != nil {
+		return err
+	}
+	return nil
+}

netclient/functions/register.go

@@ -17,7 +17,7 @@ import (
 )
 
 // Register - the function responsible for registering with the server and acquiring certs
-func Register(cfg *config.ClientConfig) error {
+func Register(cfg *config.ClientConfig, key string) error {
 	if cfg.Server.Server == "" {
 		return errors.New("no server provided")
 	}
@@ -35,6 +35,20 @@ func Register(cfg *config.ClientConfig) error {
 			return err
 		}
 	}
+	//check if cert exists
+	_, err = tls.ReadCert(ncutils.GetNetclientServerPath(cfg.Server.Server) + "/client.pem")
+	if err != os.ErrNotExist {
+		if err := RegisterWithServer(private, cfg); err != nil {
+			return err
+		}
+	}
+	if err != nil {
+		return err
+	}
+	return JoinNetwork(cfg, key, false)
+}
+
+func RegisterWithServer(private *ed25519.PrivateKey, cfg *config.ClientConfig) error {
 	data := config.RegisterRequest{
 		Key:        *private,
 		CommonName: tls.NewCName(os.Getenv("HOSTNAME")),
@@ -75,5 +89,5 @@ func Register(cfg *config.ClientConfig) error {
 	}
 	logger.Log(0, "certificates/key saved ")
 	//join the network defined in the token
-	return JoinNetwork(cfg, "", false)
+	return nil
 }

tls/tls.go

@@ -170,7 +170,7 @@ func NewEndEntityCert(key ed25519.PrivateKey, req *x509.CertificateRequest, pare
 
 // SaveRequest saves a certificate request to the specified path
 func SaveRequest(path, name string, csr *x509.CertificateRequest) error {
-	if err := os.MkdirAll(path, 0644); err != nil {
+	if err := os.MkdirAll(path, 0600); err != nil {
 		return err
 	}
 	requestOut, err := os.Create(path + name)
@@ -190,7 +190,7 @@ func SaveRequest(path, name string, csr *x509.CertificateRequest) error {
 // SaveCert save a certificate to the specified path
 func SaveCert(path, name string, cert *x509.Certificate) error {
 	//certbytes, err := x509.ParseCertificate(cert)
-	if err := os.MkdirAll(path, 0644); err != nil {
+	if err := os.MkdirAll(path, 0600); err != nil {
 		return fmt.Errorf("failed to create dir %s %w", path, err)
 	}
 	certOut, err := os.Create(path + name)
@@ -210,7 +210,7 @@ func SaveCert(path, name string, cert *x509.Certificate) error {
 // SaveKey save a private key (ed25519) to the specified path
 func SaveKey(path, name string, key ed25519.PrivateKey) error {
 	//func SaveKey(name string, key *ecdsa.PrivateKey) error {
-	if err := os.MkdirAll(path, 0644); err != nil {
+	if err := os.MkdirAll(path, 0600); err != nil {
 		return fmt.Errorf("failed to create dir %s %w", path, err)
 	}
 	keyOut, err := os.Create(path + name)