Fix key_certificate parsing/construction from data

Makefile

@@ -28,4 +28,4 @@ fmt:
 	find . -name '*.go' -exec gofmt -w -s {} \;
 
 testcommon:
-	$(GO) test -v -failfast ./lib/common/...
+	$(GO) test -failfast ./lib/common/...

lib/common/certificate.go

@@ -70,14 +70,18 @@ func (certificate Certificate) SignatureSize() (size int) {
 func (certificate Certificate) Cert() []byte {
 	var ret []byte
 	ret = append(ret, certificate.CertType.Bytes()...)
+	l, _ := certificate.Length()
+	//if err != nil && err.Error() != "certificate parsing warning: certificate data is shorter than specified by length" {
+	//}
 	data, _ := certificate.Data()
-	if certificate.CertLen.Value() != 0 && len(data) != 0 {
+	if l != 0 && len(data) != 0 {
 		ret = append(ret, certificate.CertLen.Bytes()...)
 		ret = append(ret, data...)
 	} else {
 		ret = append(ret, certificate.CertLen.Bytes()...)
 	}
-	return ret
+	//log.Println("\n\n CERTIFICATE: ", ret, l+CERT_MIN_SIZE, err)
+	return ret //[:l+CERT_MIN_SIZE]
 }
 
 //
@@ -111,7 +115,7 @@ func (certificate Certificate) Length() (length int, err error) {
 			"reason":                    "certificate data is shorter than specified by length",
 		}).Warn("certificate format warning")
 		err = errors.New("certificate parsing warning: certificate data is shorter than specified by length")
-		length = len(certificate.CertBytes)
+		length = certificate.CertLen.Value()
 	}
 	if certificate.CertLen.Value() < len(certificate.CertBytes) {
 		log.WithFields(log.Fields{
@@ -120,14 +124,13 @@ func (certificate Certificate) Length() (length int, err error) {
 			"certificate_actual_length": len(certificate.CertBytes),
 			"reason":                    "certificate contains data beyond length",
 		}).Warn("certificate format warning")
-		err = errors.New("certificate parsing warning: certificate contains data beyond length")
+		err = errors.New("certificate parsing warning: certificate data is longer than specified by length")
 		length = certificate.CertLen.Value()
-		return
 	}
-	length = certificate.CertLen.Value()
 	if err != nil {
 		return
 	}
+	length = certificate.CertLen.Value()
 	return
 }
 
@@ -144,7 +147,7 @@ func (certificate Certificate) Data() (data []byte, err error) {
 		case "certificate parsing warning: certificate data is shorter than specified by length":
 			data = certificate.CertBytes
 			return
-		case "certificate parsing warning: certificate contains data beyond length":
+		case "certificate parsing warning: certificate data is longer than specified by length":
 			data = certificate.CertBytes[:certificate.CertLen.Value()]
 			return
 		}
