Subject: [Perc] Alexey Melnikov's Discuss on draft-ietf-perc-srtp-ekt-diet-09: (with DISCUSS and COMMENT)

From: Alexey Melnikov <[email-masked]>
Date: Wed, 20 Feb 2019 05:25:37 -0800

Alexey Melnikov has entered the following ballot position for
draft-ietf-perc-srtp-ekt-diet-09: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-perc-srtp-ekt-diet/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

The document is generally quite readable, which is great. But I have a few
small issues I would like to get clarification on before recommending approval
of this document:

In 4.1:

   Message Type: The last byte is used to indicate the type of the
   EKTField.  This MUST be 2 for the FullEKTField format and 0 in
   ShortEKTField format.  Values less than 64 are mandatory to
   understand while other values are optional to understand.

I thought I knew what this meant when I read it, and then I saw this:

   A receiver
   SHOULD discard the whole EKTField if it contains any message type
   value that is less than 64 and that is not understood.

"SHOULD discard ... EKTField" makes this field NOT mandatory. (If you said
"SHOULD discard the whole packet", that would have been different.) Also, how
"discard" different from the following sentence suggesting "ignore"? I think
you have some inconsistencies/terminology problem here!

   Message type
   values that are 64 or greater but not implemented or understood can
   simply be ignored.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I share Benjamin's concern about extensibility.

General:

Mixture of normative TLS 1.2 and TLS 1.3 references is confusing in the
document. There is a note later on explaining that either can be used, but it
would be better for readers to see this note earlier in the document.

In 4.1:

  EKTMsgLength = 2BYTE;

and similarly:

   SPI = 2BYTE

The document doesn't seem to say whether or not this is in network byte order.

In 4.2.1:

   2.  The EKTPlaintext field is computed from the SRTP Master Key,
       SSRC, and ROC fields, as shown in Section 4.1.  The ROC, SRTP
       Master Key, and SSRC used in EKT processing SHOULD be the same as
       the one used in the SRTP processing.

When can they be different? I.e. why is this not a MUST?

   The computed value of the FullEKTField is written into the SRTP
   packet.

I think this might be misleading. Do you just mean appended to the end of the
SRTP packet after encrypted data? If yes, please say so to avoid confusion with
writing it into encrypted data before encryption.

In 4.3:

   When receiving a new EKTKey, implementations need to use the ekt_ttl
   field (see Section 5.2.2) to create a time after which this key
   cannot be used and they also need to create a counter that keeps
   track of how many times the key has been used to encrypt data to
   ensure it does not exceed the T value for that cipher (see ).

Missing reference after "see" here.

In 4.4.1:

   The default EKT Cipher is the Advanced Encryption Standard (AES) Key
   Wrap with Padding [RFC5649] algorithm.  It requires a plaintext
   length M that is at least one octet, and it returns a ciphertext with
   a length of N = M + (M mod 8) + 8 octets.

I started looking at RFC 5649. Maybe I was tired and my math was wrong, but I
couldn't figure out how you came up with the N value above. In particular,
where is the "+ 8" coming from?

In 4.7:

   New sender:
      A new sender SHOULD send a packet containing the FullEKTField as
      soon as possible, always before or coincident with sending its
      initial SRTP packet.  To accommodate packet loss, it is
      RECOMMENDED that three consecutive packets contain the
      FullEKTField be transmitted.  If the sender does not send a
      FullEKTField in its initial packets and receivers have not
      otherwise been provisioned with a decryption key, then decryption
      will fail and SRTP packets will be dropped until the the receives

Nit: duplicated "the".

      a FullEKTField from the sender.

In 6:

   An attacker who tampers with the bits in FullEKTField can prevent the
   intended receiver of that packet from being able to decrypt it.  This
   is a minor denial of service vulnerability.  Similarly the attacker
   could take an old FullEKTField from the same session and attach it to
   the packet.  The FullEKTField would correctly decode and pass
   integrity checks.  However, the key extracted from the FullEKTField ,
   when used to decrypt the SRTP payload, would be wrong and the SRTP
   integrity check would fail.  Note that the FullEKTField only changes
   the decryption key and does not change the encryption key.  None of
   these are considered significant attacks as any attacker that can
   modify the packets in transit and cause the integrity check to fail.

The last sentence seems to be incomplete. Did you mean "can" instead of the
last "end"?