crypto/boringssl: do dummy seal on key update too

Follow-up to b17904e.

Currently the dummy seal operation done to prime the AEAD context is
done in `Seal::from_secret()`, but really it needs to be done as part of
`PacketKey::from_secret()` to make sure every code path is covered,
including the key update one which doesn't call `Seal::from_secret()`.

It's not entirely clear why, but the problem (i.e. seal operation fail
when trying to encrypt a packet) seems to only appear after several
packets have already been encrypted with the new key, which is why this
wasn't caught by the existing key update test.

To cover that, the test is expanded by making the server send additional
packets which from local testing seems to trigger the issue.

quiche/src/crypto/boringssl.rs

@@ -65,7 +65,17 @@ impl PacketKey {
         derive_pkt_key(aead, secret, &mut key)?;
         derive_pkt_iv(aead, secret, &mut iv)?;
 
-        Self::new(aead, key, iv, enc)
+        let pkt_key = Self::new(aead, key, iv, enc)?;
+
+        // Dummy seal operation to prime the AEAD context with the nonce mask.
+        //
+        // This is needed because BoringCrypto requires the first counter (i.e.
+        // packet number) to be zero, which would not be the case for packet
+        // number spaces after Initial as the same packet number sequence is
+        // shared.
+        let _ = pkt_key.seal_with_u64_counter(0, b"", &mut [0_u8; 16], 0, None);
+
+        Ok(pkt_key)
     }
 
     pub fn open_with_u64_counter(

quiche/src/crypto/mod.rs

@@ -235,25 +235,15 @@ impl Seal {
     }
 
     pub fn from_secret(aead: Algorithm, secret: &[u8]) -> Result<Seal> {
-        let seal = Seal {
+        Ok(Seal {
             alg: aead,
 
             secret: secret.to_vec(),
 
             header: HeaderProtectionKey::from_secret(aead, secret)?,
 
             packet: PacketKey::from_secret(aead, secret, Self::ENCRYPT)?,
-        };
-
-        // Dummy seal operation to prime the AEAD context with the nonce mask.
-        //
-        // This is needed because BoringCrypto requires the first counter (i.e.
-        // packet number) to be zero, which would not be the case for packet
-        // number spaces after Initial as the same packet number sequence is
-        // shared.
-        let _ = seal.seal_with_u64_counter(0, b"", &mut [0_u8; 16], 0, None);
-
-        Ok(seal)
+        })
     }
 
     pub fn new_mask(&self, sample: &[u8]) -> Result<[u8; 5]> {

quiche/src/lib.rs

@@ -10246,15 +10246,27 @@ mod tests {
         );
 
         // Server sends message with the new key.
-        assert_eq!(pipe.server.stream_send(4, b"world", true), Ok(5));
+        assert_eq!(pipe.server.stream_send(4, b"world", false), Ok(5));
         assert_eq!(pipe.advance(), Ok(()));
 
         // Ensure update key is completed and client can decrypt packet.
         let mut r = pipe.client.readable();
         assert_eq!(r.next(), Some(4));
         assert_eq!(r.next(), None);
-        assert_eq!(pipe.client.stream_recv(4, &mut b), Ok((5, true)));
+        assert_eq!(pipe.client.stream_recv(4, &mut b), Ok((5, false)));
         assert_eq!(&b[..5], b"world");
+
+        // Server keeps sending packets to ensure encryption still works.
+        for _ in 0..10 {
+            assert_eq!(pipe.server.stream_send(4, b"world", false), Ok(5));
+            assert_eq!(pipe.advance(), Ok(()));
+
+            let mut r = pipe.client.readable();
+            assert_eq!(r.next(), Some(4));
+            assert_eq!(r.next(), None);
+            assert_eq!(pipe.client.stream_recv(4, &mut b), Ok((5, false)));
+            assert_eq!(&b[..5], b"world");
+        }
     }
 
     #[test]