Reject SRP6 values which are negative or larger than p since these are

likely totally bogus.

2 files changed, 25 insertions, 15 deletions

diff --git a/doc/relnotes/1_11_1.rst b/doc/relnotes/1_11_1.rst
index 518f4702a..8300c4a17 100644
--- a/doc/relnotes/1_11_1.rst
+++ b/doc/relnotes/1_11_1.rst
@@ -1,7 +1,7 @@
 Version 1.11.1, Not Yet Released
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-TLS and DTLS
+TLS Enhancements and Bug Fixes
 """"""""""""""""""""""""""""""""""""""""
 
 ..
@@ -18,24 +18,25 @@ secret, whereas now only 32 bytes are needed total. This change may
 also make it slightly harder for an attacker to extract session data
 from memory dumps (eg with a cold boot attack).
 
-TLS clients were not sending a next protocol message during a session
-resumption, which would cause resumption failures with servers that
-support NPN if NPN was being offered by the client.
+The keys used for session encryption were previously uniquely
+determined by the master key. Now the encrypted session blob includes
+two 80 bit salts which are used in the derivation of the cipher and
+MAC keys.
 
 New policy hooks :cpp:func:`TLS::Policy::acceptable_protocol_version`
 and :cpp:func:`TLS::Policy::allow_server_initiated_renegotiation` were
 added.
 
-The keys used for session encryption were previously uniquely
-determined by the master key. Now the encrypted session blob includes
-two 80 bit salts which are used in the derivation of the cipher and
-MAC keys.
+TLS clients were not sending a next protocol message during a session
+resumption, which would cause resumption failures with servers that
+support NPN if NPN was being offered by the client.
 
-A heartbeat request send by the counterparty during a handshake would
-be passed to the application callback as a heartbeat response.
+A bug caused heartbeat requests sent by the counterparty during a
+handshake to be passed to the application callback as if they were
+heartbeat responses.
 
-Public Key Strength Checking
-""""""""""""""""""""""""""""""""""""""""
+New Feature: Public Key Strength Checking
+""""""""""""""""""""""""""""""""""""""""""
 
 A new function :cpp:func:`Public_Key::estimated_strength` returns
 an estimate for the upper bound of the strength of the key. For
@@ -48,8 +49,17 @@ below 80 bits of strength (as estimated by ``estimated_strength``) are
 rejected. This level can be modified using a parameter to the
 :cpp:class:`Path_Validation_Restrictions` constructor.
 
+SRP6 Is Picker About Values
+""""""""""""""""""""""""""""""""""""""""
+
+The SRP6 code was checking for invalid values as specified in RFC
+5054, ones equal to zero mod p, however it would accept negative A/B
+values, or ones larger than p, neither of which should occur in a
+normal run of the protocol. These values are now rejected. Credits
+to Timothy Prepscius for pointing out these values are not normally
+used and probably signal something fishy.
 
-BigInt Modifications
+Removal of Various BigInt Functions
 """"""""""""""""""""""""""""""""""""""""
 
 Several :cpp:class:`BigInt` functions have been removed, including
diff --git a/src/constructs/srp6/srp6.cpp b/src/constructs/srp6/srp6.cpp
index f1927b648..7bc879350 100644
--- a/src/constructs/srp6/srp6.cpp
+++ b/src/constructs/srp6/srp6.cpp
@@ -91,7 +91,7 @@ srp6_client_agree(const std::string& identifier,
 
    const size_t p_bytes = group.get_p().bytes();
 
-   if(B % p == 0)
+   if(B <= 0 || B >= p)
       throw std::runtime_error("Invalid SRP parameter from server");
 
    BigInt k = hash_seq(hash_id, p_bytes, p, g);
@@ -150,7 +150,7 @@ BigInt SRP6_Server_Session::step1(const BigInt& v,
 
 SymmetricKey SRP6_Server_Session::step2(const BigInt& A)
    {
-   if(A % p == 0)
+   if(A <= 0 || A >= p)
       throw std::runtime_error("Invalid SRP parameter from client");
 
    BigInt u = hash_seq(hash_id, p_bytes, A, B);