session->minor_ver is never populated

[zhihan]

It seems to me that session->minor_ver is never populated. When we disable TLS 1.2 and below, I am hitting this error.

[zhihan]

I think I should add a line below this statement , but the comment looks a little concerning.

[zhihan]

I think the long term solution is to parse the session header and get the version fields from the headers. If omitting headers, it is assumed that the serialized data matches the default versions. Let me know if my understanding is wrong.

[hanno-becker]

I think the long term solution is to parse the session header and get the version fields from the headers. If omitting headers, it is assumed that the serialized data matches the default versions. Let me know if my understanding is wrong.

That's right @zhihan - we need to add a version specifier to serialized session data.

[hanno-becker]

I think when supporting 1.2 and 1.3 alongside each other, we require a serialized session format along the following lines:

/*
 * Serialize a session in the following format:
 * (in the presentation language of TLS, RFC 8446 section 3)
 *
 *  struct {
 *
 *    opaque mbedtls_version[3];   // major, minor, patch
 *    opaque session_format[2];    // version-specific 16-bit field determining
 *                                 // the format of the remaining
 *                                 // serialized data.
 *  
 *          Note: When updating the format, remember to keep
 *          these version+format bytes.
 *  
 *                                 // In this version, `session_format` determines
 *                                 // the setting of those compile-time
 *                                 // configuration options which influence
 *                                 // the structure of mbedtls_ssl_session.
 *  
 *    uint8_t minor_ver;           // Possible values:
 *                                 // - TLS 1.2 (MBEDTLS_SSL_MINOR_VERSION_3) 
 *                                 // - TLS 1.3 (MBEDTLS_SSL_MINOR_VERSION_4) 
 *   
 *    select (serialized_session.minor_ver) {
 *
 *      case MBEDTLS_SSL_MINOR_VERSION_3: // TLS 1.2
 *        uint64 start_time;
 *        uint8 ciphersuite[2];           // defined by the standard
 *        uint8 compression;              // 0 or 1
 *        uint8 session_id_len;           // at most 32
 *        opaque session_id[32];
 *        opaque master[48];              // fixed length in the standard
 *        uint32 verify_result;
 *        opaque peer_cert<0..2^24-1>;    // length 0 means no peer cert
 *        opaque ticket<0..2^24-1>;       // length 0 means no ticket
 *        uint32 ticket_lifetime;
 *        uint8 mfl_code;                 // up to 255 according to standard
 *        uint8 trunc_hmac;               // 0 or 1
 *        uint8 encrypt_then_mac;         // 0 or 1
 *
 *      case MBEDTLS_SSL_MINOR_VERSION_4: // TLS 1.3
 *        uint64 start_time;
 *        uint8 ciphersuite[2];
 *        uint32 ticket_lifetime;
 *        uint32 ticket_age_add;
 *        uint8 ticket_flags;
 *        opaque resumption_key<0..255>;
 *        opaque ticket<0..2^16-1>;
 *
 *   };
 *  
 * } serialized_session;
 * 
 */

[hanno-becker]

I created Mbed-TLS#4802 to track the necessary preparation on upstream Mbed TLS.

[hanno-becker]

ARMmbed#4802 has been merged. We can close this after the next merge of development into tls13-prototype