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

Install replaces opensc.conf with a minimal opensc.conf #1102 #1449

Closed
dengert opened this issue Aug 15, 2018 · 6 comments
Closed

Install replaces opensc.conf with a minimal opensc.conf #1102 #1449

dengert opened this issue Aug 15, 2018 · 6 comments

Comments

@dengert
Copy link
Member

dengert commented Aug 15, 2018

Problem Description

Install in previous versions would install opensc.conf.new. Now with c003f38 it installs a minimal opensc.conf overwriting the existing opensc.conf with any changes the user may have made. (I spent all afternoon trying to figure out what happened.)

See #1102

Proposed Resolution

At a minimum, fix the code in c003f38 that tested for an existing opensc.conf in etc/Makefile.am and install the minimal opensc.conf as opensc.conf.newLook at the removed test in: https://github.com/OpenSC/OpenSC/commit/c003f3825e5217cd3ef8272f4e8ebe47270a7847#diff-3392c95f52e00a39c33e6f4ce45b54a3L35 if [ -f "$(DESTDIR)$(sysconfdir)/opensc.conf" ]; then `

Another solution is to revert c003f38 and provide a script to remove all the comments and empty stanza formatting to produce a minimal opensc.conf from a user edited version of a full opensc.conf or opensc.conf.example with user changes.

This may also require many of the lines in opensc.conf.example that are not comments such as card_atr or secure_messaging stanzas. Users of these cards can uncomment them as needed.

Then an additional way to address #1102 is to change scconf_parse to not allocate memory for stanzas that contain only comments.

Steps to reproduce

Logs

@Jakuje
Copy link
Member

Jakuje commented Aug 16, 2018

What platform install you refer to? For example on Fedora/RHEL with RPMs, this is not going to happen. If you update opensc with not modified .conf file, you will get a new one, but if you modified this configuration before, the new file will have suffix .rpmnew. Nothing will be lost and one can merge things manually (clean up in this case). If I remember well, there is similar mechanisms in dpkg. I am not sure if there is still anyone out there installing with make install only in production systems.

@dengert
Copy link
Member Author

dengert commented Aug 16, 2018

I am referring to make install which used to install a new opensc.conf as opensc.conf.new if opensc.conf existed. Luckily most distributions do a much better job of handling configuration files.

@dengert
Copy link
Member Author

dengert commented Aug 16, 2018

Using OpenSC master which does not have c003f3825e5217cd3ef8272f4e8ebe47270a7847A I ran some quick Valgrind tests. This was using opensc-tool -l that will parse the opensc.conf

Aa full opensc.conf with comments has 47880 bytes. Running
sed '/^[[:space:]]*#/d;/^$/d' < opensc.conf > opensc.conf.no.comments
producesa 24145 byte file without comments.

Valgrind shows with scconf_free cleaning up:
definitely lost: 40 bytes in 1 blocks
indirectly lost: 72 bytes in 3 blocks

By modifying scconf_free to not free up the conf will let Vallgrind show how big the conf really is.
With the full opensc.conf:
definitely lost: 72 bytes in 2 blocks
indirectly lost: 87,490 bytes in 3,035 blocks

Using opensc.conf.no.comments:
definitely lost: 72 bytes in 2 blocks
indirectly lost: 43,337 bytes in 1,772 blocks

Thus by just running
sed '/^[[:space:]]*#/d;/^$/d' < opensc.conf > opensc.conf.no.comments
saves 44,153 bytes and 1263 blocks.

With a minimal opensc.conf, (151 bytes in my case, I eliminated all the card_atr and SM entries and comments) Valgrind shows:
definitely lost: 72 bytes in 2 blocks
indirectly lost: 588 bytes in 31 blocks

Modifying the scconf routines to not save comment would save 44K. Then Making all the card_atr and SM stazas comments would get down to the equivalent of the minimal opensc.conf in c003f3825e5217cd3ef8272f4e8ebe47270a7847A and use less the 1000 bytes.

So I propose the example opensc.conf also comment out the card_atr and SM stanzas, and add
a comment on how to create a minimal opensc.conf by using a script that contains something like
sed '/^[[:space:]]*#/d;/^$/d' < opensc.conf > opensc.conf.no.comments

If a user that has memory concerns they can create their own minimal opensc.conf

The overwriting of the opensc.conf when using make install still need to be fixed.

@frankmorgner
Copy link
Member

👍 @dengert please make a pull request which avoids overwriting opensc.conf.

The default behavior should be good enough to not require any configuration file. I only left what's required for making a bug report. If something is missing, we should hard code it (Are you missing something?).

@dengert
Copy link
Member Author

dengert commented Aug 20, 2018

As requested, Submitted PR #1453.

@dengert
Copy link
Member Author

dengert commented Aug 24, 2018

@mouse07410 While looking at the scconf_parse, I remember you wanted to use the OpenPGP driver from Tokend, but a PIV driver from PKCS#11. (or was it the reverse?)

The way I read the opensc.conf, you could add/modify the block:

app tokend {
            card_drivers = openpgp ;
           #or any other key values or blocks that are in app default for example:
           card_atr 3b:f8:13:00:00:81:31:fe:15:59:75:62:69:6b:65:79:34:d4 {
                name = "Yubikey 4";
                # Select the PKI applet to use ("PIV-II" or "openpgp")
                driver = "openpgp";
                # Recover from other applications accessing a different applet
                flags = "keep_alive";
        }
}

This would then cause tokend to use the openpgp driver for the Yubikey 4, but all the other apps
to use the PIV driver.

Note there is a app opensc-pkcs11 and app onepin-opensc-pkcs11 that could have their own settings.

`

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

No branches or pull requests

3 participants