[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 2 commits: raop: add error handling to rsa_encrypt()

[Tanu Kaskinen]

Tanu Kaskinen pushed to branch master at PulseAudio / pulseaudio


Commits:
2ee5dfff by Tanu Kaskinen at 2018-11-20T08:17:17Z
raop: add error handling to rsa_encrypt()

When reviewing another change in rsa_encrypt(), Felipe Sateler pointed
out some deficiencies in error handling. This patch adds error handling
for all openssl calls in rsa_encrypt().

This patch doesn't propagate the error all the way up to the
pa_rtsp_client owner, because there's no mechanism for doing that. I
could implement such mechanism myself, but I think it's better I don't
make such complex changes to the RAOP code, because I don't have any
RAOP hardware to test the changes. The result is that module-raop-sink
will just sit around without doing anything. I think this is still
better than having no error handling at all.

- - - - -
f2199f87 by Tanu Kaskinen at 2018-11-20T08:26:11Z
raop: fix typo: modules -> modulus

- - - - -


2 changed files:

- src/modules/raop/raop-client.c
- src/modules/raop/raop-crypto.c


Changes:

=====================================
src/modules/raop/raop-client.c
=====================================
@@ -906,6 +906,15 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp, pa_rtsp_state_t state, pa_rtsp_
                 case PA_RAOP_ENCRYPTION_MFISAP:
                 case PA_RAOP_ENCRYPTION_FAIRPLAY_SAP25: {
                     key = pa_raop_secret_get_key(c->secret);
+                    if (!key) {
+                        pa_log("pa_raop_secret_get_key() failed.");
+                        pa_rtsp_disconnect(rtsp);
+                        /* FIXME: This is an unrecoverable failure. We should notify
+                         * the pa_raop_client owner so that it could shut itself
+                         * down. */
+                        goto connect_finish;
+                    }
+
                     iv = pa_raop_secret_get_iv(c->secret);
 
                     sdp = pa_sprintf_malloc(
@@ -929,6 +938,7 @@ static void rtsp_stream_cb(pa_rtsp_client *rtsp, pa_rtsp_state_t state, pa_rtsp_
 
             pa_rtsp_announce(c->rtsp, sdp);
 
+connect_finish:
             pa_xfree(sdp);
             pa_xfree(url);
             break;


=====================================
src/modules/raop/raop-crypto.c
=====================================
@@ -74,27 +74,71 @@ static const char rsa_exponent[] =
     "AQAB";
 
 static int rsa_encrypt(uint8_t *data, int len, uint8_t *str) {
-    uint8_t modules[256];
+    uint8_t modulus[256];
     uint8_t exponent[8];
     int size;
     RSA *rsa;
-    BIGNUM *n_bn;
-    BIGNUM *e_bn;
+    BIGNUM *n_bn = NULL;
+    BIGNUM *e_bn = NULL;
+    int r;
 
     pa_assert(data);
     pa_assert(str);
 
     rsa = RSA_new();
-    size = pa_raop_base64_decode(rsa_modulus, modules);
-    n_bn = BN_bin2bn(modules, size, NULL);
+    if (!rsa) {
+        pa_log("RSA_new() failed.");
+        goto fail;
+    }
+
+    size = pa_raop_base64_decode(rsa_modulus, modulus);
+
+    n_bn = BN_bin2bn(modulus, size, NULL);
+    if (!n_bn) {
+        pa_log("n_bn = BN_bin2bn() failed.");
+        goto fail;
+    }
+
     size = pa_raop_base64_decode(rsa_exponent, exponent);
+
     e_bn = BN_bin2bn(exponent, size, NULL);
-    RSA_set0_key(rsa, n_bn, e_bn, NULL);
+    if (!e_bn) {
+        pa_log("e_bn = BN_bin2bn() failed.");
+        goto fail;
+    }
+
+    r = RSA_set0_key(rsa, n_bn, e_bn, NULL);
+    if (r == 0) {
+        pa_log("RSA_set0_key() failed.");
+        goto fail;
+    }
+
+    /* The memory allocated for n_bn and e_bn is now managed by the RSA object.
+     * Let's set n_bn and e_bn to NULL to avoid freeing the memory in the error
+     * handling code. */
+    n_bn = NULL;
+    e_bn = NULL;
 
     size = RSA_public_encrypt(len, data, str, rsa, RSA_PKCS1_OAEP_PADDING);
+    if (size == -1) {
+        pa_log("RSA_public_encrypt() failed.");
+        goto fail;
+    }
 
     RSA_free(rsa);
     return size;
+
+fail:
+    if (e_bn)
+        BN_free(e_bn);
+
+    if (n_bn)
+        BN_free(n_bn);
+
+    if (rsa)
+        RSA_free(rsa);
+
+    return -1;
 }
 
 pa_raop_secret* pa_raop_secret_new(void) {
@@ -134,6 +178,11 @@ char* pa_raop_secret_get_key(pa_raop_secret *s) {
 
     /* Encrypt our AES public key to send to the device */
     size = rsa_encrypt(s->key, AES_CHUNK_SIZE, rsa_key);
+    if (size < 0) {
+        pa_log("rsa_encrypt() failed.");
+        return NULL;
+    }
+
     pa_raop_base64_encode(rsa_key, size, &base64_key);
 
     return base64_key;



View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/compare/fe6a9a8f59932f29cc77eac2a7e2c6bd07c8c7d0...f2199f879a9bd7aadcc76b3900b7557324299a41

-- 
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/compare/fe6a9a8f59932f29cc77eac2a7e2c6bd07c8c7d0...f2199f879a9bd7aadcc76b3900b7557324299a41
You're receiving this email because of your account on gitlab.freedesktop.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20181120/0b269e2a/attachment-0001.html>