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

[Richard Barnes <[e-mail]>]

Hey Alexey,

Good catch.  I agree that those bits are malformed.  Unless folks here
object, I will change those portions to say that the receiver MUST discard
the packet if it receives an unknown message type.  TBH, I don't really see
how we can do otherwise, unless we include a length field that is required
for all message types (or maybe all message types except type 0).

--RLB

On Wed, Feb 20, 2019 at 8:25 AM Alexey Melnikov <[e-mail]>
wrote:

> 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"?
>
>
> _______________________________________________
> Perc mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/perc
>