From f018fefeb4bd9a9e6506835a3c98c89f6bfe2abf Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 9 Jul 2020 20:08:08 -0700 Subject: [PATCH] Even less bugs! --- cmd/zt_service_tests/certificate.go | 47 ++++++++++----------- core/Certificate.cpp | 12 +++--- core/Dictionary.hpp | 27 +++++++++++++ pkg/zerotier/certificate.go | 23 +++++------ pkg/zerotier/identity.go | 63 ++++++++++++++--------------- pkg/zerotier/locator.go | 14 +++---- pkg/zerotier/node.go | 10 +---- 7 files changed, 107 insertions(+), 89 deletions(-) diff --git a/cmd/zt_service_tests/certificate.go b/cmd/zt_service_tests/certificate.go index c1c780f0b..47ee0a944 100644 --- a/cmd/zt_service_tests/certificate.go +++ b/cmd/zt_service_tests/certificate.go @@ -38,7 +38,7 @@ func TestCertificate() bool { var c zerotier.Certificate c.SerialNo = make([]byte, 48) - for i := 0; i < 48 ;i++ { + for i := 0; i < 48; i++ { c.SerialNo[i] = byte(i) } c.Flags = 1234 @@ -49,13 +49,13 @@ func TestCertificate() bool { c.Subject.Timestamp = 31337 c.Subject.Identities = append(c.Subject.Identities, zerotier.CertificateIdentity{ Identity: id, - Locator: nil, + Locator: nil, }) c.Subject.Networks = append(c.Subject.Networks, zerotier.CertificateNetwork{ ID: 1111, Controller: zerotier.Fingerprint{ Address: zerotier.Address(2222), - Hash: c.SerialNo, + Hash: c.SerialNo, }, }) c.Subject.Certificates = append(c.Subject.Certificates, c.SerialNo) @@ -115,25 +115,26 @@ func TestCertificate() bool { } fmt.Printf("Checking certificate marshal/unmarshal... ") - cb, err := c.Marshal() - if err != nil { - fmt.Printf("marshal FAILED (%s)\n", err.Error()) - return false - } - fmt.Printf("marshal: %d bytes ", len(cb)) - c2, err = zerotier.NewCertificateFromBytes(cb, false) - if err != nil { - fmt.Printf("unmarshal FAILED (%s)\n", err.Error()) - return false - } - cb2, err := c2.Marshal() - if err != nil { - fmt.Printf("second marshal FAILED (%s)\n", err.Error()) - return false - } - if !bytes.Equal(cb, cb2) { - fmt.Printf("FAILED (results not equal)\n") - return false + for k := 0; k < 1024; k++ { + cb, err := c.Marshal() + if err != nil { + fmt.Printf("marshal FAILED (%s)\n", err.Error()) + return false + } + c2, err = zerotier.NewCertificateFromBytes(cb, false) + if err != nil { + fmt.Printf("unmarshal FAILED (%s)\n", err.Error()) + return false + } + cb2, err := c2.Marshal() + if err != nil { + fmt.Printf("second marshal FAILED (%s)\n", err.Error()) + return false + } + if !bytes.Equal(cb, cb2) { + fmt.Printf("FAILED (results not equal)\n") + return false + } } fmt.Println("OK") @@ -143,7 +144,7 @@ func TestCertificate() bool { fmt.Printf("CSR generate FAILED (%s)\n", err.Error()) return false } - fmt.Printf("CSR size: %d ",len(csr)) + fmt.Printf("CSR size: %d ", len(csr)) csr2, err := zerotier.NewCertificateFromBytes(csr, false) if err != nil { fmt.Printf("CSR decode FAILED (%s)\n", err.Error()) diff --git a/core/Certificate.cpp b/core/Certificate.cpp index 405778402..f89d40b82 100644 --- a/core/Certificate.cpp +++ b/core/Certificate.cpp @@ -307,19 +307,19 @@ bool Certificate::decode(const void *const data, const unsigned int len) unsigned int cnt = (unsigned int)d.getUI("s.i$"); for (unsigned int i = 0; i < cnt; ++i) { const Vector< uint8_t > &identityData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.i$.i", i)]; + const Vector< uint8_t > &locatorData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.i$.l", i)]; if (identityData.empty()) return false; Identity id; if (id.unmarshal(identityData.data(), (unsigned int)identityData.size()) <= 0) return false; - const Vector< uint8_t > &locatorData = d[Dictionary::arraySubscript(tmp, sizeof(tmp), "s.i$.l", i)]; - if (!locatorData.empty()) { + if (locatorData.empty()) { + this->addSubjectIdentity(id); + } else { Locator loc; if (loc.unmarshal(locatorData.data(), (unsigned int)locatorData.size()) <= 0) return false; this->addSubjectIdentity(id, loc); - } else { - this->addSubjectIdentity(id); } } @@ -463,10 +463,12 @@ ZT_CertificateError Certificate::verify() const (this->subject.uniqueIdSize != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE) || (this->subject.uniqueId[0] != ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384)) return ZT_CERTIFICATE_ERROR_INVALID_UNIQUE_ID_PROOF; + Dictionary d; m_encodeSubject(this->subject, d, true); Vector< uint8_t > enc; d.encode(enc); + uint8_t h[ZT_SHA384_DIGEST_SIZE]; SHA384(h, enc.data(), (unsigned int)enc.size()); static_assert(ZT_CERTIFICATE_UNIQUE_ID_TYPE_NIST_P_384_SIZE == (ZT_ECC384_PUBLIC_KEY_SIZE + 1), "incorrect size"); @@ -510,7 +512,7 @@ ZT_CertificateError Certificate::verify() const Vector< uint8_t > Certificate::createCSR(const ZT_Certificate_Subject &s, const void *uniqueId, unsigned int uniqueIdSize, const void *uniqueIdPrivate, unsigned int uniqueIdPrivateSize) { ZT_Certificate_Subject sc; - Utils::copy< sizeof(ZT_Certificate_Subject) >(&sc, &s); + Utils::copy< sizeof(ZT_Certificate_Subject) >(&sc,&s); if ((uniqueId) && (uniqueIdSize > 0) && (uniqueIdPrivate) && (uniqueIdPrivateSize > 0)) { sc.uniqueId = reinterpret_cast(uniqueId); diff --git a/core/Dictionary.hpp b/core/Dictionary.hpp index a9b6be690..0dcb3af59 100644 --- a/core/Dictionary.hpp +++ b/core/Dictionary.hpp @@ -48,6 +48,33 @@ public: ~Dictionary(); + ///* + ZT_INLINE void dump() const + { + printf("\n--\n"); + for (const_iterator e(begin()); e != end(); ++e) { + printf("%.8x %s=", Utils::fnv1a32(e->second.data(), (unsigned int)e->second.size()), e->first.c_str()); + bool binary = false; + for (Vector< uint8_t >::const_iterator c(e->second.begin()); c != e->second.end(); ++c) { + if ((*c < 33) || (*c > 126)) { + binary = true; + break; + } + } + if (binary) { + for (Vector< uint8_t >::const_iterator c(e->second.begin()); c != e->second.end(); ++c) + printf("%.2x", (unsigned int)*c); + } else { + Vector< uint8_t > s(e->second); + s.push_back(0); + printf("%s", s.data()); + } + printf("\n"); + } + printf("--\n"); + } + //*/ + /** * Get a reference to a value * diff --git a/pkg/zerotier/certificate.go b/pkg/zerotier/certificate.go index 6e2068596..d92d05412 100644 --- a/pkg/zerotier/certificate.go +++ b/pkg/zerotier/certificate.go @@ -137,13 +137,13 @@ func NewCertificateFromBytes(cert []byte, verify bool) (*Certificate, error) { ver = 1 } cerr := C.ZT_Certificate_decode((**C.ZT_Certificate)(unsafe.Pointer(&dec)), unsafe.Pointer(&cert[0]), C.int(len(cert)), ver) - defer C.ZT_Certificate_delete((*C.ZT_Certificate)(dec)) if cerr != 0 { return nil, certificateErrorToError(int(cerr)) } - if dec == unsafe.Pointer(uintptr(0)) { + if dec == nil { return nil, ErrInternal } + defer C.ZT_Certificate_delete((*C.ZT_Certificate)(dec)) goCert := NewCertificateFromCCertificate(dec) if goCert == nil { @@ -299,10 +299,10 @@ func (c *Certificate) CCertificate() *CCertificate { if len(c.Subject.Identities) > 0 { subjectIdentities = make([]C.ZT_Certificate_Identity, len(c.Subject.Identities)) for i, id := range c.Subject.Identities { - if id.Identity == nil || !id.Identity.initCIdentityPtr() { + if id.Identity == nil { return nil } - subjectIdentities[i].identity = id.Identity.cid + subjectIdentities[i].identity = id.Identity.cIdentity() if id.Locator != nil { subjectIdentities[i].locator = id.Locator.cl } @@ -370,10 +370,7 @@ func (c *Certificate) CCertificate() *CCertificate { } if c.Issuer != nil { - if !c.Issuer.initCIdentityPtr() { - return nil - } - cc.issuer = c.Issuer.cid + cc.issuer = c.Issuer.cIdentity() } cStrCopy(unsafe.Pointer(&cc.issuerName.serialNo[0]), CertificateMaxStringLength+1, c.IssuerName.SerialNo) @@ -406,12 +403,14 @@ func (c *Certificate) CCertificate() *CCertificate { // to throw away 'cc' and its components. cc2 := &CCertificate{C: unsafe.Pointer(C._ZT_Certificate_clone2(C.uintptr_t(uintptr(unsafe.Pointer(&cc)))))} runtime.SetFinalizer(cc2, func(obj interface{}) { - C.ZT_Certificate_delete((*C.ZT_Certificate)(obj.(*CCertificate).C)) + if obj != nil { + C.ZT_Certificate_delete((*C.ZT_Certificate)(obj.(*CCertificate).C)) + } }) return cc2 } -// Marshal encodes this certificae as a byte array. +// Marshal encodes this certificate as a byte array. func (c *Certificate) Marshal() ([]byte, error) { cc := c.CCertificate() if cc == nil { @@ -431,7 +430,7 @@ func (c *Certificate) Marshal() ([]byte, error) { // parts of this Certificate, if any, are ignored. A new Certificate is returned with a completed // signature. func (c *Certificate) Sign(id *Identity) (*Certificate, error) { - if id == nil || !id.HasPrivate() || !id.initCIdentityPtr() { + if id == nil || !id.HasPrivate() { return nil, ErrInvalidParameter } ctmp := c.CCertificate() @@ -440,7 +439,7 @@ func (c *Certificate) Sign(id *Identity) (*Certificate, error) { } var signedCert [16384]byte signedCertSize := C.int(16384) - rv := int(C.ZT_Certificate_sign((*C.ZT_Certificate)(ctmp.C), id.cid, unsafe.Pointer(&signedCert[0]), &signedCertSize)) + rv := int(C.ZT_Certificate_sign((*C.ZT_Certificate)(ctmp.C), id.cIdentity(), unsafe.Pointer(&signedCert[0]), &signedCertSize)) if rv != 0 { return nil, fmt.Errorf("signing failed: error %d", rv) } diff --git a/pkg/zerotier/identity.go b/pkg/zerotier/identity.go index b10dee291..6f5718544 100644 --- a/pkg/zerotier/identity.go +++ b/pkg/zerotier/identity.go @@ -43,18 +43,18 @@ type Identity struct { idtype int publicKey []byte privateKey []byte - cid unsafe.Pointer // ZT_Identity + cid unsafe.Pointer } func identityFinalizer(obj interface{}) { - id, _ := obj.(*Identity) - if id != nil && uintptr(id.cid) != 0 { - C.ZT_Identity_delete(id.cid) + cid := obj.(*Identity).cid + if cid != nil { + C.ZT_Identity_delete(cid) } } func newIdentityFromCIdentity(cid unsafe.Pointer) (*Identity, error) { - if uintptr(cid) == 0 { + if cid == nil { return nil, ErrInvalidParameter } @@ -69,39 +69,44 @@ func newIdentityFromCIdentity(cid unsafe.Pointer) (*Identity, error) { return nil, err } - id.cid = cid runtime.SetFinalizer(id, identityFinalizer) return id, nil } // initCIdentityPtr returns a pointer to the core ZT_Identity instance or nil/0 on error. -func (id *Identity) initCIdentityPtr() bool { - if uintptr(id.cid) == 0 { - str := id.PrivateKeyString() +func (id *Identity) cIdentity() unsafe.Pointer { + if id.cid == nil { + str := []byte(id.PrivateKeyString()) if len(str) == 0 { - str = id.String() + str = []byte(id.String()) } - idCStr := C.CString(str) - defer C.free(unsafe.Pointer(idCStr)) - id.cid = C.ZT_Identity_fromString(idCStr) - if uintptr(id.cid) == 0 { - return false + if len(str) == 0 { + return nil } - runtime.SetFinalizer(id, identityFinalizer) + str = append(str, byte(0)) + id.cid = C.ZT_Identity_fromString((*C.char)(unsafe.Pointer(&str[0]))) } - return true + return id.cid } // NewIdentity generates a new identity of the selected type. func NewIdentity(identityType int) (*Identity, error) { + var cid unsafe.Pointer switch identityType { case C.ZT_IDENTITY_TYPE_C25519: - return newIdentityFromCIdentity(C.ZT_Identity_new(C.ZT_IDENTITY_TYPE_C25519)) + cid = C.ZT_Identity_new(C.ZT_IDENTITY_TYPE_C25519) case C.ZT_IDENTITY_TYPE_P384: - return newIdentityFromCIdentity(C.ZT_Identity_new(C.ZT_IDENTITY_TYPE_P384)) + cid = C.ZT_Identity_new(C.ZT_IDENTITY_TYPE_P384) + default: + return nil, ErrInvalidParameter } - return nil, ErrInvalidParameter + id, err := newIdentityFromCIdentity(cid) + if err != nil { + return nil, err + } + id.cid = cid + return id, nil } // NewIdentityFromString generates a new identity from its string representation. @@ -172,8 +177,7 @@ func (id *Identity) HasPrivate() bool { return len(id.privateKey) > 0 } // Fingerprint gets this identity's address plus hash of public key(s). func (id *Identity) Fingerprint() *Fingerprint { - id.initCIdentityPtr() - return newFingerprintFromCFingerprint(C.ZT_Identity_fingerprint(id.cid)) + return newFingerprintFromCFingerprint(C.ZT_Identity_fingerprint(id.cIdentity())) } // PrivateKeyString returns the full identity.secret if the private key is set, @@ -210,24 +214,17 @@ func (id *Identity) String() string { // LocallyValidate performs local self-validation of this identity func (id *Identity) LocallyValidate() bool { - if !id.initCIdentityPtr() { - return false - } - return C.ZT_Identity_validate(id.cid) != 0 + return C.ZT_Identity_validate(id.cIdentity()) != 0 } // Sign signs a message with this identity func (id *Identity) Sign(msg []byte) ([]byte, error) { - if !id.initCIdentityPtr() { - return nil, ErrInvalidKey - } - var dataP unsafe.Pointer if len(msg) > 0 { dataP = unsafe.Pointer(&msg[0]) } var sig [96]byte - sigLen := C.ZT_Identity_sign(id.cid, dataP, C.uint(len(msg)), unsafe.Pointer(&sig[0]), 96) + sigLen := C.ZT_Identity_sign(id.cIdentity(), dataP, C.uint(len(msg)), unsafe.Pointer(&sig[0]), 96) if sigLen <= 0 { return nil, ErrInvalidKey } @@ -237,14 +234,14 @@ func (id *Identity) Sign(msg []byte) ([]byte, error) { // Verify verifies a signature func (id *Identity) Verify(msg, sig []byte) bool { - if len(sig) == 0 || !id.initCIdentityPtr() { + if len(sig) == 0 { return false } var dataP unsafe.Pointer if len(msg) > 0 { dataP = unsafe.Pointer(&msg[0]) } - return C.ZT_Identity_verify(id.cid, dataP, C.uint(len(msg)), unsafe.Pointer(&sig[0]), C.uint(len(sig))) != 0 + return C.ZT_Identity_verify(id.cIdentity(), dataP, C.uint(len(msg)), unsafe.Pointer(&sig[0]), C.uint(len(sig))) != 0 } // Equals performs a deep equality test between this and another identity diff --git a/pkg/zerotier/locator.go b/pkg/zerotier/locator.go index c03d19b04..0926066b8 100644 --- a/pkg/zerotier/locator.go +++ b/pkg/zerotier/locator.go @@ -30,10 +30,10 @@ type Locator struct { cl unsafe.Pointer } -func newLocatorFromCLocator(cl unsafe.Pointer, requiresDeletion bool) (*Locator, error) { +func newLocatorFromCLocator(cl unsafe.Pointer, needFinalizer bool) (*Locator, error) { loc := new(Locator) loc.cl = cl - err := loc.init(requiresDeletion) + err := loc.init(needFinalizer) if err != nil { return nil, err } @@ -48,8 +48,7 @@ func NewLocator(ts int64, endpoints []Endpoint, signer *Identity) (*Locator, err for _, e := range endpoints { eps = append(eps, e.cep) } - signer.initCIdentityPtr() - loc := C.ZT_Locator_create(C.int64_t(ts), &eps[0], C.uint(len(eps)), signer.cid) + loc := C.ZT_Locator_create(C.int64_t(ts), &eps[0], C.uint(len(eps)), signer.cIdentity()) if uintptr(loc) == 0 { return nil, ErrInvalidParameter } @@ -93,8 +92,7 @@ func (loc *Locator) Validate(id *Identity) bool { if id == nil { return false } - id.initCIdentityPtr() - return C.ZT_Locator_verify(loc.cl, id.cid) != 0 + return C.ZT_Locator_verify(loc.cl, id.cIdentity()) != 0 } func (loc *Locator) Bytes() []byte { @@ -135,7 +133,7 @@ func locatorFinalizer(obj interface{}) { } } -func (loc *Locator) init(requiresDeletion bool) error { +func (loc *Locator) init(needFinalizer bool) error { loc.Timestamp = int64(C.ZT_Locator_timestamp(loc.cl)) cfp := C.ZT_Locator_fingerprint(loc.cl) if uintptr(unsafe.Pointer(cfp)) == 0 { @@ -149,7 +147,7 @@ func (loc *Locator) init(requiresDeletion bool) error { } var buf [4096]byte loc.String = C.GoString(C.ZT_Locator_toString(loc.cl, (*C.char)(unsafe.Pointer(&buf[0])), 4096)) - if requiresDeletion { + if needFinalizer { runtime.SetFinalizer(loc, locatorFinalizer) } return nil diff --git a/pkg/zerotier/node.go b/pkg/zerotier/node.go index 910e78e2c..83c2669c7 100644 --- a/pkg/zerotier/node.go +++ b/pkg/zerotier/node.go @@ -449,10 +449,7 @@ func (n *Node) Leave(nwid NetworkID) error { // AddRoot designates a peer as root, adding it if missing. func (n *Node) AddRoot(id *Identity) (*Peer, error) { - if !id.initCIdentityPtr() { - return nil, ErrInvalidKey - } - rc := C.ZT_Node_addRoot(n.zn, nil, id.cid) + rc := C.ZT_Node_addRoot(n.zn, nil, id.cIdentity()) if rc != 0 { return nil, ErrInvalidParameter } @@ -537,10 +534,7 @@ func (n *Node) AddPeer(id *Identity) error { if id == nil { return ErrInvalidParameter } - if !id.initCIdentityPtr() { - return ErrInvalidKey - } - rc := C.ZT_Node_addPeer(n.zn, nil, id.cid) + rc := C.ZT_Node_addPeer(n.zn, nil, id.cIdentity()) if rc != 0 { return ErrInvalidParameter }