Skip to content

Comments

Relax absolut path checking in our 'file' scheme implementation#27482

Closed
levitte wants to merge 1 commit intoopenssl:masterfrom
levitte:fix-27461
Closed

Relax absolut path checking in our 'file' scheme implementation#27482
levitte wants to merge 1 commit intoopenssl:masterfrom
levitte:fix-27461

Conversation

@levitte
Copy link
Member

@levitte levitte commented Apr 23, 2025

So far, we strictly obeyed RFC 8089, which only allows absolute paths
in a file: URI. However, this seems to give a confusing user
experience, where something like file:foo.pem wouldn't open foo.pem,
even though it's there in the current directory, but file:$(pwd)/foo.pem
would.

To be less surprising for such use cases, we relax our implementation
visavi RFC 8089 to allow relative paths.

Fixes #27461

@DDvO
Copy link
Contributor

DDvO commented Apr 23, 2025

Nice that this generalization even simplifies the code.

@petrovr
Copy link

petrovr commented Apr 23, 2025

This is not correction to #27461 .
Which X.509 lookup method to replace "by file" or "by dir"?

@levitte
Copy link
Member Author

levitte commented Apr 23, 2025

"by store" is the OSSL_STORE replacement of "by file" and "by dir".

@levitte levitte marked this pull request as ready for review April 24, 2025 02:27
@levitte levitte 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 tests: present The PR has suitable tests present labels Apr 24, 2025
mattcaswell
mattcaswell previously approved these changes Apr 24, 2025
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

I wonder if this is worthy of a CHANGES.md entry?

@levitte
Copy link
Member Author

levitte commented Apr 24, 2025

I wonder if this is worthy of a CHANGES.md entry?

Good idea. I added it just now... which means that you will have to re-approve as soon as I've also rebased...

So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes openssl#27461
@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 Apr 24, 2025
@openssl-machine openssl-machine 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 Apr 25, 2025
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member

t8m commented Apr 25, 2025

Merged to the master branch. Thank you.

@t8m t8m closed this Apr 25, 2025
openssl-machine pushed a commit that referenced this pull request Apr 25, 2025
So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes #27461

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #27482)
@levitte levitte deleted the fix-27461 branch April 25, 2025 14:34
DDvO pushed a commit to siemens/openssl that referenced this pull request Jun 16, 2025
So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes openssl#27461

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#27482)
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes openssl#27461

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#27482)
MichaelA-Fireblocks pushed a commit to MichaelA-Fireblocks/openssl that referenced this pull request Jul 15, 2025
So far, we strictly obeyed [RFC 8089], which only allows absolute paths
in a 'file:' URI.  However, this seems to give a confusing user
experience, where something like 'file:foo.pem' wouldn't open foo.pem,
even though it's there in the current directory, but 'file:$(pwd)/foo.pem'
would.

To be less surprising for such use cases, we relax our implementation
visavi [RFC 8089] to allow relative paths.

[RFC 8089]: https://datatracker.ietf.org/doc/html/rfc8089

Fixes openssl#27461

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#27482)
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 tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loading certs via file: scheme only works for absolute paths and fails silently on error

6 participants