Page MenuHomeFreeBSD

security/libu2f-host: add USERS/GROUPS framework
ClosedPublic

Authored by cpm on Jun 23 2017, 6:22 PM.

Details

Summary
- Fix LICENSE
- Use GROUPS
- Add sample config file
- Add pkg-message
- Take maintainership

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Hi Baptiste,

Would you mind to add the u2f.conf to the port that you've already committed?

Thanks

security/libu2f-host/pkg-plist
2 ↗(On Diff #30007)

All your rules do is change permissions, shouldn't you be using devfs.rules for this instead of devd actions?

security/libu2f-host/pkg-plist
2 ↗(On Diff #30007)

It was discussed with hselasky@

I want to hear bapt's opinion, because he has already committed this port.

u2f.conf changes the permissions of the $cdev node of the ugen device so that the libu2f-host can access this devices on FreeBSD.

Please feel free to comment or propose changes.

Can you update diff against the existing port?

security/libu2f-host/files/u2f.conf.in
6 ↗(On Diff #30007)

Did you convert devd rules from udev via script? If so please add it under /usr/ports/Tools/scripts/ or create USES.

8 ↗(On Diff #30007)

This grants r/w access for default group which is probably wheel unless overriden via /etc/devfs.rules. Maybe create a dedicated group (e.g. u2f) and document it somewhere (manpage or pkg-message). Here're a few examples:

  • comms/telldus-core/files/patch-tdadmin-freebsd-devd-tellstick.conf
  • devel/msp430-debug-stack/files/mspfet.conf
  • print/cups/files/cups.conf.sample
  • graphics/sane-backends/files/pkg-message.in
security/libu2f-host/pkg-plist
2 ↗(On Diff #30007)

Can you quote the discussion or post a link to the achived copy?

sorry I have missed that review

if you want to grab the port and make it yours, feel free to grab it :)

I would prefer a devfs.rules but maybe the devd conf file is more userfriendly

security/libu2f-host/files/u2f.conf.in
8 ↗(On Diff #30007)

I agree

In D11328#240123, @bapt wrote:

sorry I have missed that review

if you want to grab the port and make it yours, feel free to grab it :)

I would prefer a devfs.rules but maybe the devd conf file is more userfriendly

Thanks Baptiste!

I hope that Hans will shed some light on the device permissions then I'll proceed to change the port accordingly.

cpm marked an inline comment as done.Jul 16 2017, 1:18 PM
cpm added inline comments.
security/libu2f-host/files/u2f.conf.in
6 ↗(On Diff #30007)

Indeed, writing an converter script would be useful, but I did it from scratch.

8 ↗(On Diff #30007)

Thanks for pointing this out! I need to dig more to be completely sure.

security/libu2f-host/pkg-plist
2 ↗(On Diff #30007)

Please, take a look at your inbox.

It might be better if this library installed its own group which these devices are chowned to?

cpm marked an inline comment as done.
cpm edited the summary of this revision. (Show Details)

It might be better if this library installed its own group which these devices are chowned to?

Regarding device permissions, please check if I forgot anything else.

Thanks for your quick reply, Hans

cpm retitled this revision from security/libu2f-host: Yubico Universal 2nd Factor (U2F) Host C Library to security/libu2f-host: add USERS/GROUPS framework.Jul 16 2017, 3:48 PM

Update diff against r445430

Looks good to me with regards to the USB bits and the devd script. I'm not sure if you should install u2f.conf directly or postfixed with .sample which will require user-interaction.

This revision is now accepted and ready to land.Jul 16 2017, 4:03 PM

Looks good to me with regards to the USB bits and the devd script. I'm not sure if you should install u2f.conf directly or postfixed with .sample which will require user-interaction.

Well, I think that just installing u2f.conf would be enough. Maybe adding a note in pkg-message to remind consumers that they should restart devd?

Thanks again :)

In D11328#240519, @cpm wrote:

Looks good to me with regards to the USB bits and the devd script. I'm not sure if you should install u2f.conf directly or postfixed with .sample which will require user-interaction.

Well, I think that just installing u2f.conf would be enough. Maybe adding a note in pkg-message to remind consumers that they should restart devd?

Here is an example:

======================================================================
                            
Note that a sample file is installed in %%PREFIX%%/etc/devd/u2f.conf
to allow u2f access permissions.

You should only restart devd with the command:
# service devd restart

======================================================================
cpm edited edge metadata.
cpm edited the summary of this revision. (Show Details)
  • Fix LICENSE
  • Rewrite pkg-message to remove redundant information.
This revision now requires review to proceed.Jul 16 2017, 4:28 PM
This revision is now accepted and ready to land.Jul 16 2017, 5:18 PM
GIDs
172 ↗(On Diff #30835)

Maybe pick group-only free number e.g., 116, 141, 151.

UIDs
177 ↗(On Diff #30835)

Maybe drop this, so GIDs can reserve group-only number.

security/libu2f-host/Makefile
31 ↗(On Diff #30835)

Why do you need a separate user as well? Does the port install a daemon and/or rc.d script?

security/libu2f-host/files/pkg-message.in
3 ↗(On Diff #30835)

"Note that" is redundant.

7 ↗(On Diff #30835)

You're forgetting the user has to be part of u2f group. Maybe rephrase the whole file e.g.,

The package requires read/write access to USB devices. To facilitate
such access it comes with a devd.conf(5) file, but you still need to
restart devd(8), add the desired users to "u2f" group and log those
out of the current session. For example:

  $ pw group mod u2f -m <user>
  $ shutdown -r now

For details, see %%PREFIX%%/etc/devd/u2f.conf
security/libu2f-host/pkg-plist
11 ↗(On Diff #30835)

Can you alphabetically sort as if @sample was absent? See the output from make restage && make makeplist.

cpm edited edge metadata.
cpm edited the summary of this revision. (Show Details)
  • Use only GROUPS
  • Rewrite pkg-message according to Jan's proposal
  • Sort pkg-plist
This revision now requires review to proceed.Jul 16 2017, 5:58 PM
cpm marked 5 inline comments as done.Jul 16 2017, 5:59 PM
cpm marked an inline comment as not done.
This comment was removed by jbeich.

Don't forget to bump PORTREVISION.

Macro lgtm:

This revision is now accepted and ready to land.Jul 16 2017, 6:09 PM

Thank you all for the great review!

This revision was automatically updated to reflect the committed changes.