After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778456 - [review] lr/pkcs11-pin: add support for PKCS#11 PINs
[review] lr/pkcs11-pin: add support for PKCS#11 PINs
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-02-10 15:57 UTC by Lubomir Rintel
Modified: 2017-11-27 17:22 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Lubomir Rintel 2017-02-10 15:57:35 UTC
For the private key we use the private-key-password property. Let's also add analogous properties for {phase2-,}{ca-cert,client-cert}, since they also might be on a PIN-protected PKCS#11 token.

https://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/pkcs11-pin
Comment 1 Thomas Haller 2017-02-16 15:25:16 UTC
>> core/8021x: add password properties for certificates

the new properties are not freed in finalize().


>> supplicant: set PIN for objects on PKCS#11 tokens


indention at several places:

+                                                 nm_setting_802_1x_get_phase2_client_cert_password_flags (setting),
+                                        error)) {


rest lgtm



pushed several follow-up commits on your branch.
Comment 2 Lubomir Rintel 2017-02-16 16:28:38 UTC
(In reply to Thomas Haller from comment #1)
> >> core/8021x: add password properties for certificates
> 
> the new properties are not freed in finalize().
> 
> 
> >> supplicant: set PIN for objects on PKCS#11 tokens
> 
> 
> indention at several places:
> 
> +                                                
> nm_setting_802_1x_get_phase2_client_cert_password_flags (setting),
> +                                        error)) {
> 
> 

Fixed

> pushed several follow-up commits on your branch.

The commits look fine to me.
Comment 3 Beniamino Galvani 2017-02-17 12:37:58 UTC
> core/8021x: add password properties for certificates

+NM_AVAILABLE_IN_1_8
+const char *           nm_setting_802_1x_get_ca_cert_password        (NMSetting8021x *setting);

These should also be added to libnm.ver.


+       case PROP_PHASE2_CA_CERT_PASSWORD_FLAGS:
+               priv->phase2_ca_cert_password_flags = g_value_get_flags (value);
+               break;
+

Extra line.

The rest LGTM.
Comment 4 Lubomir Rintel 2017-02-25 21:18:18 UTC
b4a976fd1175c2311bd2c2ee6b23345cfd56efd7
Comment 5 David Woodhouse 2017-11-27 17:22:29 UTC
Did you only fix this for 802.1x and not for VPN clients?