@@ -159,6 +162,16 @@ func (certificate Certificate) Data() (data []byte, err error) {
 func ReadCertificate(data []byte) (certificate *Certificate, remainder []byte, err error) {
 	certificate = &Certificate{}
 	certificate.CertType, err = NewInteger(data[0:1])
+	if err != nil {
+		log.WithFields(log.Fields{
+			"at":           "(Certificate) ReadCertificate",
+			"certificate":  certificate,
+			"data":         data,
+			"reason":       "error parsing certificate type",
+			"error":        err,
+			"error_reason": err.Error(),
+		}).Warn("certificate format warning")
+	}
 	certificate.CertLen = &Integer{}
 	cert_len := len(data)
 
@@ -174,6 +187,7 @@ func ReadCertificate(data []byte) (certificate *Certificate, remainder []byte, e
 	} else {
 		certificate.CertLen, err = NewInteger(data[1:CERT_MIN_SIZE])
 		//		_, err = certificate.Type()
+		//log.Println("Calculated len AT LEN", cert_len, "Stated len AT LEN", certificate.CertLen.Value())
 		if err != nil {
 			//return
 			log.WithFields(log.Fields{
@@ -182,7 +196,7 @@ func ReadCertificate(data []byte) (certificate *Certificate, remainder []byte, e
 				"certificate_min_size":     CERT_MIN_SIZE,
 				"reason":                   "certificate size is invalid",
 			}).Warn("certificate format warning")
-			err = errors.New("error parsing certificate type: certificate type is invalid")
+			//err = errors.New("error parsing certificate type: certificate type is invalid")
 		}
 		certificate.CertBytes = data[CERT_MIN_SIZE:]
 		_, err = certificate.Length()
@@ -192,12 +206,11 @@ func ReadCertificate(data []byte) (certificate *Certificate, remainder []byte, e
 				certificate.CertLen, err = NewInteger([]byte{00000000})
 				return
 			case "certificate parsing warning: certificate data is shorter than specified by length":
-				//err = nil
 				return
-			case "certificate parsing warning: certificate contains data beyond length":
+			case "certificate parsing warning: certificate data is longer than specified by length":
 				certificate.CertBytes = data[CERT_MIN_SIZE:]
-				remainder = data[CERT_MIN_SIZE+certificate.CertLen.Value():]
+				l, _ := certificate.Length()
-				err = nil
+				remainder = data[CERT_MIN_SIZE+l:]
 				return
 			}
 		}

lib/common/certificate_test.go

@@ -1,8 +1,9 @@
 package common
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestCertificateTypeIsFirstByte(t *testing.T) {
@@ -24,11 +25,13 @@ func TestCertificateLengthCorrect(t *testing.T) {
 
 	bytes := []byte{0x03, 0x00, 0x02, 0xff, 0xff}
 	certificate, _, err := ReadCertificate(bytes)
+	assert.Nil(err, "ReadCertificate() should not return an error with valid data")
+
 	cert_len, err := certificate.Length()
-	assert.Nil(err)
+	assert.Nil(err, "ReadCertificate() should not return an error with valid data")
 
 	assert.Equal(cert_len, 2, "certificate.Length() should return integer from second two bytes")
-	assert.Nil(err)
+	assert.Nil(err, "ReadCertificate() should not return an error with valid data")
 }
 
 func TestCertificateLengthErrWhenTooShort(t *testing.T) {
@@ -52,7 +55,14 @@ func TestCertificateLengthErrWhenDataTooShort(t *testing.T) {
 
 	bytes := []byte{0x03, 0x00, 0x02, 0xff}
 	certificate, _, err := ReadCertificate(bytes)
+	if assert.NotNil(err) {
+		assert.Equal("certificate parsing warning: certificate data is shorter than specified by length", err.Error(), "correct error message should be returned")
+	}
+
 	cert_len, err := certificate.Length()
+	if assert.NotNil(err) {
+		assert.Equal("certificate parsing warning: certificate data is shorter than specified by length", err.Error(), "correct error message should be returned")
+	}
 
 	assert.Equal(cert_len, 2, "certificate.Length() did not return indicated length when data was actually missing")
 	if assert.NotNil(err) {
@@ -80,11 +90,14 @@ func TestCertificateDataWhenTooLong(t *testing.T) {
 
 	bytes := []byte{0x03, 0x00, 0x02, 0xff, 0xff, 0xaa, 0xaa}
 	certificate, _, err := ReadCertificate(bytes)
+	if assert.NotNil(err) {
+		assert.Equal("certificate parsing warning: certificate data is longer than specified by length", err.Error(), "correct error message should be returned")
+	}
 
 	cert_len, err := certificate.Length()
 
 	if assert.NotNil(err) {
-		assert.Equal("certificate parsing warning: certificate contains data beyond length", err.Error(), "correct error message should be returned")
+		assert.Equal("certificate parsing warning: certificate data is longer than specified by length", err.Error(), "correct error message should be returned")
 	}
 
 	assert.Equal(cert_len, 2, "certificate.Length() did not return indicated length when data was too long")
@@ -98,6 +111,9 @@ func TestCertificateDataWhenTooShort(t *testing.T) {
 
 	bytes := []byte{0x03, 0x00, 0x02, 0xff}
 	certificate, _, err := ReadCertificate(bytes)
+	if assert.NotNil(err) {
+		assert.Equal("certificate parsing warning: certificate data is shorter than specified by length", err.Error(), "correct error message should be returned")
+	}
 	cert_data, err := certificate.Data()
 
 	if assert.NotNil(err) {
@@ -142,7 +158,7 @@ func TestReadCertificateWithRemainder(t *testing.T) {
 	assert.Equal(len(cert.Cert()), 5, "ReadCertificate() did not return correct amount of data for certificate with extra data")
 	assert.Equal(len(remainder), 1, "ReadCertificate() returned incorrect length remainder on certificate with extra data")
 	assert.Equal(1, int(remainder[0]), "ReadCertificate() did not return correct remainder value")
-	assert.Nil(err)
+	assert.NotNil(err)
 }
 
 func TestReadCertificateWithInvalidLength(t *testing.T) {

lib/common/key_certificate.go

@@ -28,6 +28,7 @@ payload :: data
 
 import (
 	"errors"
+
 	"github.com/go-i2p/go-i2p/lib/crypto"
 	log "github.com/sirupsen/logrus"
 )
@@ -78,6 +79,10 @@ const (
 	KEYCERT_SPK_SIZE    = 128
 )
 
+const (
+	KEYCERT_MIN_SIZE = 7
+)
+
 type KeyCertificate struct {
 	CertificateInterface
 	PKType   *Integer
@@ -106,15 +111,15 @@ func (key_certificate KeyCertificate) SigningPublicKeyType() (signing_pubkey_typ
 	//	data_len := len(key_certificate.CertificateInterface.CertBytes)
 	if len(key_certificate.SPKType.Bytes()) < 2 {
 		log.WithFields(log.Fields{
-			"at":           "(KeyCertificate) SingingPublicKeyType",
+			"at":           "(KeyCertificate) SigningPublicKeyType",
 			"data_len":     len(key_certificate.SPKType.Bytes()),
 			"required_len": 2,
 			"reason":       "not enough data",
-		}).Error("error retrieving Singning Public Key type")
+		}).Error("error retrieving Signing Public Key type")
 		err = errors.New("error retrieving signing public key type: not enough data")
 		return
 	}
-	log.Println("Signing Public Key Type", key_certificate.SPKType)
+	log.Println("Signing Public Key Type", key_certificate.SPKType) //.Value())
 	return key_certificate.SPKType.Value(), nil
 }
 
@@ -254,7 +259,7 @@ func (key_certificate KeyCertificate) SignatureSize() (size int) {
 func ReadKeyCertificate(data []byte) (key_certificate KeyCertificate, err error) {
 	key_certificate.SPKType = &Integer{}
 	key_certificate.PKType = &Integer{}
-	cert, _, err := ReadCertificate(data)
+	cert, remainder, err := ReadCertificate(data)
 	if err != nil {
 		return
 	}
@@ -262,35 +267,37 @@ func ReadKeyCertificate(data []byte) (key_certificate KeyCertificate, err error)
 	if err != nil {
 		return
 	}
-	log.Println("KEYSANDCERT CERT TYPE=", cert_type, cert.CertBytes)
+	log.Println("KEYSANDCERT CERT TYPE=", cert_type, cert.CertBytes, remainder)
 	key_certificate.CertificateInterface = cert
-	data = cert.CertBytes
+	data = cert.Cert()
 	data_len := len(data)
-	if data_len < 2 {
+	if data_len < KEYCERT_MIN_SIZE {
-		log.WithFields(log.Fields{
-			"at":           "(KeyCertificate) SigningPublicKeyType",
-			"data_len":     data_len,
-			"required_len": 2,
-			"reason":       "not enough data",
-		}).Error("error parsing key certificate signing public key")
-		err = errors.New("error parsing key certificate signing public key: not enough data")
-		//		key_certificate.SPKType, err = NewInteger(data[:])
-		key_certificate.PKType, err = NewInteger(data[:])
-		return
-	}
-	//	key_certificate.SPKType, err = NewInteger(data[0:2])
-	key_certificate.PKType, err = NewInteger(data[0:2])
-	if data_len < 4 {
 		log.WithFields(log.Fields{
 			"at":           "(KeyCertificate) PublicKeyType",
 			"data_len":     data_len,
-			"required_len": 4,
+			"required_len": KEYCERT_MIN_SIZE,
 			"reason":       "not enough data",
 		}).Error("error parsing key certificate public key")
 		err = errors.New("error parsing key certificate public key: not enough data")
 		return
 	}
-	//	key_certificate.PKType, err = NewInteger(data[2:4])
+	log.Println("KEYSANDCERT=", data, "| len=", data_len, "| 0=", data[0], "| 1=", data[1])
-	key_certificate.SPKType, err = NewInteger(data[2:4])
+	key_certificate.SPKType, err = NewInteger(data[len(data)-2 : len(data)])
+	if err != nil {
+		log.WithFields(log.Fields{
+			"at":       "(KeyCertificate) SigningPublicKeyType",
+			"key_type": key_certificate.PKType,
+			"reason":   "failed to read signing public key type",
+		}).Error("error parsing key certificate signing public key")
+	}
+	key_certificate.PKType, err = NewInteger(data[len(data)-4 : len(data)-2])
+	if err != nil {
+		log.WithFields(log.Fields{
+			"at":       "(KeyCertificate) PublicKeyType",
+			"key_type": key_certificate.PKType,
+			"reason":   "failed to read public key type",
+		}).Error("error parsing key certificate public key")
+		err = errors.New("error parsing key certificate public key: not enough data")
+	}
 	return
 }

lib/common/key_certificate_test.go

@@ -1,8 +1,9 @@
 package common
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestSingingPublicKeyTypeReturnsCorrectInteger(t *testing.T) {
@@ -20,6 +21,9 @@ func TestPublicKeyTypeReportsWhenDataTooSmall(t *testing.T) {
 	assert := assert.New(t)
 
 	key_cert, err := ReadKeyCertificate([]byte{0x05, 0x00, 0x01, 0x00})
+	if assert.NotNil(err) {
+		assert.Equal("error parsing key certificate public key: not enough data", err.Error(), "correct error message should be returned")
+	}
 	//	assert.NotNil(err, "ReadKeyCertificate() returned error with valid data")
 	_, err = key_cert.PublicKeyType()