Safe guard for empty alpn.

Per ~~~-5535: it is not safe to call SSL_select_next_proto()
with invalid alpn_in.

Although our code is safe because it is only used via set_alpn_select_callback
which performs the check, an additional guard is added just in case these functions
are used in other context.

2 files changed, 18 insertions, 1 deletions

diff --git a/.bleep b/.bleep
index 79df49e..321c159 100644
--- a/.bleep
+++ b/.bleep
@@ -1 +1 @@
-5a6061f33868fbf1f246f048941964592bc73d85
\ No newline at end of file
+7e74e88f3e50bfdb38eeea244f3100d8477db7e4
\ No newline at end of file
diff --git a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs
index 55a7139..3780a94 100644
--- a/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs
+++ b/pingora-core/src/listeners/tls/boringssl_openssl/mod.rs
@@ -138,9 +138,20 @@ mod alpn {
     use super::*;
     use crate::tls::ssl::{select_next_proto, AlpnError, SslRef};
 
+    fn valid_alpn(alpn_in: &[u8]) -> bool {
+        if alpn_in.is_empty() {
+            return false;
+        }
+        // TODO: can add more thorough validation here.
+        true
+    }
+
     // A standard implementation provided by the SSL lib is used below
 
     pub fn prefer_h2<'a>(_ssl: &mut SslRef, alpn_in: &'a [u8]) -> Result<&'a [u8], AlpnError> {
+        if !valid_alpn(alpn_in) {
+            return Err(AlpnError::NOACK);
+        }
         match select_next_proto(ALPN::H2H1.to_wire_preference(), alpn_in) {
             Some(p) => Ok(p),
             _ => Err(AlpnError::NOACK), // unknown ALPN, just ignore it. Most clients will fallback to h1
@@ -148,6 +159,9 @@ mod alpn {
     }
 
     pub fn h1_only<'a>(_ssl: &mut SslRef, alpn_in: &'a [u8]) -> Result<&'a [u8], AlpnError> {
+        if !valid_alpn(alpn_in) {
+            return Err(AlpnError::NOACK);
+        }
         match select_next_proto(ALPN::H1.to_wire_preference(), alpn_in) {
             Some(p) => Ok(p),
             _ => Err(AlpnError::NOACK), // unknown ALPN, just ignore it. Most clients will fallback to h1
@@ -155,6 +169,9 @@ mod alpn {
     }
 
     pub fn h2_only<'a>(_ssl: &mut SslRef, alpn_in: &'a [u8]) -> Result<&'a [u8], AlpnError> {
+        if !valid_alpn(alpn_in) {
+            return Err(AlpnError::ALERT_FATAL);
+        }
         match select_next_proto(ALPN::H2.to_wire_preference(), alpn_in) {
             Some(p) => Ok(p),
             _ => Err(AlpnError::ALERT_FATAL), // cannot agree