haraka/Haraka#1099 use TLSSocket instead of createSecurePair #1672

Merged
merged 1 commit into from Oct 18, 2016

Conversation

Projects
None yet
4 participants
@typingArtist
Collaborator

typingArtist commented Oct 18, 2016

While this change looks like cosmetics, it is needed for advanced TLSSocket operations like OCSP Stapling. This also fixes long-lasting #1099.

Checklist:

  • no need for updating the docs
  • no need for updating tests
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Oct 18, 2016

Current coverage is 35.97% (diff: 6.66%)

Merging #1672 into master will increase coverage by 0.08%

@@             master      #1672   diff @@
==========================================
  Files            24         24          
  Lines          6068       6054    -14   
  Methods         782        782          
  Messages          0          0          
  Branches       1535       1533     -2   
==========================================
  Hits           2178       2178          
+ Misses         3890       3876    -14   
  Partials          0          0          

Powered by Codecov. Last update b27008f...53c166a

codecov-io commented Oct 18, 2016

Current coverage is 35.97% (diff: 6.66%)

Merging #1672 into master will increase coverage by 0.08%

@@             master      #1672   diff @@
==========================================
  Files            24         24          
  Lines          6068       6054    -14   
  Methods         782        782          
  Messages          0          0          
  Branches       1535       1533     -2   
==========================================
  Hits           2178       2178          
+ Misses         3890       3876    -14   
  Partials          0          0          

Powered by Codecov. Last update b27008f...53c166a

@msimerson

This comment has been minimized.

Show comment
Hide comment
@msimerson

msimerson Oct 18, 2016

Member

While I'm really happy to see that crusty old tls_socket get some love, it might be better to just jettison that sucker. See #1099 (we can do that now b/c we recently dropped support for node 0.10).

Member

msimerson commented Oct 18, 2016

While I'm really happy to see that crusty old tls_socket get some love, it might be better to just jettison that sucker. See #1099 (we can do that now b/c we recently dropped support for node 0.10).

@typingArtist

This comment has been minimized.

Show comment
Hide comment
@typingArtist

typingArtist Oct 18, 2016

Collaborator

My aim was just to make it work with TLSSocket for allowing me to add OCSP Stapling functionality (see #1673).

If you don’t mind, I leave the refactoring to you.

Collaborator

typingArtist commented Oct 18, 2016

My aim was just to make it work with TLSSocket for allowing me to add OCSP Stapling functionality (see #1673).

If you don’t mind, I leave the refactoring to you.

@typingArtist typingArtist referenced this pull request Oct 18, 2016

Closed

Ocsp stapling #1673

4 of 5 tasks complete

@msimerson msimerson merged commit 2a51d5a into haraka:master Oct 18, 2016

2 of 3 checks passed

codecov/patch 6.66% of diff hit (target 35.89%)
Details
codecov/project 35.97% (+0.08%) compared to b27008f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@typingArtist typingArtist deleted the typingArtist:tlssocket branch Oct 18, 2016

typingArtist added a commit to typingArtist/Haraka that referenced this pull request Oct 18, 2016

@smfreegard

This comment has been minimized.

Show comment
Hide comment
@smfreegard

smfreegard Oct 18, 2016

Collaborator

Why did you only change the server end to use TLSSocket and ignore the client? Surely supporting OSCP stapling in both is desirable?

Collaborator

smfreegard commented Oct 18, 2016

Why did you only change the server end to use TLSSocket and ignore the client? Surely supporting OSCP stapling in both is desirable?

@typingArtist

This comment has been minimized.

Show comment
Hide comment
@typingArtist

typingArtist Oct 18, 2016

Collaborator

Definitely it is. I just started with the one end I expected to be more work.

Collaborator

typingArtist commented Oct 18, 2016

Definitely it is. I just started with the one end I expected to be more work.

@smfreegard

This comment has been minimized.

Show comment
Hide comment
@smfreegard

smfreegard Oct 18, 2016

Collaborator

Ok - no great. Nice work on this. I've wanted to support SMTP STS or DANE in Haraka for a while now, but it's reliant of Node getting support for DNSSEC and TLSA record types etc.

Collaborator

smfreegard commented Oct 18, 2016

Ok - no great. Nice work on this. I've wanted to support SMTP STS or DANE in Haraka for a while now, but it's reliant of Node getting support for DNSSEC and TLSA record types etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment