check for root privileges to access eeprom on Dell N3248TE-ON #17529

justindthomas · 2023-12-15T20:18:41Z

Why I did it

Accessing a variety of configuration using show involves populating an Eeprom object representing the system eeprom that requires root level privileges to read. The current platform code throws an exception and requires the user to elevate privileges unnecessarily. Here's an example:

jdt@sonic:/$ show interface status
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/dist-packages/sonic_eeprom/eeprom_base.py", line 244, in read_eeprom_bytes
    F = self.open_eeprom()
        ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_eeprom/eeprom_base.py", line 232, in open_eeprom
    return io.open(eeprom_file, "rb")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
PermissionError: [Errno 13] Permission denied: '/sys/class/i2c-adapter/i2c-2/2-0050/eeprom'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/dist-packages/sonic_platform/eeprom.py", line 29, in __init__
    self.eeprom_data = self.read_eeprom()
                       ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_eeprom/eeprom_tlvinfo.py", line 258, in read_eeprom
    h = self.read_eeprom_bytes(self._TLV_INFO_HDR_LEN)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_eeprom/eeprom_base.py", line 267, in read_eeprom_bytes
    raise IOError("Failed to read eeprom : %s" % (str(e)))
OSError: Failed to read eeprom : [Errno 13] Permission denied: '/sys/class/i2c-adapter/i2c-2/2-0050/eeprom'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/intfutil", line 896, in <module>
    main()
  File "/usr/local/bin/intfutil", line 876, in main
    interface_stat.display_intf_status()
  File "/usr/local/bin/intfutil", line 448, in display_intf_status
    self.get_intf_status()
  File "/usr/local/lib/python3.11/dist-packages/utilities_common/multi_asic.py", line 157, in wrapped_run_on_all_asics
    func(self,  *args, **kwargs)
  File "/usr/local/bin/intfutil", line 535, in get_intf_status
    self.table += self.generate_intf_status()
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/intfutil", line 479, in generate_intf_status
    port_oper_speed_get(self.db, key),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/intfutil", line 204, in port_oper_speed_get
    optics_type = port_optics_get(db, intf_name, PORT_OPTICS_TYPE)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/bin/intfutil", line 224, in port_optics_get
    if is_rj45_port(intf_name):
       ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/utilities_common/platform_sfputil_helper.py", line 120, in is_rj45_port
    platform_chassis = sonic_platform.platform.Platform().get_chassis()
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_platform/platform.py", line 24, in __init__
    self._chassis = Chassis()
                    ^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_platform/chassis.py", line 93, in __init__
    self._eeprom = Eeprom()
                   ^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/sonic_platform/eeprom.py", line 32, in __init__
    raise RuntimeError("Eeprom is not Programmed")
RuntimeError: Eeprom is not Programmed

After adjusting the eeprom.py file to check for root privileges on this operation (and to populate the field with "N/A" if unprivileged), the output is as follows:

jdt@sonic:/sys/class/i2c-adapter$ show interfaces status
  Interface        Lanes    Speed    MTU    FEC            Alias    Vlan    Oper    Admin            Type    Asym PFC
