Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

verify_fun/3 is not called for intermediate certificates #4682

Closed
omh opened this issue Mar 27, 2021 · 1 comment
Closed

verify_fun/3 is not called for intermediate certificates #4682

omh opened this issue Mar 27, 2021 · 1 comment
Assignees
Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS

Comments

@omh
Copy link
Contributor

omh commented Mar 27, 2021

Describe the bug
verify_fun/3 is not called for intermediate certificates.

To Reproduce
test.exs:

defmodule A do
  def run do

    opts = [
      verify: :verify_none,
      verify_fun: {&verify_fun/3, nil},
      reuse_sessions: false
    ]

    :ssl.connect('google.com', 443, opts)
  end

  defp verify_fun(_cert, event, state) do
    IO.inspect(event, label: "event")
    {:valid, state}
  end
end

Application.ensure_all_started(:ssl)
A.run()
$ elixir -v        
Erlang/OTP 23 [erts-11.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [dtrace]

Elixir 1.11.4 (compiled with Erlang/OTP 23)

$ elixir test.exs 
event: {:bad_cert, :unknown_ca}.  # <- this is the peer cert
result: {:ok,
 {:sslsocket, {:gen_tcp, #Port<0.5>, :tls_connection, :undefined},
  [#PID<0.129.0>, #PID<0.128.0>]}}

Expected behavior
Expected verify_fun/3 to be run for both peer and intermediate certificate as it does on previous versions:

$ elixir -v
Erlang/OTP 23 [erts-11.0.2] [source] [64-bit] [smp:1:1] [ds:1:1:10] [async-threads:1]

Elixir 1.10.3 (compiled with Erlang/OTP 21)

$ elixir test.exs 
event: {:bad_cert, :unknown_ca}.  # <- this is the intermediate cert
event: {:extension,
 {:Extension, {2, 5, 29, 14}, false,
  <<39, 20, 194, 251, 23, 187, 0, 246, 84, 89, 181, 195, 240, 134, 189, 162, 85,
    123, 30, 72>>}}
event: {:extension,
 {:Extension, {2, 5, 29, 35}, false,
  {:AuthorityKeyIdentifier,
   <<152, 209, 248, 110, 16, 235, 207, 155, 236, 96, 159, 24, 144, 27, 160, 235,
     125, 9, 253, 43>>, :asn1_NOVALUE, :asn1_NOVALUE}}}
event: {:extension,
 {:Extension, {1, 3, 6, 1, 5, 5, 7, 1, 1}, false,
  [
    {:AccessDescription, {1, 3, 6, 1, 5, 5, 7, 48, 1},
     {:uniformResourceIdentifier, 'http://ocsp.pki.goog/gts1o1core'}},
    {:AccessDescription, {1, 3, 6, 1, 5, 5, 7, 48, 2},
     {:uniformResourceIdentifier, 'http://pki.goog/gsr2/GTS1O1.crt'}}
  ]}}
event: {:extension,
 {:Extension, {2, 5, 29, 17}, false,
  [
    dNSName: '*.google.com',
    dNSName: '*.android.com',
    dNSName: '*.appengine.google.com',
 ... snip ...
    dNSName: '*.gvt1.com',
    dNSName: '*.gvt2.com',
    ...
  ]}}
event: {:extension,
 {:Extension, {2, 5, 29, 32}, false,
  [
    {:PolicyInformation, {2, 23, 140, 1, 2, 2}, :asn1_NOVALUE},
    {:PolicyInformation, {1, 3, 6, 1, 4, 1, 11129, 2, 5, 3}, :asn1_NOVALUE}
  ]}}
event: {:extension,
 {:Extension, {2, 5, 29, 31}, false,
  [
    {:DistributionPoint,
     {:fullName,
      [uniformResourceIdentifier: 'http://crl.pki.goog/GTS1O1core.crl']},
     :asn1_NOVALUE, :asn1_NOVALUE}
  ]}}
event: {:extension,
 {:Extension, {1, 3, 6, 1, 4, 1, 11129, 2, 4, 2}, false,
  <<4, 129, 242, 0, 240, 0, 118, 0, 246, 92, 148, 47, 209, 119, 48, 34, 20, 84,
    24, 8, 48, 148, 86, 142, 227, 77, 19, 25, 51, 191, 223, 12, 47, 32, 11, 204,
    78, 241, 100, 227, 0, 0, 1, 120, ...>>}}
event: :valid_peer.  # <- this is the peer/leaf cert

Affected versions
23.3 (works on at least 23.0.2).

Additional context
Use case: trying to retrieve all certs received from server no matter the validity.

@omh omh added the bug Issue is reported as a bug label Mar 27, 2021
@IngelaAndin IngelaAndin added the team:PS Assigned to OTP team PS label Mar 29, 2021
@IngelaAndin IngelaAndin self-assigned this Mar 30, 2021
@IngelaAndin
Copy link
Contributor

IngelaAndin commented Mar 30, 2021

The behaviour seems to have changed in OTP-23.2. I am not sure why yet, this is not an intentional change. Although specifying a verify_fun when not verifying is semantically strange and maybe we should not allow it as verify_none is implemented as av verify_fun that accepts all verification errors. I will keep digging!

IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Apr 1, 2021
When trying to reconstruct an incomplete chain we must not
use a constructed chain that is a shorter incomplete chain than
the original chain. This might happen if we have a chain that we
will not be able to reconstruct and hence verify. Even though this
is not a problem from a verification point (will fail with unknown_ca anyway)
it could cause different behavior for a verify_fun.

Closes erlang#4682
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Apr 1, 2021
When trying to reconstruct an incomplete chain we must not
use a constructed chain that is a shorter incomplete chain than
the original chain. This might happen if we have a chain that we
will not be able to reconstruct and hence verify. Even though this
is not a problem from a verification point (will fail with unknown_ca anyway)
it could cause different behavior for a verify_fun.

Closes erlang#4682
IngelaAndin added a commit that referenced this issue Apr 3, 2021
…iour/GH-4682/OTP-17296

ssl: Make sure incomplete chain is a prefix of the new chain candidate
rickard-green pushed a commit that referenced this issue Apr 27, 2021
… maint-23

* ingela/ssl/verify-fun-behaviour/GH-4682/OTP-17296:
  ssl: Make sure incomplete chain is a prefix of the new chain candidate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

2 participants