* #9641: audiortp: Fixed CryptoContext management

They are cached in AudioRtpFactory, but may be replaced in
AudioSrtpSession. Therefore, whenever we reinitialize these contexts
we must update the cached values in audiortpfactory. In addition, since
they will outlive a given AudioSrtpSession, they must NOT be deleted in
AudioSrtpSession's destructor but rather in AudioRtpFactory's.

daemon/src/audio/audiortp/audio_rtp_factory.cpp

@@ -44,13 +44,15 @@ namespace sfl {
 
 AudioRtpFactory::AudioRtpFactory(SIPCall *ca) : rtpSession_(NULL),
     audioRtpThreadMutex_(), srtpEnabled_(false), helloHashEnabled_(false),
-    remoteContext_(NULL), localContext_(NULL), ca_(ca),
+    cachedRemoteContext_(0), cachedLocalContext_(0), ca_(ca),
     keyExchangeProtocol_(NONE)
 {}
 
 AudioRtpFactory::~AudioRtpFactory()
 {
     delete rtpSession_;
+    delete cachedLocalContext_;
+    delete cachedRemoteContext_;
 }
 
 void AudioRtpFactory::initConfig()
@@ -86,7 +88,7 @@ void AudioRtpFactory::initSession()
     ost::MutexLock m(audioRtpThreadMutex_);
 
     if (srtpEnabled_) {
-        std::string zidFilename(Manager::instance().voipPreferences.getZidFile());
+        const std::string zidFilename(Manager::instance().voipPreferences.getZidFile());
 
         switch (keyExchangeProtocol_) {
 
@@ -114,8 +116,8 @@ void AudioRtpFactory::start(AudioCodec* audiocodec)
     if (rtpSession_ == NULL)
         throw AudioRtpFactoryException("AudioRtpFactory: Error: RTP session was null when trying to start audio thread");
 
-    if (keyExchangeProtocol_ == SDES and localContext_ and remoteContext_)
-        static_cast<AudioSrtpSession *>(rtpSession_)->restoreCryptoContext(localContext_, remoteContext_);
+    if (keyExchangeProtocol_ == SDES and cachedLocalContext_ and cachedRemoteContext_)
+        static_cast<AudioSrtpSession *>(rtpSession_)->restoreCryptoContext(cachedLocalContext_, cachedRemoteContext_);
 
     if (rtpSession_->startRtpThread(*audiocodec) != 0)
         throw AudioRtpFactoryException("AudioRtpFactory: Error: Failed to start AudioRtpSession thread");
@@ -125,14 +127,6 @@ void AudioRtpFactory::stop()
 {
     ost::MutexLock mutex(audioRtpThreadMutex_);
 
-    if (rtpSession_ == NULL)
-        return;
-
-    if (keyExchangeProtocol_ == SDES) {
-        localContext_ = static_cast<AudioSrtpSession*>(rtpSession_)->localCryptoCtx_;
-        remoteContext_ = static_cast<AudioSrtpSession*>(rtpSession_)->remoteCryptoCtx_;
-    }
-
     delete rtpSession_;
     rtpSession_ = NULL;
 }
@@ -170,17 +164,19 @@ sfl::AudioZrtpSession * AudioRtpFactory::getAudioZrtpSession()
 void sfl::AudioRtpFactory::initLocalCryptoInfo()
 {
     if (rtpSession_ && keyExchangeProtocol_ == SDES) {
-        AudioSrtpSession *srtpSession = static_cast<AudioSrtpSession*>(rtpSession_);
-        srtpSession->initLocalCryptoInfo();
-        ca_->getLocalSDP()->setLocalSdpCrypto(srtpSession->getLocalCryptoInfo());
+        AudioSrtpSession *srtp = static_cast<AudioSrtpSession*>(rtpSession_);
+        // the context is invalidated and deleted by the call to initLocalCryptoInfo
+        cachedLocalContext_ = srtp->initLocalCryptoInfo();
+        ca_->getLocalSDP()->setLocalSdpCrypto(srtp->getLocalCryptoInfo());
     }
 }
 
 void AudioRtpFactory::setRemoteCryptoInfo(sfl::SdesNegotiator& nego)
 {
-    if (rtpSession_ and keyExchangeProtocol_ == SDES)
-        static_cast<AudioSrtpSession *>(rtpSession_)->setRemoteCryptoInfo(nego);
-    else
+    if (rtpSession_ and keyExchangeProtocol_ == SDES) {
+        AudioSrtpSession *srtp = static_cast<AudioSrtpSession *>(rtpSession_);
+        cachedRemoteContext_ = srtp->setRemoteCryptoInfo(nego);
+    } else
         throw AudioRtpFactoryException("RTP: Error: rtpSession_ is NULL in setRemoteCryptoInfo");
 }
 

daemon/src/audio/audiortp/audio_rtp_factory.h

@@ -153,10 +153,10 @@ class AudioRtpFactory {
         bool helloHashEnabled_;
 
         /** Remote srtp crypto context to be set into incoming data queue. */
-        ost::CryptoContext *remoteContext_;
+        ost::CryptoContext *cachedRemoteContext_;
 
         /** Local srtp crypto context to be set into outgoing data queue. */
-        ost::CryptoContext *localContext_;
+        ost::CryptoContext *cachedLocalContext_;
 
         SIPCall *ca_;
         KeyExchangeProtocol keyExchangeProtocol_;

daemon/src/audio/audiortp/audio_srtp_session.cpp

@@ -131,13 +131,8 @@ AudioSrtpSession::AudioSrtpSession(SIPCall &call) :
     remoteOfferIsSet_(false)
 {}
 
-AudioSrtpSession::~AudioSrtpSession()
-{
-    delete localCryptoCtx_;
-    delete remoteCryptoCtx_;
-}
 
-void AudioSrtpSession::initLocalCryptoInfo()
+ost::CryptoContext* AudioSrtpSession::initLocalCryptoInfo()
 {
     DEBUG("AudioSrtp: Set cryptographic info for this rtp session");
 
@@ -150,6 +145,7 @@ void AudioSrtpSession::initLocalCryptoInfo()
     localCryptoCtx_->deriveSrtpKeys(0);
 
     setOutQueueCryptoContext(localCryptoCtx_);
+    return localCryptoCtx_;
 }
 
 std::vector<std::string> AudioSrtpSession::getLocalCryptoInfo()
@@ -184,7 +180,8 @@ std::vector<std::string> AudioSrtpSession::getLocalCryptoInfo()
     return crypto_vector;
 }
 
-void AudioSrtpSession::setRemoteCryptoInfo(sfl::SdesNegotiator& nego)
+ost::CryptoContext*
+AudioSrtpSession::setRemoteCryptoInfo(sfl::SdesNegotiator& nego)
 {
     if (not remoteOfferIsSet_) {
         DEBUG("%s", nego.getKeyInfo().c_str());
@@ -203,9 +200,9 @@ void AudioSrtpSession::setRemoteCryptoInfo(sfl::SdesNegotiator& nego)
         initializeRemoteCryptoContext();
         setInQueueCryptoContext(remoteCryptoCtx_);
 
-        // initLocalCryptoInfo();
         remoteOfferIsSet_ = true;
     }
+    return remoteCryptoCtx_;
 }
 
 void AudioSrtpSession::initializeLocalMasterKey()
@@ -266,23 +263,19 @@ void AudioSrtpSession::initializeRemoteCryptoContext()
 
     CryptoSuiteDefinition crypto = sfl::CryptoSuites[remoteCryptoSuite_];
 
-    if (remoteCryptoCtx_) {
     delete remoteCryptoCtx_;
-        remoteCryptoCtx_ = NULL;
-    }
-
     remoteCryptoCtx_ = new ost::CryptoContext(0x0,
                                               0,    // roc,
                                               0L,   // keydr,
-            SrtpEncryptionAESCM,             // encryption algo
-            SrtpAuthenticationSha1Hmac,      // authtication algo
+                                              SrtpEncryptionAESCM,
+                                              SrtpAuthenticationSha1Hmac,
                                               remoteMasterKey_,
                                               remoteMasterKeyLength_,
                                               remoteMasterSalt_,
                                               remoteMasterSaltLength_,
                                               crypto.encryptionKeyLength / 8,
                                               crypto.srtpAuthKeyLength / 8,
-            crypto.masterSaltLength / 8,                         // session salt len
+                                              crypto.masterSaltLength / 8,
                                               crypto.srtpAuthTagLength / 8);
 
 }
@@ -293,28 +286,31 @@ void AudioSrtpSession::initializeLocalCryptoContext()
 
     CryptoSuiteDefinition crypto = sfl::CryptoSuites[localCryptoSuite_];
 
-    if (localCryptoCtx_)
     delete localCryptoCtx_;
-
     localCryptoCtx_ = new ost::CryptoContext(OutgoingDataQueue::getLocalSSRC(),
                                              0,     // roc,
                                              0L,    // keydr,
-            SrtpEncryptionAESCM,             // encryption algo
-            SrtpAuthenticationSha1Hmac,      // authtication algo
+                                             SrtpEncryptionAESCM,
+                                             SrtpAuthenticationSha1Hmac,
                                              localMasterKey_,
                                              localMasterKeyLength_,
                                              localMasterSalt_,
                                              localMasterSaltLength_,
                                              crypto.encryptionKeyLength / 8,
                                              crypto.srtpAuthKeyLength / 8,
-            crypto.masterSaltLength / 8,                         // session salt len
+                                             crypto.masterSaltLength / 8,
                                              crypto.srtpAuthTagLength / 8);
 }
 
-void AudioSrtpSession::restoreCryptoContext(ost::CryptoContext *localContext, ost::CryptoContext *remoteContext)
+void AudioSrtpSession::restoreCryptoContext(ost::CryptoContext *localContext,
+                                            ost::CryptoContext *remoteContext)
 {
-    setInQueueCryptoContext(remoteContext);
-    setOutQueueCryptoContext(localContext);
+    delete remoteCryptoCtx_;
+    remoteCryptoCtx_ = remoteContext;
+    delete localCryptoCtx_;
+    localCryptoCtx_ = localContext;
+    setInQueueCryptoContext(remoteCryptoCtx_);
+    setOutQueueCryptoContext(localCryptoCtx_);
 }
 
 }

daemon/src/audio/audiortp/audio_srtp_session.h

@@ -73,7 +73,6 @@ class AudioSrtpSession : public AudioSymmetricRtpSession {
          * Constructor for this rtp session
          */
         AudioSrtpSession(SIPCall &call);
-        ~AudioSrtpSession();
 
         /**
          * Used to get sdp crypto header to be included in sdp session. This
@@ -84,15 +83,17 @@ class AudioSrtpSession : public AudioSymmetricRtpSession {
 
         /**
          * Set remote crypto header from incoming sdp offer
+         * @return The new remote crypto context, to be cached by the caller
          */
-        void setRemoteCryptoInfo(sfl::SdesNegotiator &nego);
+        ost::CryptoContext* setRemoteCryptoInfo(sfl::SdesNegotiator &nego);
 
         /**
          * Init local crypto context for outgoing data
          * this method must be called before sending first Invite request
          * with SDP offer.
+         * @return The new local crypto context, to be cached by the caller
          */
-        void initLocalCryptoInfo();
+        ost::CryptoContext* initLocalCryptoInfo();
 
         /**
          * Restore the cryptographic context. most likely useful to restore
@@ -100,6 +101,8 @@ class AudioSrtpSession : public AudioSymmetricRtpSession {
          */
         void restoreCryptoContext(ost::CryptoContext *, ost::CryptoContext *);
 
+    private:
+        NON_COPYABLE(AudioSrtpSession);
 
         /** Remote srtp crypto context to be set into incoming data queue. */
         ost::CryptoContext* remoteCryptoCtx_;
@@ -107,9 +110,6 @@ class AudioSrtpSession : public AudioSymmetricRtpSession {
         /** Local srtp crypto context to be set into outgoing data queue. */
         ost::CryptoContext* localCryptoCtx_;
 
-    private:
-        NON_COPYABLE(AudioSrtpSession);
-
         /**
          * Init local master key according to current crypto context
          * as defined in SdesNegotiator.h