-----------  -----------  -------  -----  -----  ---------------  ------  ------  -------  --------------  ----------
  Ethernet0            1       1G   9100    N/A       oneGigE1/1   trunk    down     down             N/A         N/A
  Ethernet1            2       1G   9100    N/A       oneGigE1/2   trunk      up       up             N/A         N/A
  Ethernet2            3       1G   9100    N/A       oneGigE1/3   trunk      up       up             N/A         N/A
  Ethernet3            4       1G   9100    N/A       oneGigE1/4   trunk      up       up             N/A         N/A
  Ethernet4            5       1G   9100    N/A       oneGigE1/5  routed    down     down             N/A         N/A
  Ethernet5            6       1G   9100    N/A       oneGigE1/6  routed    down     down             N/A         N/A
  Ethernet6            7       1G   9100    N/A       oneGigE1/7  routed    down     down             N/A         N/A
  Ethernet7            8       1G   9100    N/A       oneGigE1/8  routed    down     down             N/A         N/A
  Ethernet8            9       1G   9100    N/A       oneGigE1/9  routed    down     down             N/A         N/A
  Ethernet9           10       1G   9100    N/A      oneGigE1/10   trunk      up       up             N/A         N/A
 Ethernet10           11       1G   9100    N/A      oneGigE1/11   trunk      up       up             N/A         N/A
 Ethernet11           12       1G   9100    N/A      oneGigE1/12   trunk      up       up             N/A         N/A
 Ethernet12           13       1G   9100    N/A      oneGigE1/13   trunk    down       up             N/A         N/A
 Ethernet13           14       1G   9100    N/A      oneGigE1/14   trunk      up       up             N/A         N/A
 Ethernet14           15       1G   9100    N/A      oneGigE1/15   trunk      up       up             N/A         N/A
 Ethernet15           16       1G   9100    N/A      oneGigE1/16   trunk    down     down             N/A         N/A
 Ethernet16           17       1G   9100    N/A      oneGigE1/17   trunk      up       up             N/A         N/A
 Ethernet17           18       1G   9100    N/A      oneGigE1/18   trunk      up       up             N/A         N/A
 Ethernet18           19       1G   9100    N/A      oneGigE1/19   trunk      up       up             N/A         N/A
 Ethernet19           20       1G   9100    N/A      oneGigE1/20   trunk      up       up             N/A         N/A
 Ethernet20           21       1G   9100    N/A      oneGigE1/21   trunk      up       up             N/A         N/A
 Ethernet21           22       1G   9100    N/A      oneGigE1/22   trunk      up       up             N/A         N/A
 Ethernet22           23       1G   9100    N/A      oneGigE1/23   trunk      up       up             N/A         N/A
 Ethernet23           24       1G   9100    N/A      oneGigE1/24   trunk      up       up             N/A         N/A
 Ethernet24           25       1G   9100    N/A      oneGigE1/25  routed    down     down             N/A         N/A
 Ethernet25           26       1G   9100    N/A      oneGigE1/26   trunk      up       up             N/A         N/A
 Ethernet26           27       1G   9100    N/A      oneGigE1/27  routed    down     down             N/A         N/A
 Ethernet27           28       1G   9100    N/A      oneGigE1/28  routed    down     down             N/A         N/A
 Ethernet28           29       1G   9100    N/A      oneGigE1/29  routed    down     down             N/A         N/A
 Ethernet29           30       1G   9100    N/A      oneGigE1/30   trunk    down     down             N/A         N/A
 Ethernet30           31       1G   9100    N/A      oneGigE1/31  routed    down     down             N/A         N/A
 Ethernet31           32       1G   9100    N/A      oneGigE1/32  routed    down     down             N/A         N/A
 Ethernet32           33       1G   9100    N/A      oneGigE1/33  routed    down     down             N/A         N/A
 Ethernet33           34       1G   9100    N/A      oneGigE1/34  routed    down     down             N/A         N/A
 Ethernet34           35       1G   9100    N/A      oneGigE1/35  routed    down     down             N/A         N/A
 Ethernet35           36       1G   9100    N/A      oneGigE1/36   trunk      up       up             N/A         N/A
 Ethernet36           37       1G   9100    N/A      oneGigE1/37  routed    down     down             N/A         N/A
 Ethernet37           38       1G   9100    N/A      oneGigE1/38  routed    down     down             N/A         N/A
 Ethernet38           39       1G   9100    N/A      oneGigE1/39  routed    down     down             N/A         N/A
 Ethernet39           40       1G   9100    N/A      oneGigE1/40  routed    down     down             N/A         N/A
 Ethernet40           41       1G   9100    N/A      oneGigE1/41  routed    down     down             N/A         N/A
 Ethernet41           42       1G   9100    N/A      oneGigE1/42  routed    down     down             N/A         N/A
 Ethernet42           43       1G   9100    N/A      oneGigE1/43  routed    down     down             N/A         N/A
 Ethernet43           44       1G   9100    N/A      oneGigE1/44  routed    down     down             N/A         N/A
 Ethernet44           45       1G   9100    N/A      oneGigE1/45  routed    down     down             N/A         N/A
 Ethernet45           46       1G   9100    N/A      oneGigE1/46  routed    down     down             N/A         N/A
 Ethernet46           47       1G   9100    N/A      oneGigE1/47  routed    down     down             N/A         N/A
 Ethernet47           48       1G   9100    N/A      oneGigE1/48  routed    down     down             N/A         N/A
 Ethernet48           64      10G   9100    N/A      tenGigE1/49   trunk      up       up  SFP/SFP+/SFP28         N/A
 Ethernet49           63      10G   9100    N/A      tenGigE1/50   trunk    down       up  SFP/SFP+/SFP28         N/A
 Ethernet50           62      10G   9100    N/A      tenGigE1/51   trunk    down       up             N/A         N/A
 Ethernet51           61      10G   9100    N/A      tenGigE1/52   trunk    down     down             N/A         N/A
 Ethernet52  69,70,71,72     100G   9100    N/A  hundredGigE1/53  routed    down     down             N/A         N/A
 Ethernet56  73,74,75,76     100G   9100   none  hundredGigE1/54   trunk      up       up             N/A         N/A

Work item tracking

Microsoft ADO (number only):

How I did it

I added these lines to eeprom.py, around the section that makes the privileged call:

