centralized the defines into a single file

This commit is contained in:
csoler 2017-11-19 19:34:54 +01:00
parent e72bd9ff4f
commit 863e6256c3
5 changed files with 55 additions and 47 deletions

View File

@ -1,31 +0,0 @@
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).
V07_NON_BACKWARD_COMPATIBLE_CHANGE_002:
What: Use RSA+SHA256 instead of RSA+SHA1 for PGP certificate signatures
Why: Sha1 is likely to be prone to primary collisions anytime soon, so it is urgent to turn to a more secure solution.
Backward compat: unpatched peers are able to verify signatures since openpgp-sdk already handle it.
V07_NON_BACKWARD_COMPATIBLE_CHANGE_003:
What: Do not hash PGP certificate twice when signing
Why: hasing twice is not per se a security issue, but it makes it harder to change the settings for hashing.
Backward compat: patched peers cannot connect to non patched peers.

View File

@ -1382,7 +1382,7 @@ ops_secret_key_t *secret_key = NULL ;
// then do the signature.
ops_boolean_t not_raw = !use_raw_signature ;
#ifdef V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
#if V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
ops_memory_t *memres = ops_sign_buf(data,len,OPS_SIG_BINARY,OPS_HASH_SHA256,secret_key,ops_false,ops_false,not_raw,not_raw) ;
#else
ops_memory_t *memres = ops_sign_buf(data,len,OPS_SIG_BINARY,OPS_HASH_SHA1,secret_key,ops_false,ops_false,not_raw,not_raw) ;

View File

@ -43,6 +43,7 @@
#include "pgp/pgpkeyutil.h"
#include "retroshare/rspeers.h" // for RsPeerDetails structure
#include "retroshare/rsdefines.h"
#include "retroshare/rsids.h" // for RsPeerDetails structure
#include "rsserver/p3face.h"
@ -821,7 +822,7 @@ X509 *AuthSSLimpl::SignX509ReqWithGPG(X509_REQ *req, long /*days*/)
ASN1_BIT_STRING *signature = const_cast<ASN1_BIT_STRING*>(tmp_signature);
#endif
//EVP_PKEY *pkey = NULL;
#ifdef V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
#if V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
const EVP_MD *type = EVP_sha256();
#else
const EVP_MD *type = EVP_sha1();
@ -871,7 +872,7 @@ X509 *AuthSSLimpl::SignX509ReqWithGPG(X509_REQ *req, long /*days*/)
inl=i2d_re_X509_tbs(x509,&buf_in) ; // this does the i2d over x509->cert_info
#endif
#ifdef V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
#if V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
sigoutl=2048; // hashoutl; //EVP_PKEY_size(pkey);
unsigned char *buf_sigout=(unsigned char *)OPENSSL_malloc((unsigned int)sigoutl);
@ -965,8 +966,10 @@ X509 *AuthSSLimpl::SignX509ReqWithGPG(X509_REQ *req, long /*days*/)
/* cleanup */
if(buf_in != NULL)
OPENSSL_free(buf_in) ;
#if !V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
if(buf_hashout != NULL)
OPENSSL_free(buf_hashout) ;
#endif
if(buf_sigout != NULL)
OPENSSL_free(buf_sigout) ;
std::cerr << "GPGAuthMgr::SignX509Req() err: FAIL" << std::endl;
@ -1025,7 +1028,7 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic)
#endif
#ifdef V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
#if V07_NON_BACKWARD_COMPATIBLE_CHANGE_002
const EVP_MD *type = EVP_sha256();
#else
const EVP_MD *type = EVP_sha1();
@ -1075,7 +1078,7 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic)
i2d(data,&p);
#endif
#ifndef V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
#if !V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
/* data in buf_in, ready to be hashed */
EVP_DigestInit_ex(ctx,type, NULL);
EVP_DigestUpdate(ctx,(unsigned char *)buf_in,inl);
@ -1108,7 +1111,7 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic)
std::cerr << "AuthSSLimpl::AuthX509() verifying the gpg sig with keyprint : " << pd.fpr << std::endl;
std::cerr << "Sigoutl = " << sigoutl << std::endl ;
std::cerr << "pd.fpr = " << pd.fpr << std::endl ;
#ifndef V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
#if !V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
std::cerr << "hashoutl = " << hashoutl << std::endl ;
#endif
#endif
@ -1173,7 +1176,7 @@ bool AuthSSLimpl::AuthX509WithGPG(X509 *x509,uint32_t& diagnostic)
// passed, verify the signature itself
#ifdef V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
#if V07_NON_BACKWARD_COMPATIBLE_CHANGE_003
if (!AuthGPG::getAuthGPG()->VerifySignBin(buf_in, inl, buf_sigout, (unsigned int) sigoutl, pd.fpr)) {
#else
if (!AuthGPG::getAuthGPG()->VerifySignBin(buf_hashout, hashoutl, buf_sigout, (unsigned int) sigoutl, pd.fpr)) {

View File

@ -611,8 +611,7 @@ bool getX509id(X509 *x509, RsPeerId& xid)
X509_get0_signature(&signature,&algor,x509);
#endif
#ifndef V07_NON_BACKWARD_COMPATIBLE_CHANGE_001
#if 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
@ -626,6 +625,12 @@ bool getX509id(X509 *x509, RsPeerId& xid)
// 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).
//
if(RsPeerId::SIZE_IN_BYTES > Sha256CheckSum::SIZE_IN_BYTES)
return false ;
xid = RsPeerId(RsDirUtil::sha256sum(ASN1_STRING_data(const_cast<ASN1_BIT_STRING*>(signature)),ASN1_STRING_length(signature)).toByteArray()) ;
#else
int signlen = ASN1_STRING_length(signature);
if (signlen < CERTSIGNLEN)
{
@ -646,12 +651,6 @@ bool getX509id(X509 *x509, RsPeerId& xid)
#warning csoler 2017-02-19: This is cryptographically horrible. We should hash the entire signature here!
xid = RsPeerId(&signdata[signlen - CERTSIGNLEN]) ;
#else
if(RsPeerId::SIZE_IN_BYTES > Sha256CheckSum::SIZE_IN_BYTES)
return false ;
xid = RsPeerId(RsDirUtil::sha256sum(ASN1_STRING_data(const_cast<ASN1_BIT_STRING*>(signature)),ASN1_STRING_length(signature)).toByteArray()) ;
#endif
return true;

View File

@ -0,0 +1,37 @@
/**************************************************************************************************************************************************
*
* 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).
*
* V07_NON_BACKWARD_COMPATIBLE_CHANGE_002:
*
* What: Use RSA+SHA256 instead of RSA+SHA1 for PGP certificate signatures
*
* Why: Sha1 is likely to be prone to primary collisions anytime soon, so it is urgent to turn to a more secure solution.
*
* Backward compat: unpatched peers are able to verify signatures since openpgp-sdk already handle it.
*
* V07_NON_BACKWARD_COMPATIBLE_CHANGE_003:
*
* What: Do not hash PGP certificate twice when signing
*
* Why: hasing twice is not per se a security issue, but it makes it harder to change the settings for hashing.
*
* Backward compat: patched peers cannot connect to non patched peers.
***************************************************************************************************************************************************/
#define V07_NON_BACKWARD_COMPATIBLE_CHANGE_001 False
#define V07_NON_BACKWARD_COMPATIBLE_CHANGE_002 False
#define V07_NON_BACKWARD_COMPATIBLE_CHANGE_003 False