Skip to content

replace rfc7512 URI patch with latest version in Fedora#172

Closed
becm wants to merge 0 commit into
OpenVPN:masterfrom
becm:master
Closed

replace rfc7512 URI patch with latest version in Fedora#172
becm wants to merge 0 commit into
OpenVPN:masterfrom
becm:master

Conversation

@becm
Copy link
Copy Markdown
Contributor

@becm becm commented Apr 16, 2020

The pkcs11-helper patch for handling rfc7512 PKCS11 URIs has known flaws:
https://bugzilla.redhat.com/show_bug.cgi?id=1516474
https://bugzilla.redhat.com/show_bug.cgi?id=1825496

This got fixed in Fedora, which should have a large user base for pkcs11-helper.
All changes in patch are platform-independent so behaviour should be consistent.

Update might fix #1075 if problems are related to serial length.
Initial post not clear on this, the serial value looks "obfuscated".

@becm becm changed the title update RFC7512 URI patch to latest version update/extend apply RFC7512 URI encoding fixes Apr 18, 2020
@becm becm changed the title update/extend apply RFC7512 URI encoding fixes update/extend encoding fixes for RFC7512 URI patch Apr 18, 2020
@becm becm changed the title update/extend encoding fixes for RFC7512 URI patch apply/extend fixes to RFC7512 URI patch Apr 18, 2020
@becm becm changed the title apply/extend fixes to RFC7512 URI patch apply bugfixes to RFC7512 URI patch Apr 23, 2020
@becm
Copy link
Copy Markdown
Contributor Author

becm commented Apr 23, 2020

Patch orign now includes 2nd bugfix as well.

@mattock
Copy link
Copy Markdown
Member

mattock commented Apr 23, 2020

I've added this to today's meeting agenda:

@mattock
Copy link
Copy Markdown
Member

mattock commented Apr 24, 2020

In yesterday's meeting we agreed that having a look at the Fedora's version of the patch would make sense as that is in relatively wide use and the code paths touched don't seem at all Windows-specific.

@dsommers is this the correct patch?

@dsommers
Copy link
Copy Markdown
Member

@dsommers is this the correct patch?

* https://koji.fedoraproject.org/koji/fileinfo?rpmID=20355303&filename=pkcs11-helper-rfc7512.patch

I would suggest to use what you find here: https://src.fedoraproject.org/rpms/pkcs11-helper/tree/master

This is the repository used by Koji to fetch the needed files doing the build. Koji is just the build system for Fedora packages. The master branch from the URL I'm pointing at is what is used in Fedora Rawhide (the "development" branch of the next Fedora). Alternatively you can pull what is found in the latest 'f??' branch, which targets a specific Fedora release - but that means you need to check the branch each time you consider fetching a new patch.

I would suggest having a simple script downloading this patch, as when that patch disappears it means something has happened in the upstream pkcs11-helper. And also if the patching fails, then we get a different notification that something has changed.

@becm
Copy link
Copy Markdown
Contributor Author

becm commented Apr 24, 2020

Sorry in advance for the wall of text, but it seemed to me code origin and timeline were also a source of confusion in the 2020-04-23 meeting...

Origin of work seems to be @dwmw2's pull request (upstream).

  • 2015, May: Integration of rfc7512 changes in Fedora code for pkcs11-helper 1.11.
  • 2017, February: Release of pkcs11-helper 1.22, also updated in Fedora.
  • 2017, Oktober 9th: copyright notice in pkcs11-helper changed, no upstream adaption.
  • 2017, Oktober 18th: OpenVPN adds patch (still matching v1.22) from upstream.
  • 2017, November: A bug gets fixed in Fedora patch, update of pull request.
    OpenVPN's libpkcs11-helper-1.dll out of sync with RH/Fedora (outdated local patch).
  • 2020, April: Investigation of rfc7512 URI problems in OpenVPN on Windows.
    Identified locally missing update and a further bug in upstream code.
    Travis CI Windows/x64 build works for affected token serial and CertID.
  • 2020-04-18: Report bug and suggested fix to Fedora bug tracker.
  • 2020-04-20: Fix is applied to upstream (force push).
  • 2020-04-23: Discussion of this pull request in OpenVPN meeting.
  • 2020-04-24: Fedora code updated.

@becm
Copy link
Copy Markdown
Contributor Author

becm commented Apr 24, 2020

@dsommers @mattock beware when not applying the above changes directly:

  • the upstream pull request is off by a line when applied to pkcs11-helper v1.22
  • Fedoras patch is currently missing the latest upstream bugfix (and has a git header referencing outdated commit IDs)

@dwmw2
Copy link
Copy Markdown

dwmw2 commented Apr 24, 2020

Sorry, I got as far as updating OpenSC/pkcs11-helper#4 but hadn't updated the Fedora packages. Should be done now.

@becm
Copy link
Copy Markdown
Contributor Author

becm commented Apr 24, 2020

To me it seems the build environment would have to be adapted to cleanly support downloaded patches, it already does some matching to find patches for extracted source packages. Most components to do more seem to be there but I am not confident to tinker around without creating a mess in the process.

As a intermediate step (and to match the result of the 2020-04-23 meeting) we could for now just replace the local copy with the pristine Fedora patch (git header included). Thanks to @dwmw2's latest update this should work without drawbacks.
patch set update frequency of sample size 2 suggests; OK for the next 2-3 years

If either updates pkcs11-helper, some sort of manual action will be needed anyways.

@dsommers any reservations to the just make a static copy approach until someone is in the mood to tackle build environment refactoring?

@mattock
Copy link
Copy Markdown
Member

mattock commented Apr 25, 2020

I would be fine with just updating the patch we have in this repository.

@dsommers
Copy link
Copy Markdown
Member

For a quick solution, static copy is fine. But I think it way be worth looking into automating the patch fetching, and it seems like it will take quite some time before this issue will be resolved in pkcs11-helper natively.

That said, it is @mattock who has the final word.

@becm becm changed the title apply bugfixes to RFC7512 URI patch replace rfc7512 URI patch with latest version in Fedora Apr 25, 2020
mattock added a commit to mattock/openvpn-build that referenced this pull request Aug 17, 2020
URL: OpenVPN#172
Signed-off-by: Samuli Seppänen <samuli@openvpn.net>
@mattock
Copy link
Copy Markdown
Member

mattock commented Aug 17, 2020

This is now part of #175

cron2 pushed a commit that referenced this pull request Aug 26, 2020
URL: #172
Signed-off-by: Samuli Seppänen <samuli@openvpn.net>
@becm becm closed this Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants