Improve validation of bytes when generating address from byte slice

.gitignore

@@ -1 +1,4 @@
 log
+i2p-backup
+/tmp
+/*.txt

I2PAddr.go

@@ -73,21 +73,32 @@ func validateAddressFormat(addr string) error {
 }
 
 func validateBase64Encoding(addr string) error {
-	buf := make([]byte, i2pB64enc.DecodedLen(len(addr)))
-	if _, err := i2pB64enc.Decode(buf, []byte(addr)); err != nil {
+	// Use DecodeString which handles buffer allocation internally
+	// and returns the actual decoded bytes, providing better validation
+	decoded, err := i2pB64enc.DecodeString(addr)
+	if err != nil {
 		return fmt.Errorf("invalid base64 encoding: %w", err)
 	}
+
+	// Validate that we got a reasonable amount of decoded data
+	// This prevents edge cases where decoding succeeds but produces empty/minimal output
+	if len(decoded) == 0 {
+		return fmt.Errorf("base64 decoding produced empty result")
+	}
+
 	return nil
 }
 
 // NewI2PAddrFromBytes creates a new I2P address from a byte array.
 func NewI2PAddrFromBytes(addr []byte) (I2PAddr, error) {
-	if len(addr) > MaxAddressLength || len(addr) < MinAddressLength {
-		return I2PAddr(""), fmt.Errorf("invalid address length: got %d, want between %d and %d",
-			len(addr), MinAddressLength, MaxAddressLength)
+	// Calculate the expected encoded length to validate against string constraints
+	encodedLen := i2pB64enc.EncodedLen(len(addr))
+	if encodedLen > MaxAddressLength || encodedLen < MinAddressLength {
+		return I2PAddr(""), fmt.Errorf("invalid address length: encoded length %d, want between %d and %d",
+			encodedLen, MinAddressLength, MaxAddressLength)
 	}
 
-	encoded := make([]byte, i2pB64enc.EncodedLen(len(addr)))
+	encoded := make([]byte, encodedLen)
 	i2pB64enc.Encode(encoded, addr)
 	return I2PAddr(encoded), nil
 }

I2PAddr_test.go

@@ -211,6 +211,18 @@ func Test_KeyGenerationAndHandling(t *testing.T) {
 		if buf.String() != expected {
 			t.Errorf("StoreKeysIncompat wrote incorrect data. Got '%s', want '%s'", buf.String(), expected)
 		}
+		// store the buffer content to a permanent local file in this directory
+		err = ioutil.WriteFile("test_keys.txt", buf.Bytes(), 0644)
+		if err != nil {
+			t.Fatalf("Failed to write buffer content to file: '%v'", err)
+		}
+		content, err := ioutil.ReadFile("test_keys.txt")
+		if err != nil {
+			t.Fatalf("Failed to read test_keys.txt: '%v'", err)
+		}
+		if string(content) != expected {
+			t.Errorf("StoreKeysIncompat wrote incorrect data to file. Got '%s', want '%s'", string(content), expected)
+		}
 	})
 
 	t.Run("StoreKeys", func(t *testing.T) {

I2PKeys.go

@@ -1,13 +1,10 @@
 package i2pkeys
 
 import (
-	"bytes"
 	"crypto"
 	"encoding/base32"
 	"encoding/base64"
-	"errors"
 	"fmt"
-	"io"
 	"os"
 	"strings"
 )
@@ -57,99 +54,6 @@ func fileExists(filename string) (bool, error) {
 	return !info.IsDir(), nil
 }
 
-// LoadKeysIncompat loads keys from a non-standard format
-func LoadKeysIncompat(r io.Reader) (I2PKeys, error) {
-	log.Debug("Loading keys from reader")
-	var buff bytes.Buffer
-	_, err := io.Copy(&buff, r)
-	if err != nil {
-		log.WithError(err).Error("Error copying from reader, did not load keys")
-		return I2PKeys{}, fmt.Errorf("error copying from reader: %w", err)
-	}
-
-	parts := strings.Split(buff.String(), "\n")
-	if len(parts) < 2 {
-		err := errors.New("invalid key format: not enough data")
-		log.WithError(err).Error("Error parsing keys")
-		return I2PKeys{}, err
-	}
-
-	k := I2PKeys{I2PAddr(parts[0]), parts[1]}
-	log.WithField("keys", k).Debug("Loaded keys")
-	return k, nil
-}
-
-// load keys from non-standard format by specifying a text file.
-// If the file does not exist, generate keys, otherwise, fail
-// closed.
-func LoadKeys(r string) (I2PKeys, error) {
-	log.WithField("filename", r).Debug("Loading keys from file")
-	exists, err := fileExists(r)
-	if err != nil {
-		log.WithError(err).Error("Error checking if file exists")
-		return I2PKeys{}, err
-	}
-	if !exists {
-		// File doesn't exist so we'll generate new keys
-		log.WithError(err).Debug("File does not exist, attempting to generate new keys")
-		k, err := NewDestination()
-		if err != nil {
-			log.WithError(err).Error("Error generating new keys")
-			return I2PKeys{}, err
-		}
-		// Save the new keys to the file
-		err = StoreKeys(*k, r)
-		if err != nil {
-			log.WithError(err).Error("Error saving new keys to file")
-			return I2PKeys{}, err
-		}
-		return *k, nil
-	}
-	fi, err := os.Open(r)
-	if err != nil {
-		log.WithError(err).WithField("filename", r).Error("Error opening file")
-		return I2PKeys{}, fmt.Errorf("error opening file: %w", err)
-	}
-	defer fi.Close()
-	log.WithField("filename", r).Debug("File opened successfully")
-	return LoadKeysIncompat(fi)
-}
-
-// store keys in non standard format
-func StoreKeysIncompat(k I2PKeys, w io.Writer) error {
-	log.Debug("Storing keys")
-	_, err := io.WriteString(w, k.Address.Base64()+"\n"+k.Both)
-	if err != nil {
-		log.WithError(err).Error("Error writing keys")
-		return fmt.Errorf("error writing keys: %w", err)
-	}
-	log.WithField("keys", k).Debug("Keys stored successfully")
-	return nil
-}
-
-func StoreKeys(k I2PKeys, r string) error {
-	log.WithField("filename", r).Debug("Storing keys to file")
-	if _, err := os.Stat(r); err != nil {
-		if os.IsNotExist(err) {
-			log.WithField("filename", r).Debug("File does not exist, creating new file")
-			fi, err := os.Create(r)
-			if err != nil {
-				log.WithError(err).Error("Error creating file")
-				return err
-			}
-			defer fi.Close()
-			return StoreKeysIncompat(k, fi)
-		}
-	}
-	fi, err := os.Open(r)
-	if err != nil {
-		log.WithError(err).Error("Error opening file")
-		return err
-	}
-	defer fi.Close()
-	return StoreKeysIncompat(k, fi)
-}
-
 func (k I2PKeys) Network() string {
 	return k.Address.Network()
 }
@@ -164,16 +68,34 @@ func (k I2PKeys) Public() crypto.PublicKey {
 	return k.Address
 }
 
+// Private returns the private key as a byte slice.
 func (k I2PKeys) Private() []byte {
 	log.Debug("Extracting private key")
-	src := strings.Split(k.String(), k.Addr().String())[0]
-	var dest []byte
-	_, err := i2pB64enc.Decode(dest, []byte(src))
+
+	// The private key is everything after the public key in the combined string
+	fullKeys := k.String()
+	publicKey := k.Addr().String()
+
+	// Find where the public key ends in the full string
+	if !strings.HasPrefix(fullKeys, publicKey) {
+		log.Error("Invalid key format: public key not found at start of combined keys")
+		return nil
+	}
+
+	// Extract the private key portion (everything after the public key)
+	privateKeyB64 := fullKeys[len(publicKey):]
+
+	// Pre-allocate destination slice with appropriate capacity
+	dest := make([]byte, i2pB64enc.DecodedLen(len(privateKeyB64)))
+
+	n, err := i2pB64enc.Decode(dest, []byte(privateKeyB64))
 	if err != nil {
 		log.WithError(err).Error("Error decoding private key")
-		panic(err)
+		return nil // Return nil instead of panicking
 	}
-	return dest
+
+	// Return only the portion that was actually decoded
+	return dest[:n]
 }
 
 // Returns the keys (both public and private), in I2Ps base64 format. Use this

LoadKeys.go

@@ -0,0 +1,68 @@
+package i2pkeys
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+)
+
+// LoadKeysIncompat loads keys from a non-standard format
+func LoadKeysIncompat(r io.Reader) (I2PKeys, error) {
+	log.Debug("Loading keys from reader")
+	var buff bytes.Buffer
+	_, err := io.Copy(&buff, r)
+	if err != nil {
+		log.WithError(err).Error("Error copying from reader, did not load keys")
+		return I2PKeys{}, fmt.Errorf("error copying from reader: %w", err)
+	}
+
+	parts := strings.Split(buff.String(), "\n")
+	if len(parts) < 2 {
+		err := errors.New("invalid key format: not enough data")
+		log.WithError(err).Error("Error parsing keys")
+		return I2PKeys{}, err
+	}
+
+	k := I2PKeys{I2PAddr(parts[0]), parts[1]}
+	log.WithField("keys", k).Debug("Loaded keys")
+	return k, nil
+}
+
+// load keys from non-standard format by specifying a text file.
+// If the file does not exist, generate keys, otherwise, fail
+// closed.
+func LoadKeys(r string) (I2PKeys, error) {
+	log.WithField("filename", r).Debug("Loading keys from file")
+	exists, err := fileExists(r)
+	if err != nil {
+		log.WithError(err).Error("Error checking if file exists")
+		return I2PKeys{}, err
+	}
+	if !exists {
+		// File doesn't exist so we'll generate new keys
+		log.WithError(err).Debug("File does not exist, attempting to generate new keys")
+		k, err := NewDestination()
+		if err != nil {
+			log.WithError(err).Error("Error generating new keys")
+			return I2PKeys{}, err
+		}
+		// Save the new keys to the file
+		err = StoreKeys(*k, r)
+		if err != nil {
+			log.WithError(err).Error("Error saving new keys to file")
+			return I2PKeys{}, err
+		}
+		return *k, nil
+	}
+	fi, err := os.Open(r)
+	if err != nil {
+		log.WithError(err).WithField("filename", r).Error("Error opening file")
+		return I2PKeys{}, fmt.Errorf("error opening file: %w", err)
+	}
+	defer fi.Close()
+	log.WithField("filename", r).Debug("File opened successfully")
+	return LoadKeysIncompat(fi)
+}

NewI2PKeys.go

@@ -116,6 +116,16 @@ func (c *samClient) generateKeys(ctx context.Context, conn net.Conn, keyType str
 	if err != nil {
 		return nil, err
 	}
+	log.Println("Generated keys:", pub, priv)
+	if len(pub) == 0 || len(priv) == 0 {
+		return nil, fmt.Errorf("invalid key response: %s", response)
+	}
+	if len(pub) > maxResponseSize || len(priv) > maxResponseSize {
+		return nil, fmt.Errorf("key response too large: %s", response)
+	}
+	if len(pub) < 128 || len(priv) < 128 {
+		return nil, fmt.Errorf("key response too small: %s", response)
+	}
 
 	return &I2PKeys{
 		Address: I2PAddr(pub),

StoreKeys.go

@@ -0,0 +1,45 @@
+package i2pkeys
+
+import (
+	"fmt"
+	"io"
+	"os"
+)
+
+// store keys in non standard format
+func StoreKeysIncompat(k I2PKeys, w io.Writer) error {
+	log.Debug("Storing keys")
+	_, err := io.WriteString(w, k.Address.Base64()+"\n"+k.Both)
+	if err != nil {
+		log.WithError(err).Error("Error writing keys")
+		return fmt.Errorf("error writing keys: %w", err)
+	}
+	log.WithField("keys", k).Debug("Keys stored successfully")
+	return nil
+}
+
+func StoreKeys(k I2PKeys, r string) error {
+	log.WithField("filename", r).Debug("Storing keys to file")
+	if _, err := os.Stat(r); err != nil {
+		if os.IsNotExist(err) {
+			log.WithField("filename", r).Debug("File does not exist, creating new file")
+			fi, err := os.Create(r)
+			if err != nil {
+				log.WithError(err).Error("Error creating file")
+				return err
+			}
+			defer fi.Close()
+			return StoreKeysIncompat(k, fi)
+		}
+		// If stat failed for reasons other than file not existing, return the error
+		return err
+	}
+	// File exists - open in write mode to allow overwriting
+	fi, err := os.OpenFile(r, os.O_WRONLY|os.O_TRUNC, 0644)
+	if err != nil {
+		log.WithError(err).Error("Error opening file")
+		return err
+	}
+	defer fi.Close()
+	return StoreKeysIncompat(k, fi)
+}