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

WIP: SFTP: Pass client version and source IP to backend #19744

Closed
wants to merge 1 commit into from

Conversation

olljanat
Copy link
Contributor

Description

Pass client version and source IP to backend. Fixes #19698

Marked as WIP because minio/minio-go#1961 needs to be merged first.

Motivation and Context

Without this it is not possible to control SFTP clients access with policies and audit logs show incorrect information.

How to test this PR?

Audit log before this:

test [15/May/2024:09:20:32 +0000] 127.0.0.1 minioadmin 17CF9F065E2717E2 GetBucketLocation  200 128 0 0 "" "MinIO (linux; amd64) minio-go/v7.0.70" 66a89f99-6421-4003-9ac2-271264625332 
test [15/May/2024:09:20:32 +0000] 127.0.0.1 minioadmin 17CF9F065E2F7F0B ListObjectsV2  200 291 0 0 "" "MinIO (linux; amd64) minio-go/v7.0.70" 66a89f99-6421-4003-9ac2-271264625332 
test [15/May/2024:09:20:52 +0000] 127.0.0.1 minioadmin 17CF9F0AFD1B711D GetBucketLocation  200 128 0 0 "" "MinIO (linux; amd64) minio-go/v7.0.70" 66a89f99-6421-4003-9ac2-271264625332 

Audit log after this PR:

test [15/May/2024:11:13:25 +0000] 1.2.3.4 minioadmin 17CFA52F3F903F60 ListObjectsV2  200 616 0 0 "" "SSH-2.0-WinSCP_release_5.9.3" 66a89f99-6421-4003-9ac2-271264625332 
test [15/May/2024:11:13:34 +0000] 1.2.3.4 minioadmin 17CFA5315752A1EC GetBucketLocation  200 128 0 0 "" "SSH-2.0-WinSCP_release_5.9.3" 66a89f99-6421-4003-9ac2-271264625332 
test [15/May/2024:11:13:34 +0000] 1.2.3.4 minioadmin 17CFA53157621751 HeadBucket  200 0 0 0 "" "SSH-2.0-WinSCP_release_5.9.3" 66a89f99-6421-4003-9ac2-271264625332 

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

Creds: credentials.NewStaticV4(ui.Credentials.AccessKey, ui.Credentials.SecretKey, ""),
Secure: globalIsTLS,
Transport: globalRemoteFTPClientTransport,
CustomRequestHeaders: headers,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CustomRequestHeaders: headers,
CustomRequestHeaders: headers,

These are things we cannot support use SetAppInfo() and write your own RoundTripper override and add the x-forwarded-for or x-real-ip.

Let MinIO team do the clean work here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harshavardhana I tried to ask in #19698 (comment) how MinIO prefer to get this implemented but didn't got any answers.

I'm not really familiar with RoundTripper but can look about it.
However, why you closed this PR? Now I cannot update it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I closed it because we will send it, there are code changes that you are making here are not correct.

  • passing custom headers by extending minio-go
  • not using existing APIs for user agent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but because of GitHub limitations, I cannot re-open this and neither can you if I force push corrected code to closed PR.

Of course I can always create new PR but most of the projects want to avoid having multiple PRs about same topic.

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

Successfully merging this pull request may close these issues.

feat: Improve SourceIp and UserAgent usability for SFTP connections.
2 participants