if os.geteuid() == 0:
...
else:
    self.eeprom_data = "N/A"

How to verify it

Place the new file at /usr/local/lib/python3.11/dist-packages/sonic_platform/eeprom.py on the host and /usr/local/lib/python3.9/dist-packages/sonic_platform/eeprom.py on the pmon container and run show interfaces status.

Tested branch (Please provide the tested image version)

master

Description for the changelog

Require elevated privileges to execute privileged command to read system eeprom.

justindthomas · 2023-12-23T00:55:02Z

Confirmed working as expected on the current master branch compiled today.

justindthomas · 2024-03-20T16:58:03Z

@jeff-yin This one needs a review from Dell, too. It's probably the the more impactful of the two - the dmidecode change in #17509 eliminates an annoyance. This one actually changes the behavior of the show command so that it works when it should.

jeff-yin · 2024-03-20T17:22:54Z

@lguohan @zhangyanzhao @prgeor would you mind giving me the ability to assign reviewers to PRs in this repo? I would use this privilege to assign reviewers and approve PRs for Dell platforms.

jeff-yin · 2024-03-20T17:23:31Z

In the meantime, please assign reviewers @arunlk-dell @vpsubramaniam @aravindmani-1

prgeor · 2024-03-25T20:22:38Z

platform/broadcom/sonic-platform-modules-dell/n3248te/sonic_platform/eeprom.py

+            try:
+                self.eeprom_data = self.read_eeprom()
+            except Exception:
+                self.eeprom_data = "N/A"


@justindthomas can we keep the default value self.eeprom_data = "N/A" outside of the if condition at the beginning of the of init?

zhangyanzhao · 2024-03-25T20:56:40Z

@arunlk-dell @vpsubramaniam @aravindmani-1 can you please help to review this PR? Thanks.

justindthomas · 2024-03-25T21:09:57Z

platform/broadcom/sonic-platform-modules-dell/n3248te/sonic_platform/eeprom.py

@@ -25,12 +25,12 @@ def __init__(self):
        self.eeprom_path = f
        super(Eeprom, self).__init__(self.eeprom_path, 0, '', True)
        self.eeprom_tlv_dict = dict()
+        self.eeprom_data = "N/A"


@prgeor is this what you had in mind?

vpsubramaniam

There is a dependent PR that also needs to be merged along with this PR,
Dependent PR: #17509

check for root priveleges to access eeprom

a5efee4

justindthomas requested a review from lguohan as a code owner December 15, 2023 20:18

remove problematic import

34d9c71

justindthomas changed the title ~~check for root priveleges to access eeprom on Dell N3248TE-ON~~ check for root privileges to access eeprom on Dell N3248TE-ON Mar 20, 2024

prgeor reviewed Mar 25, 2024

View reviewed changes

prgeor previously approved these changes Mar 25, 2024

View reviewed changes

prgeor added the Platform: DellEMC label Mar 25, 2024

move default eeprom_data setting

6c36a44

justindthomas dismissed prgeor’s stale review via 6c36a44 March 25, 2024 21:09

justindthomas commented Mar 25, 2024

View reviewed changes

vpsubramaniam approved these changes Apr 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check for root privileges to access eeprom on Dell N3248TE-ON #17529

check for root privileges to access eeprom on Dell N3248TE-ON #17529

justindthomas commented Dec 15, 2023 •

edited

Loading

justindthomas commented Dec 23, 2023

justindthomas commented Mar 20, 2024 •

edited

Loading

jeff-yin commented Mar 20, 2024

jeff-yin commented Mar 20, 2024

prgeor Mar 25, 2024

zhangyanzhao commented Mar 25, 2024 •

edited

Loading

justindthomas Mar 25, 2024

vpsubramaniam left a comment

check for root privileges to access eeprom on Dell N3248TE-ON #17529

Are you sure you want to change the base?

check for root privileges to access eeprom on Dell N3248TE-ON #17529

Conversation

justindthomas commented Dec 15, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

Tested branch (Please provide the tested image version)

Description for the changelog

justindthomas commented Dec 23, 2023

justindthomas commented Mar 20, 2024 • edited Loading

jeff-yin commented Mar 20, 2024

jeff-yin commented Mar 20, 2024

prgeor Mar 25, 2024

Choose a reason for hiding this comment

zhangyanzhao commented Mar 25, 2024 • edited Loading

justindthomas Mar 25, 2024

Choose a reason for hiding this comment

vpsubramaniam left a comment

Choose a reason for hiding this comment

justindthomas commented Dec 15, 2023 •

edited

Loading

justindthomas commented Mar 20, 2024 •

edited

Loading

zhangyanzhao commented Mar 25, 2024 •

edited

Loading