| Message ID | 20241103124525.8392-1-juraj@sarinay.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9907cda95fcbf44141b1292faab89cf8ec542f22 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: nfc: Propagate ISO14443 type A target ATS to userspace via netlink | expand |
On Sun, Nov 03, 2024 at 01:45:25PM +0100, Juraj Šarinay wrote: > Add a 20-byte field ats to struct nfc_target and expose it as > NFC_ATTR_TARGET_ATS via the netlink interface. The payload contains > 'historical bytes' that help to distinguish cards from one another. > The information is commonly used to assemble an emulated ATR similar > to that reported by smart cards with contacts. > > Add a 20-byte field target_ats to struct nci_dev to hold the payload > obtained in nci_rf_intf_activated_ntf_packet() and copy it to over to > nfc_target.ats in nci_activate_target(). The approach is similar > to the handling of 'general bytes' within ATR_RES. Hi Juraj, Perhaps I misunderstand things, and perhaps there is precedence in relation to ATR_RES. But I am slightly concerned that this leans towards exposing internal details rather then semantics via netlink. > Replace the hard-coded size of rats_res within struct > activation_params_nfca_poll_iso_dep by the equal constant NFC_ATS_MAXSIZE > now defined in nfc.h > > Within NCI, the information corresponds to the 'RATS Response' activation > parameter that omits the initial length byte TL. This loses no > information and is consistent with our handling of SENSB_RES that > also drops the first (constant) byte. > > Tested with nxp_nci_i2c on a few type A targets including an > ICAO 9303 compliant passport. > > I refrain from the corresponding change to digital_in_recv_ats() > to have the few drivers based on digital.h fill nfc_target.ats, > as I have no way to test it. That class of drivers appear not to set > NFC_ATTR_TARGET_SENSB_RES either. Consider a separate patch to propagate > (all) the parameters. > > Signed-off-by: Juraj Šarinay <juraj@sarinay.com> ...
On Wed, 2024-11-06 at 10:18 +0000, Simon Horman wrote: > > Add a 20-byte field ats to struct nfc_target and expose it as > > NFC_ATTR_TARGET_ATS via the netlink interface. The payload contains > > 'historical bytes' that help to distinguish cards from one another. > > The information is commonly used to assemble an emulated ATR similar > > to that reported by smart cards with contacts. > > Perhaps I misunderstand things, and perhaps there is precedence in relation > to ATR_RES. But I am slightly concerned that this leans towards exposing > internal details rather then semantics via netlink. > Hi Simon Thanks for the feedback. NFC_ATTR_TARGET_ATS would serve a similar purpose as the following attributes the kernel already exposes (see nfc.h): * @NFC_ATTR_TARGET_SENS_RES: NFC-A targets extra information such as NFCID * @NFC_ATTR_TARGET_SEL_RES: NFC-A targets extra information (useful if the * target is not NFC-Forum compliant) * @NFC_ATTR_TARGET_NFCID1: NFC-A targets identifier, max 10 bytes * @NFC_ATTR_TARGET_SENSB_RES: NFC-B targets extra information, max 12 bytes * @NFC_ATTR_TARGET_SENSF_RES: NFC-F targets extra information, max 18 bytes * @NFC_ATTR_TARGET_ISO15693_DSFID: ISO 15693 Data Storage Format Identifier * @NFC_ATTR_TARGET_ISO15693_UID: ISO 15693 Unique Identifier The ATR I am after means "Answer To Reset" as defined in ISO 7816. It has little to do with ATR_RES := "Attribute Request Response" defined in the NFC Digital specification. I only mentioned ATR_RES as the source of the information handled by nci_store_general_bytes_nfc_dep(), the function that motivated some of the code I propose to add. Part 3 of the PC/SC Specification, Section 3.1.3.2.3 on ATR may perhaps serve as a more authoritative source of motivation for the patch. https://pcscworkgroup.com/Download/Specifications/pcsc3_v2.01.09.pdf The goal is to access contactless identification cards via PC/SC. Juraj
On 11/6/24 15:58, Juraj Šarinay wrote: > On Wed, 2024-11-06 at 10:18 +0000, Simon Horman wrote: >>> Add a 20-byte field ats to struct nfc_target and expose it as >>> NFC_ATTR_TARGET_ATS via the netlink interface. The payload contains >>> 'historical bytes' that help to distinguish cards from one another. >>> The information is commonly used to assemble an emulated ATR similar >>> to that reported by smart cards with contacts. >> >> Perhaps I misunderstand things, and perhaps there is precedence in relation >> to ATR_RES. But I am slightly concerned that this leans towards exposing >> internal details rather then semantics via netlink. >> > > Hi Simon > > Thanks for the feedback. NFC_ATTR_TARGET_ATS would serve a similar > purpose as the following attributes the kernel already exposes (see > nfc.h): > > * @NFC_ATTR_TARGET_SENS_RES: NFC-A targets extra information such as NFCID > * @NFC_ATTR_TARGET_SEL_RES: NFC-A targets extra information (useful if the > * target is not NFC-Forum compliant) > * @NFC_ATTR_TARGET_NFCID1: NFC-A targets identifier, max 10 bytes > * @NFC_ATTR_TARGET_SENSB_RES: NFC-B targets extra information, max 12 bytes > * @NFC_ATTR_TARGET_SENSF_RES: NFC-F targets extra information, max 18 bytes > * @NFC_ATTR_TARGET_ISO15693_DSFID: ISO 15693 Data Storage Format Identifier > * @NFC_ATTR_TARGET_ISO15693_UID: ISO 15693 Unique Identifier > > The ATR I am after means "Answer To Reset" as defined in ISO 7816. It > has little to do with ATR_RES := "Attribute Request Response" defined > in the NFC Digital specification. I only mentioned ATR_RES as the > source of the information handled by nci_store_general_bytes_nfc_dep(), > the function that motivated some of the code I propose to add. > > Part 3 of the PC/SC Specification, Section 3.1.3.2.3 on ATR may perhaps > serve as a more authoritative source of motivation for the patch. > > https://pcscworkgroup.com/Download/Specifications/pcsc3_v2.01.09.pdf > > The goal is to access contactless identification cards via PC/SC. Makes sense, thanks for the additional details. Small nit: you should have retained Krzysztof tag from from v1. No need to repost. Thanks, Paolo
Hello: This patch was applied to netdev/net-next.git (main) by Paolo Abeni <pabeni@redhat.com>: On Sun, 3 Nov 2024 13:45:25 +0100 you wrote: > Add a 20-byte field ats to struct nfc_target and expose it as > NFC_ATTR_TARGET_ATS via the netlink interface. The payload contains > 'historical bytes' that help to distinguish cards from one another. > The information is commonly used to assemble an emulated ATR similar > to that reported by smart cards with contacts. > > Add a 20-byte field target_ats to struct nci_dev to hold the payload > obtained in nci_rf_intf_activated_ntf_packet() and copy it to over to > nfc_target.ats in nci_activate_target(). The approach is similar > to the handling of 'general bytes' within ATR_RES. > > [...] Here is the summary with links: - [net-next,v2] net: nfc: Propagate ISO14443 type A target ATS to userspace via netlink https://git.kernel.org/netdev/net-next/c/9907cda95fcb You are awesome, thank you!
diff --git a/include/net/nfc/nci.h b/include/net/nfc/nci.h index dc36519d16aa..09efcaed7c3f 100644 --- a/include/net/nfc/nci.h +++ b/include/net/nfc/nci.h @@ -475,7 +475,7 @@ struct nci_rf_discover_ntf { #define NCI_OP_RF_INTF_ACTIVATED_NTF nci_opcode_pack(NCI_GID_RF_MGMT, 0x05) struct activation_params_nfca_poll_iso_dep { __u8 rats_res_len; - __u8 rats_res[20]; + __u8 rats_res[NFC_ATS_MAXSIZE]; }; struct activation_params_nfcb_poll_iso_dep { diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h index ea8595651c38..e180bdf2f82b 100644 --- a/include/net/nfc/nci_core.h +++ b/include/net/nfc/nci_core.h @@ -265,6 +265,10 @@ struct nci_dev { /* stored during intf_activated_ntf */ __u8 remote_gb[NFC_MAX_GT_LEN]; __u8 remote_gb_len; + + /* stored during intf_activated_ntf */ + __u8 target_ats[NFC_ATS_MAXSIZE]; + __u8 target_ats_len; }; /* ----- NCI Devices ----- */ diff --git a/include/net/nfc/nfc.h b/include/net/nfc/nfc.h index 3a3781838c67..127e6c7d910d 100644 --- a/include/net/nfc/nfc.h +++ b/include/net/nfc/nfc.h @@ -86,6 +86,8 @@ struct nfc_ops { * is a type A one. The %sens_res most significant byte must be byte 2 * as described by the NFC Forum digital specification (i.e. the platform * configuration one) while %sens_res least significant byte is byte 1. + * @ats_len: length of Answer To Select in bytes + * @ats: Answer To Select returned by an ISO 14443 Type A target upon activation */ struct nfc_target { u32 idx; @@ -105,6 +107,8 @@ struct nfc_target { u8 is_iso15693; u8 iso15693_dsfid; u8 iso15693_uid[NFC_ISO15693_UID_MAXSIZE]; + u8 ats_len; + u8 ats[NFC_ATS_MAXSIZE]; }; /** diff --git a/include/uapi/linux/nfc.h b/include/uapi/linux/nfc.h index 4fa4e979e948..2f5b4be25261 100644 --- a/include/uapi/linux/nfc.h +++ b/include/uapi/linux/nfc.h @@ -164,6 +164,7 @@ enum nfc_commands { * @NFC_ATTR_VENDOR_SUBCMD: Vendor specific sub command * @NFC_ATTR_VENDOR_DATA: Vendor specific data, to be optionally passed * to a vendor specific command implementation + * @NFC_ATTR_TARGET_ATS: ISO 14443 type A target Answer To Select */ enum nfc_attrs { NFC_ATTR_UNSPEC, @@ -198,6 +199,7 @@ enum nfc_attrs { NFC_ATTR_VENDOR_ID, NFC_ATTR_VENDOR_SUBCMD, NFC_ATTR_VENDOR_DATA, + NFC_ATTR_TARGET_ATS, /* private: internal use only */ __NFC_ATTR_AFTER_LAST }; @@ -225,6 +227,7 @@ enum nfc_sdp_attr { #define NFC_GB_MAXSIZE 48 #define NFC_FIRMWARE_NAME_MAXSIZE 32 #define NFC_ISO15693_UID_MAXSIZE 8 +#define NFC_ATS_MAXSIZE 20 /* NFC protocols */ #define NFC_PROTO_JEWEL 1 diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index f456a5911e7d..1ec5955fe469 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -757,6 +757,14 @@ int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id) } EXPORT_SYMBOL(nci_core_conn_close); +static void nci_set_target_ats(struct nfc_target *target, struct nci_dev *ndev) +{ + if (ndev->target_ats_len > 0) { + target->ats_len = ndev->target_ats_len; + memcpy(target->ats, ndev->target_ats, target->ats_len); + } +} + static int nci_set_local_general_bytes(struct nfc_dev *nfc_dev) { struct nci_dev *ndev = nfc_get_drvdata(nfc_dev); @@ -939,8 +947,11 @@ static int nci_activate_target(struct nfc_dev *nfc_dev, msecs_to_jiffies(NCI_RF_DISC_SELECT_TIMEOUT)); } - if (!rc) + if (!rc) { ndev->target_active_prot = protocol; + if (protocol == NFC_PROTO_ISO14443) + nci_set_target_ats(target, ndev); + } return rc; } diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c index 994a0a1efb58..a818eff27e6b 100644 --- a/net/nfc/nci/ntf.c +++ b/net/nfc/nci/ntf.c @@ -402,7 +402,7 @@ static int nci_extract_activation_params_iso_dep(struct nci_dev *ndev, switch (ntf->activation_rf_tech_and_mode) { case NCI_NFC_A_PASSIVE_POLL_MODE: nfca_poll = &ntf->activation_params.nfca_poll_iso_dep; - nfca_poll->rats_res_len = min_t(__u8, *data++, 20); + nfca_poll->rats_res_len = min_t(__u8, *data++, NFC_ATS_MAXSIZE); pr_debug("rats_res_len %d\n", nfca_poll->rats_res_len); if (nfca_poll->rats_res_len > 0) { memcpy(nfca_poll->rats_res, @@ -531,6 +531,28 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev, return NCI_STATUS_OK; } +static int nci_store_ats_nfc_iso_dep(struct nci_dev *ndev, + const struct nci_rf_intf_activated_ntf *ntf) +{ + ndev->target_ats_len = 0; + + if (ntf->activation_params_len <= 0) + return NCI_STATUS_OK; + + if (ntf->activation_params.nfca_poll_iso_dep.rats_res_len > NFC_ATS_MAXSIZE) { + pr_debug("ATS too long\n"); + return NCI_STATUS_RF_PROTOCOL_ERROR; + } + + if (ntf->activation_params.nfca_poll_iso_dep.rats_res_len > 0) { + ndev->target_ats_len = ntf->activation_params.nfca_poll_iso_dep.rats_res_len; + memcpy(ndev->target_ats, ntf->activation_params.nfca_poll_iso_dep.rats_res, + ndev->target_ats_len); + } + + return NCI_STATUS_OK; +} + static void nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, const struct sk_buff *skb) { @@ -660,6 +682,14 @@ static void nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev, if (err != NCI_STATUS_OK) pr_err("unable to store general bytes\n"); } + + /* store ATS to be reported later in nci_activate_target */ + if (ntf.rf_interface == NCI_RF_INTERFACE_ISO_DEP && + ntf.activation_rf_tech_and_mode == NCI_NFC_A_PASSIVE_POLL_MODE) { + err = nci_store_ats_nfc_iso_dep(ndev, &ntf); + if (err != NCI_STATUS_OK) + pr_err("unable to store ATS\n"); + } } if (!(ntf.activation_rf_tech_and_mode & NCI_RF_TECH_MODE_LISTEN_MASK)) { diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c index dd2ce73a24fb..6a40b8d0350d 100644 --- a/net/nfc/netlink.c +++ b/net/nfc/netlink.c @@ -96,6 +96,11 @@ static int nfc_genl_send_target(struct sk_buff *msg, struct nfc_target *target, goto nla_put_failure; } + if (target->ats_len > 0 && + nla_put(msg, NFC_ATTR_TARGET_ATS, target->ats_len, + target->ats)) + goto nla_put_failure; + genlmsg_end(msg, hdr); return 0;
Add a 20-byte field ats to struct nfc_target and expose it as NFC_ATTR_TARGET_ATS via the netlink interface. The payload contains 'historical bytes' that help to distinguish cards from one another. The information is commonly used to assemble an emulated ATR similar to that reported by smart cards with contacts. Add a 20-byte field target_ats to struct nci_dev to hold the payload obtained in nci_rf_intf_activated_ntf_packet() and copy it to over to nfc_target.ats in nci_activate_target(). The approach is similar to the handling of 'general bytes' within ATR_RES. Replace the hard-coded size of rats_res within struct activation_params_nfca_poll_iso_dep by the equal constant NFC_ATS_MAXSIZE now defined in nfc.h Within NCI, the information corresponds to the 'RATS Response' activation parameter that omits the initial length byte TL. This loses no information and is consistent with our handling of SENSB_RES that also drops the first (constant) byte. Tested with nxp_nci_i2c on a few type A targets including an ICAO 9303 compliant passport. I refrain from the corresponding change to digital_in_recv_ats() to have the few drivers based on digital.h fill nfc_target.ats, as I have no way to test it. That class of drivers appear not to set NFC_ATTR_TARGET_SENSB_RES either. Consider a separate patch to propagate (all) the parameters. Signed-off-by: Juraj Šarinay <juraj@sarinay.com> --- v2: - add kdoc to new entries in struct nfc_target as suggested by Jakub Kicinski - use NFC_ATS_MAXSIZE as the length of the relevant buffer within struct activation_params_nfca_poll_iso_dep instead of the integer 20 v1: https://lore.kernel.org/netdev/20241027143710.5345-1-juraj@sarinay.com/ include/net/nfc/nci.h | 2 +- include/net/nfc/nci_core.h | 4 ++++ include/net/nfc/nfc.h | 4 ++++ include/uapi/linux/nfc.h | 3 +++ net/nfc/nci/core.c | 13 ++++++++++++- net/nfc/nci/ntf.c | 32 +++++++++++++++++++++++++++++++- net/nfc/netlink.c | 5 +++++ 7 files changed, 60 insertions(+), 3 deletions(-)