)]}'
{
  "commit": "c1d9ac02514a138129872a036e3f8a1074dcb8bd",
  "tree": "0a3dab51852bfa5e04be4debc2bb7e4dc4652144",
  "parents": [
    "e1a860c3745c77cb83228dde1b73fa62eaf43930"
  ],
  "author": {
    "name": "David Benjamin",
    "email": "davidben@google.com",
    "time": "Wed May 22 13:56:56 2024 -0400"
  },
  "committer": {
    "name": "Boringssl LUCI CQ",
    "email": "boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com",
    "time": "Wed Jun 05 16:02:50 2024 +0000"
  },
  "message": "Make SSL_select_next_proto more robust to invalid calls.\n\nSSL_select_next_proto has some fairly complex preconditions:\n\n- The peer and supported list must be valid protocol lists\n- The supported list must not be empty. The peer list may be empty due\n  to one of NPN\u0027s edge cases.\n\nIn the context of how this function is meant to be used, these are\nreasonable preconditions. The caller should not serialize its own list\nwrong, and it makes no sense to try to negotiate a protocol when you\ndon\u0027t support any protocols. In particular, it complicates NPN\u0027s weird\n\"opportunistic\" protocol.\n\nHowever, the preconditions are unchecked and a bit subtle. Violating\nthem will result in memory errors. Bad syntax on the protocol lists is\nmostly not a concern (you should encode your own list correctly and the\nlibrary checks the peer\u0027s list), but the second rule is somewhat of a\nmess in practice:\n\nDespite having the same precondition in reality, OpenSSL did not\ndocument this. Their documentation implies things which are impossible\nwithout this precondition, but they forgot to actually write down the\nprecondition. There\u0027s an added complexity that OpenSSL never updated the\nparameter names to match the role reversal between ALPN and NPN.\n\nThere are thus a few cases where a buggy caller may pass an empty\n\"supported\" list.\n\n- An NPN client called SSL_select_next_proto despite not actually\n  supporting any NPN protocols.\n\n- An NPN client called SSL_select_next_proto, flipped the parameters,\n  and the server advertised no protocols.\n\n- An ALPN server called SSL_select_next_proto, passed its own list in as\n  the second parameter, despite not actually supporting any ALPN\n  protocols.\n\nIn these scenarios, the \"opportunistic\" protocol returned in the\nOPENSSL_NPN_NO_OVERLAP case will be out of bounds. If the caller\ndiscards it, this does not matter. If the caller returns it through the\nNPN or ALPN selection callback, they have a problem. ALPN servers are\nexpected to discard it, though some may be buggy. NPN clients may\nimplement either behavior.\n\nOlder versions of some callers have exhibited variations on the above\nmistakes, so empirically folks don\u0027t always get it right. OpenSSL\u0027s\nwrong documentation also does not help matters. Instead, have\nSSL_select_next_proto just check these preconditions. That is not a\nperformance-sensitive function and these preconditions are easy to\ncheck. While I\u0027m here, rewrite it with CBS so it is much more\nstraightforwardly correct.\n\nWhat to return when the preconditions fail is tricky, but we need to\noutput *some* protocol, so we output the empty protocol. This, per the\nprevious test and doc fixes, is actually fine in NPN, so one of the\nabove buggy callers is not retroactively made OK. But it is not fine in\nALPN, so we still need to document that callers need to avoid this\nstate. To that end, revamp the documentation a bit.\n\nThanks to Joe Birr-Pixton for reporting this!\n\nFixed: 735\nChange-Id: I4378a082385e7334e6abaa6705e6b15d6843f6c5\nReviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69028\nReviewed-by: Bob Beck \u003cbbe@google.com\u003e\nCommit-Queue: David Benjamin \u003cdavidben@google.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "6a92a288893f521efdb4489ff2f40c12e55c9477",
      "old_mode": 33188,
      "old_path": "include/openssl/ssl.h",
      "new_id": "63b66b41e919608a4c52bad83632655917fd34c4",
      "new_mode": 33188,
      "new_path": "include/openssl/ssl.h"
    },
    {
      "type": "modify",
      "old_id": "20a5d3034e77e06d9b5e12297fc346bd7baa4ce9",
      "old_mode": 33188,
      "old_path": "ssl/extensions.cc",
      "new_id": "8b2de598f29d793de83d5786fa528ff40cda9507",
      "new_mode": 33188,
      "new_path": "ssl/extensions.cc"
    },
    {
      "type": "modify",
      "old_id": "a34033531d0d17fbaa50d4f5ffdd862a9740c92e",
      "old_mode": 33188,
      "old_path": "ssl/internal.h",
      "new_id": "5744dfef4419b16c664e41226facc92a82d866a3",
      "new_mode": 33188,
      "new_path": "ssl/internal.h"
    },
    {
      "type": "modify",
      "old_id": "ec0ee893e58d49890f5545d02b533d07e362309c",
      "old_mode": 33188,
      "old_path": "ssl/ssl_lib.cc",
      "new_id": "278c7a9b48756b81246be6af66e0cdea0a9a2c9a",
      "new_mode": 33188,
      "new_path": "ssl/ssl_lib.cc"
    },
    {
      "type": "modify",
      "old_id": "3cb4998659c4fcd6d3c418ca366e19af1d058bc9",
      "old_mode": 33188,
      "old_path": "ssl/ssl_test.cc",
      "new_id": "c3442773b72c0a8e87912dda461e83b5e49e74ba",
      "new_mode": 33188,
      "new_path": "ssl/ssl_test.cc"
    }
  ]
}