Opened 10 years ago

Closed 10 years ago

#1976 defect closed fixed (fixed)

twisted.internet.ssl.Certificate.peerFromTransport can return invalid Certificates

Reported by: exarkun Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description

If there is no peer certificate, it creates a Certificate wrapped around None.

It should fail some other, better way.

Change History (8)

comment:1 Changed 10 years ago by glyph

It seems like there are 3 choices: return None, return a Deferred which fires when the cert is available, or raise an exception.

Right now I think I favor raising an exception.

comment:2 Changed 10 years ago by exarkun

I like exceptions.

comment:3 Changed 10 years ago by glyph

Ready for review in certerr-1976.

comment:4 Changed 10 years ago by glyph

  • Keywords review added

comment:5 Changed 10 years ago by glyph

  • Priority changed from high to highest

comment:6 Changed 10 years ago by jerub

  • Keywords review removed

Sweet change. Nice and simple, correct docstrings that exist, etc.

I'm confused about the lack of an actual error message to go in twisted.internet.error.CertificateError. It seems all the python code I've ever written has had both an exception, and some kind of information about the exception that occurred. Here, CertificateError() is created without any message. This causes str(CertificateError()) to equal . I don't think I like that.

Please alter the implementation or usage of CertificateError so that str() isn't dangerously low on information.

comment:7 Changed 10 years ago by glyph

  • Resolution set to fixed
  • Status changed from new to closed

(In [17837]) ssl.Certificate.peerFromTransport no longer returns invalid certificates.

This change fixes a defect where sometimes a transport without a certificate would raise exceptions, sometimes return an invalid Certificate object wrapped around None. This fixes that bug, as well as adding some test coverage for those constructors and the error reporting surrounding them.

Author: glyph

Reviewer: jerub

Fixes #1976

comment:8 Changed 5 years ago by <automation>

  • Owner glyph deleted
Note: See TracTickets for help on using tickets.