#2801 some failures can reveal session-id and session-key on output
Closed: Fixed 2 years ago by tkopecek. Opened 3 years ago by ktdreyer.

When we're in watch_tasks and DNS fails mid-way through, rpkg will write the full URL to the console output. This can accidentally reveal the session-id and session-key.

Sanitized example with rpkg container-build:

rhpkg container-build ...
Created task: 12345678
Task info: https://koji.example.com/koji/taskinfo?taskID=12345678
Watching tasks (this may be safely interrupted)...
12345678 buildContainer (noarch): free
12345678 buildContainer (noarch): free -> open (builder.example.com)
Request error: POST::https://koji.example.com/koji?session-id=XXXXXXXXX&session-key=XXXX-XXXXXXXXXXXX&callnum=21::<PreparedRequest [POST]>
Could not execute container_build: HTTPSConnectionPool(host='koji.example.com', port=443): Max retries exceeded with url: /kojihub?XXXXXXXXX&session-key=XXXX-XXXXXXXXXXXX&callnum=21 (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7feec13c91d0>: Failed to establish a new connection: [Errno -2] Name or service not known',))

The "XXXXX" values are private and rpkg should not print them.


Metadata Update from @tkopecek:
- Custom field Size adjusted to None
- Issue set to the milestone: 1.26
- Issue tagged with: bug

3 years ago

This particular log line was mistakenly introduced by PR #2450, and that part was reverted in PR #2787

Thanks. I was wondering if that PR #2787 fixed this.

Metadata Update from @tkopecek:
- Issue set to the milestone: None (was: 1.26)

3 years ago

Metadata Update from @tkopecek:
- Issue close_status updated to: Duplicate
- Issue status updated to: Closed (was: Open)

3 years ago

I'm seeing this happen again with python3-koji-1.26.1-1.el8

Metadata Update from @ktdreyer:
- Issue status updated to: Open (was: Closed)

2 years ago

Metadata Update from @tkopecek:
- Issue set to the milestone: 1.28

2 years ago

Hmm, it is urllib3 which is propagating these values. So, it can manifest in any API call. Masking it on server side would render these values unusable on any connecting application. Maybe it is ok as that client should know what was sending but it looks too general to me. Nevertheless, proposal in #3203

Metadata Update from @jcupova:
- Issue tagged with: testing-ready

2 years ago

Metadata Update from @jobrauer:
- Issue tagged with: testing-done

2 years ago

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #3203 Merged 2 years ago