Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Security queries are not implemented to spec #2024

Closed
jacobkeeler opened this issue Jul 15, 2021 · 1 comment
Closed

Security queries are not implemented to spec #2024

jacobkeeler opened this issue Jul 15, 2021 · 1 comment
Labels
bug A defect in the library protocol Relating to the protocol layer

Comments

@jacobkeeler
Copy link

jacobkeeler commented Jul 15, 2021

Bug Report

With the acceptance of SDL 0317, this library's implementation of security queries does not match the spec fully. Several issues exist with the current implementation such as:

  1. When a security query is received from Core, its header is entirely ignored and the library assumes that it is a Send Handshake Data request. This causes issues if Core were to send another type of query, such as a Send Internal Error notification.
    // Tear off the binary header of the client protocol message to get at the actual TLS handshake
    // TODO: (Joel F.)[2016-02-15] Should check for errors
    NSData *clientHandshakeData = [clientHandshakeMessage.payload subdataWithRange:NSMakeRange(12, (clientHandshakeMessage.payload.length - 12))];
  1. Hardcoded values are used when sending security queries for fields such as query type and query ID. Any predefined values should be properly added as constants.
    serverTLSPayload.functionID = 0x01; // TLS Handshake message
    serverTLSPayload.rpcType = 0x00;
    serverTLSPayload.correlationID = 0x00;

should be changed to something like

    serverTLSPayload.queryID = SDLQueryIDSendHandshakeData;
    serverTLSPayload.queryType = SDLQueryTypeResponse;
    serverTLSPayload.sequenceNumber = requestHeader.sequenceNumber;
  1. Security queries are constructed using an RPC header, which has a slightly different format than a security query header. Security query headers should be implemented separate from RPC headers.
    // For a control service packet, we need a binary header with a function ID corresponding to what type of packet we're sending.
    SDLRPCPayload *serverTLSPayload = [[SDLRPCPayload alloc] init];
    ...
    NSData *binaryData = serverTLSPayload.data;

    return [SDLProtocolMessage messageWithHeader:serverMessageHeader andPayload:binaryData];

should be changed to something like

    // For a control service packet, we need a binary header with a function ID corresponding to what type of packet we're sending.
    SDLQueryPayload *serverTLSPayload = [[SDLQueryPayload alloc] init];
    ...
    NSData *binaryData = serverTLSPayload.data;

    return [SDLProtocolMessage messageWithHeader:serverMessageHeader andPayload:binaryData];
OS & Version Information
  • iOS Version: N/A
  • SDL iOS Version: master
  • Testing Against: N/A
@theresalech
Copy link
Contributor

See Core issue smartdevicelink/sdl_core#3755; the mobile libraries will need to account for this by accepting both the NOTIFICATION and REQUEST types for SendHandshakeData and just treating them as REQUEST type.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug A defect in the library protocol Relating to the protocol layer
Projects
None yet
Development

No branches or pull requests

3 participants