diff --git a/LIST_OF_NON_BACKWARD_COMPATIBLE_CHANGES_FOR_V07.txt b/LIST_OF_NON_BACKWARD_COMPATIBLE_CHANGES_FOR_V07.txt new file mode 100644 index 000000000..83a9efa1b --- /dev/null +++ b/LIST_OF_NON_BACKWARD_COMPATIBLE_CHANGES_FOR_V07.txt @@ -0,0 +1,14 @@ +V07_NON_BACKWARD_COMPATIBLE_CHANGE_001: + + What: Computes the node id by performing a sha256 hash of the certificate's PGP signature, instead of simply picking up the last 20 bytes of it. + + Why: There is no real risk in forging a certificate with the same ID as the authentication is performed over the PGP signature of the certificate + which hashes the full SSL certificate (i.e. the full serialized CERT_INFO structure). However the possibility to + create two certificates with the same IDs is a problem, as it can be used to cause disturbance in the software. + + Backward compat: makes connexions impossible with non patched peers, probably because the SSL id that is computed is not the same on both side, + and in particular unpatched peers see a cerficate with ID different (because computed with the old method) than the ID that was + submitted when making friends. + + Note: the advantage of basing the ID on the signature rather than the public key is not very clear, given that the signature is based on a hash + of the public key (and the rest of the certificate info). diff --git a/libretroshare/src/pqi/sslfns.cc b/libretroshare/src/pqi/sslfns.cc index ab79d9825..16ba1744d 100644 --- a/libretroshare/src/pqi/sslfns.cc +++ b/libretroshare/src/pqi/sslfns.cc @@ -610,6 +610,22 @@ bool getX509id(X509 *x509, RsPeerId& xid) X509_get0_signature(&signature,&algor,x509); #endif + +#ifndef V07_NON_BACKWARD_COMPATIBLE_CHANGE_001 + + // What: Computes the node id by performing a sha256 hash of the certificate's PGP signature, instead of simply picking up the last 20 bytes of it. + // + // Why: There is no real risk in forging a certificate with the same ID as the authentication is performed over the PGP signature of the certificate + // which hashes the full SSL certificate (i.e. the full serialized CERT_INFO structure). However the possibility to + // create two certificates with the same IDs is a problem, as it can be used to cause disturbance in the software. + // + // Backward compat: makes connexions impossible with non patched peers, probably because the SSL id that is computed is not the same on both side, + // and in particular unpatched peers see a cerficate with ID different (because computed with the old method) than the ID that was + // submitted when making friends. + // + // Note: the advantage of basing the ID on the signature rather than the public key is not very clear, given that the signature is based on a hash + // of the public key (and the rest of the certificate info). + // int signlen = ASN1_STRING_length(signature); if (signlen < CERTSIGNLEN) { @@ -627,14 +643,16 @@ bool getX509id(X509 *x509, RsPeerId& xid) * more randomness */ -#warning csoler 2017-02-19: This is cryptographically horrible. We should do a hash of the public key here!!! +#warning csoler 2017-02-19: This is cryptographically horrible. We should hash the entire signature here! xid = RsPeerId(&signdata[signlen - CERTSIGNLEN]) ; +#else - //for(int i = signlen - CERTSIGNLEN; i < signlen; i++) - //{ - // rs_sprintf_append(xid, "%02x", (uint16_t) (((uint8_t *) (signdata))[i])); - //} + if(RsPeerId::SIZE_IN_BYTES > Sha256CheckSum::SIZE_IN_BYTES) + return false ; + + xid = RsPeerId(RsDirUtil::sha256sum(ASN1_STRING_data(const_cast(signature)),ASN1_STRING_length(signature)).toByteArray()) ; +#endif return true; } diff --git a/libretroshare/src/retroshare/rsids.h b/libretroshare/src/retroshare/rsids.h index addd08a85..21a8fd754 100644 --- a/libretroshare/src/retroshare/rsids.h +++ b/libretroshare/src/retroshare/rsids.h @@ -221,6 +221,7 @@ static const int CERT_SIGN_LEN = 16 ; // = CERTSIGNLEN static const int PGP_KEY_ID_SIZE = 8 ; static const int PGP_KEY_FINGERPRINT_SIZE = 20 ; static const int SHA1_SIZE = 20 ; +static const int SHA256_SIZE = 32 ; // These constants are random, but should be different, in order to make the various IDs incompatible with each other. // @@ -236,10 +237,12 @@ static const uint32_t RS_GENERIC_ID_GROUTER_ID_TYPE = 0x0009 ; static const uint32_t RS_GENERIC_ID_GXS_TUNNEL_ID_TYPE = 0x0010 ; static const uint32_t RS_GENERIC_ID_GXS_DISTANT_CHAT_ID_TYPE = 0x0011 ; static const uint32_t RS_GENERIC_ID_NODE_GROUP_ID_TYPE = 0x0012 ; +static const uint32_t RS_GENERIC_ID_SHA256_ID_TYPE = 0x0013 ; typedef t_RsGenericIdType< SSL_ID_SIZE , false, RS_GENERIC_ID_SSL_ID_TYPE> SSLIdType ; typedef t_RsGenericIdType< PGP_KEY_ID_SIZE , true, RS_GENERIC_ID_PGP_ID_TYPE> PGPIdType ; typedef t_RsGenericIdType< SHA1_SIZE , false, RS_GENERIC_ID_SHA1_ID_TYPE> Sha1CheckSum ; +typedef t_RsGenericIdType< SHA256_SIZE , false, RS_GENERIC_ID_SHA256_ID_TYPE> Sha256CheckSum ; typedef t_RsGenericIdType< PGP_KEY_FINGERPRINT_SIZE, true, RS_GENERIC_ID_PGP_FINGERPRINT_TYPE> PGPFingerprintType ; typedef t_RsGenericIdType< CERT_SIGN_LEN , false, RS_GENERIC_ID_GXS_GROUP_ID_TYPE > GXSGroupId ; diff --git a/libretroshare/src/util/rsdir.cc b/libretroshare/src/util/rsdir.cc index 42e075cdf..8be50f277 100644 --- a/libretroshare/src/util/rsdir.cc +++ b/libretroshare/src/util/rsdir.cc @@ -610,6 +610,27 @@ Sha1CheckSum RsDirUtil::sha1sum(const unsigned char *data, uint32_t size) return Sha1CheckSum(sha_buf) ; } +Sha256CheckSum RsDirUtil::sha256sum(const unsigned char *data, uint32_t size) +{ + SHA256_CTX sha_ctx ; + + if(SHA256_DIGEST_LENGTH != 32) + throw std::runtime_error("Warning: can't compute Sha2561Sum with sum size != 32") ; + + SHA256_Init(&sha_ctx); + while(size > 512) + { + SHA256_Update(&sha_ctx, data, 512); + data = &data[512] ; + size -= 512 ; + } + SHA256_Update(&sha_ctx, data, size); + + unsigned char sha_buf[SHA256_DIGEST_LENGTH]; + SHA256_Final(&sha_buf[0], &sha_ctx); + + return Sha256CheckSum(sha_buf) ; +} bool RsDirUtil::saveStringToFile(const std::string &file, const std::string &str) { std::ofstream out(file.c_str(), std::ios_base::out | std::ios_base::binary); diff --git a/libretroshare/src/util/rsdir.h b/libretroshare/src/util/rsdir.h index 8f52b009f..3fd3b9f1d 100644 --- a/libretroshare/src/util/rsdir.h +++ b/libretroshare/src/util/rsdir.h @@ -101,7 +101,8 @@ bool cleanupDirectoryFaster(const std::string& dir, const std::set