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

Configure: do not check for an absolute prefix in cross-builds #16445

Closed
wants to merge 1 commit into from
Closed

Configure: do not check for an absolute prefix in cross-builds #16445

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Contributor

@vszakats vszakats commented Aug 27, 2021

No description provided.

@openssl-machine openssl-machine added hold: cla required The contributor needs to submit a license agreement and removed hold: cla required The contributor needs to submit a license agreement labels Aug 27, 2021
Configure Outdated
@@ -937,7 +937,8 @@ while (@argvcopy)
{
$config{prefix}=$1;
die "Directory given with --prefix MUST be absolute\n"
unless file_name_is_absolute($config{prefix});
unless $user{CROSS_COMPILE} ||
Copy link
Member

@t8m t8m Aug 27, 2021

Choose a reason for hiding this comment

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

I think the check needs to be moved down after all the options are parsed. And $config should be used instead of $user.

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.

The change seems OK to me now. However it is no longer acceptable with CLA: trivial. Could you please submit a regular CLA? https://www.openssl.org/policies/cla.html

@t8m t8m added approval: review pending This pull request needs review by a committer branch: master Merge to master branch cla: trivial One of the commits is marked as 'CLA: trivial' hold: cla required The contributor needs to submit a license agreement triaged: bug The issue/pr is/fixes a bug labels Aug 30, 2021
@t-j-h
Copy link
Member

t-j-h commented Aug 30, 2021

@t8m is correct in that although this is a simple change, it steps outside the project comfort zone for legally trivial (a different measurement).

@t8m
Copy link
Member

t8m commented Aug 30, 2021

(I already did before, for an earlier commit email address)

Perhaps you could change the commit's e-mail address to use this old one?

@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Aug 30, 2021
The check is always made according to the host platform's rules, which may
not be true for true when the target platform is different, e.g. when
cross-building for Windows on a Linux machine. So skip this check when
used together with the `--cross-compile-prefix=` option.

CLA: trivial

Fixes #9520
@vszakats vszakats closed this Sep 13, 2021
@vszakats vszakats deleted the xabsprefix branch September 13, 2021 19:44
vszakats added a commit to curl/curl-for-win that referenced this pull request Dec 3, 2021
Instead of using OpenSSL as a default backend, schannel builds will
tap on the OS's TLS/SSL capabilities, meaning the exact feature set will
depend on OS version (and possibly OS configuration settings).

This should generally not pose a problem when running on a fairly recent
(officially supported) Windows release (meaning Windows 10 or equivalent
server version).

The expected advantage of these builds is significantly smaller on-disk
and in-memory footprint, faster and simpler builds, and all the upsides
of avoiding a complex 3rd party library such an OpenSSL with its
vulnerabilities and attack surface. One specific vulnerability that this
will universally fix is CVE-2019-12572 which to this day is not fixed in
OpenSSL, and in fact OpenSSL denies this has anything to do with OpenSSL
itself, so there is little chance this will ever get addressed.

This branch also allows to go back to the curl-for-win promise of not using
any local patches, by dropping a necessary OpenSSL patch to be able to
mitigate said CVE somewhat better. Another decisive moment was OpenSSL's
recently laid out plans that made it unlikely that this, or other pending
TLS/SSL/QUIC-related bits will get the expected attention in the
foreseeable future.

Ref: https://blog.mirch.io/2019/06/10/cve-2019-12572-pia-windows-privilege-escalation-malicious-openssl-engine/
Ref: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-12572
Ref: openssl/openssl#17151
Ref: openssl/openssl#16445
Ref: openssl/openssl#9520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch cla: trivial One of the commits is marked as 'CLA: trivial' triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants