Skip to content

Conversation

@daum3ns
Copy link
Contributor

@daum3ns daum3ns commented Mar 4, 2025

The change checks for all HTTP methods in ssl_record, not only GET, POST, PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial

The change checks for all HTTP methods in ssl_record, not only GET, POST,
PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial
@t-j-h
Copy link
Member

t-j-h commented Mar 4, 2025

Ok with trivial for this

@t8m t8m added branch: master Applies to master branch approval: review pending This pull request needs review by a committer triaged: feature The issue/pr requests/adds a feature cla: trivial One of the commits is marked as 'CLA: trivial' tests: exempted The PR is exempt from requirements for testing labels Mar 4, 2025
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with CLA: trivial

@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Mar 4, 2025
@daum3ns
Copy link
Contributor Author

daum3ns commented Mar 5, 2025

i don't understand the failing test... can someone give me a hint?

@t8m
Copy link
Member

t8m commented Mar 5, 2025

i don't understand the failing test... can someone give me a hint?

It is unrelated to your PR. There is an intermittent issue which we are currently investigating.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@t8m t8m added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Mar 5, 2025
@t8m
Copy link
Member

t8m commented Mar 5, 2025

Merged to the master branch. Thank you for your contribution.

@t8m t8m closed this Mar 5, 2025
openssl-machine pushed a commit that referenced this pull request Mar 5, 2025
The change checks for all HTTP methods in ssl_record, not only GET, POST,
PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #26968)
daum3ns pushed a commit to daum3ns/httpd that referenced this pull request Mar 18, 2025
Instead of disabling the ssl filter and hand a fake request back to the
core handler we should abort the connection directly. The current
implementation allows to exhaust workers by sending HTTP requests on HTTPS port.
Additionally the OpenSSL library doesn't detect HTTP PATCH, OPTIONS, DELETE
and TRACE methods, so the current implementation is also buggy.
See this openssl PR:
openssl/openssl#26968
daum3ns added a commit to daum3ns/httpd that referenced this pull request Mar 18, 2025
Shutdown the ssl filter instead of disabling it and create a fake request
to pass it to the core filter. The current implementation allows to
exhaust worker threads by sending HTTP on HTTPS. Additionally the
Openssl library doesn't recognize the http methods PATCH, DELETE,
OPTIONS and TRACE, so the current implementation only works partially.
See the PR here: openssl/openssl#26968
daum3ns added a commit to daum3ns/httpd that referenced this pull request Mar 18, 2025
Shutdown the ssl filter and abort the connection instead of disabling
is and pass a fake request to the core handler. The current implementation
allows to exhaust workers by sendin HTTP request to HTTPS port. Additionally
the Openssl lib doesn't detect the http methods PATCH, DELETE, OPTIONS and TRACE.
So the current implementation only works partially.
See openssl PR: openssl/openssl#26968
DDvO pushed a commit to siemens/openssl that referenced this pull request Jun 16, 2025
The change checks for all HTTP methods in ssl_record, not only GET, POST,
PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#26968)
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
The change checks for all HTTP methods in ssl_record, not only GET, POST,
PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#26968)
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
The change checks for all HTTP methods in ssl_record, not only GET, POST,
PUT and HEAD. (additionally PATCH, DELETE, OPTIONS and TRACE)

CLA: trivial

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#26968)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch cla: trivial One of the commits is marked as 'CLA: trivial' tests: exempted The PR is exempt from requirements for testing triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants