From 6d8f9a6414f07aa06972875bdcf029c687b5ff52 Mon Sep 17 00:00:00 2001 From: Eugene Fryntov Date: Thu, 6 Jun 2024 12:44:33 -0400 Subject: [PATCH 1/3] Fix notice handling for ConnectedServerRegion callback - Add new notice `NoticeConnectedServerRegion` to handle server region callbacks separately from `ActiveTunnel` - Update mobile libraries to handle `ConnectedServerRegion` notices --- MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java | 2 +- .../iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m | 2 +- psiphon/controller.go | 2 ++ psiphon/notice.go | 7 +++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java b/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java index 9c4115d54..24913f2ac 100644 --- a/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java +++ b/MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java @@ -1084,7 +1084,7 @@ private void handlePsiphonNotice(String noticeJSON) { enableUdpGwKeepalive(); } } - // Also report the tunnel's egress region to the host service + } else if (noticeType.equals("ConnectedServerRegion")) { mHostService.onConnectedServerRegion( notice.getJSONObject("data").getString("serverRegion")); } else if (noticeType.equals("ApplicationParameters")) { diff --git a/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m b/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m index bb756fb5b..f8c3b3c4e 100644 --- a/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m +++ b/MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m @@ -1174,7 +1174,7 @@ - (void)handlePsiphonNotice:(NSString * _Nonnull)noticeJSON { }); } } - else if ([noticeType isEqualToString:@"ActiveTunnel"]) { + else if ([noticeType isEqualToString:@"ConnectedServerRegion"]) { id region = [notice valueForKeyPath:@"data.serverRegion"]; if (![region isKindOfClass:[NSString class]]) { [self logMessage:[NSString stringWithFormat: @"ActiveTunnel notice missing data.serverRegion: %@", noticeJSON]]; diff --git a/psiphon/controller.go b/psiphon/controller.go index f2217cc3d..8a9731cf2 100755 --- a/psiphon/controller.go +++ b/psiphon/controller.go @@ -1004,6 +1004,8 @@ loop: connectedTunnel.dialParams.ServerEntry.SupportsSSHAPIRequests(), connectedTunnel.dialParams.ServerEntry.Region) + NoticeConnectedServerRegion(connectedTunnel.dialParams.ServerEntry.Region) + if isFirstTunnel { // Signal a connected request on each 1st tunnel establishment. For diff --git a/psiphon/notice.go b/psiphon/notice.go index b41a3edef..ed856df21 100644 --- a/psiphon/notice.go +++ b/psiphon/notice.go @@ -687,6 +687,13 @@ func NoticeActiveTunnel(diagnosticID, protocol string, isTCS bool, serverRegion "serverRegion", serverRegion) } +// NoticeConnectedServerRegion reports the region of the connected server +func NoticeConnectedServerRegion(serverRegion string) { + singletonNoticeLogger.outputNotice( + "ConnectedServerRegion", 0, + "serverRegion", serverRegion) +} + // NoticeSocksProxyPortInUse is a failure to use the configured LocalSocksProxyPort func NoticeSocksProxyPortInUse(port int) { singletonNoticeLogger.outputNotice( From a2cb7dc445e6d1abd84c18b3118e7cddd313ef80 Mon Sep 17 00:00:00 2001 From: Eugene Fryntov Date: Thu, 6 Jun 2024 13:41:40 -0400 Subject: [PATCH 2/3] Add handling for non-diagnostic notices - Add a new flag `noticeIsNotDiagnostic` to indicate notices that should be emitted to the host application but not written to the diagnostic file - Update `outputNotice` function to handle the `noticeIsNotDiagnostic` flag - Update `NoticeBytesTransferred` to use the `noticeIsNotDiagnostic` flag --- psiphon/notice.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/psiphon/notice.go b/psiphon/notice.go index ed856df21..24cf5952c 100644 --- a/psiphon/notice.go +++ b/psiphon/notice.go @@ -198,11 +198,12 @@ func setNoticeFiles( } const ( - noticeIsDiagnostic = 1 - noticeIsHomepage = 2 - noticeClearHomepages = 4 - noticeSyncHomepages = 8 - noticeSkipRedaction = 16 + noticeIsDiagnostic = 1 + noticeIsHomepage = 2 + noticeClearHomepages = 4 + noticeSyncHomepages = 8 + noticeSkipRedaction = 16 + noticeIsNotDiagnostic = 32 ) // outputNotice encodes a notice in JSON and writes it to the output writer. @@ -279,17 +280,22 @@ func (nl *noticeLogger) outputNotice(noticeType string, noticeFlags uint32, args if nl.rotatingFile != nil { if !skipWriter { - skipWriter = (noticeFlags¬iceIsDiagnostic != 0) + // Skip writing to the host application if the notice is diagnostic + // and not explicitly marked as not diagnostic + skipWriter = (noticeFlags¬iceIsDiagnostic != 0) && (noticeFlags¬iceIsNotDiagnostic == 0) } if !skipRedaction { + // Only write to the rotating file if the notice is not explicitly marked as not diagnostic. + if noticeFlags¬iceIsNotDiagnostic == 0 { - err := nl.outputNoticeToRotatingFile(output) + err := nl.outputNoticeToRotatingFile(output) - if err != nil { - output := makeNoticeInternalError( - fmt.Sprintf("write rotating file failed: %s", err)) - nl.writer.Write(output) + if err != nil { + output := makeNoticeInternalError( + fmt.Sprintf("write rotating file failed: %s", err)) + nl.writer.Write(output) + } } } } @@ -839,10 +845,12 @@ func NoticeClientUpgradeDownloaded(filename string) { // transferred since the last NoticeBytesTransferred. This is not a diagnostic // notice: the user app has requested this notice with EmitBytesTransferred // for functionality such as traffic display; and this frequent notice is not -// intended to be included with feedback. +// intended to be included with feedback. The noticeIsNotDiagnostic flag +// ensures that these notices are emitted to the host application but not written +// to the diagnostic file to avoid cluttering it with frequent updates. func NoticeBytesTransferred(diagnosticID string, sent, received int64) { singletonNoticeLogger.outputNotice( - "BytesTransferred", 0, + "BytesTransferred", noticeIsNotDiagnostic, "diagnosticID", diagnosticID, "sent", sent, "received", received) From 50dfa0287373273ee75eae8ebbd153b458f812ef Mon Sep 17 00:00:00 2001 From: Eugene Fryntov Date: Thu, 6 Jun 2024 15:25:43 -0400 Subject: [PATCH 3/3] Remove serverRegion paramater from NoticeActiveTunnel The parameter will be reported by `NoticeConnectedServerRegion` --- psiphon/controller.go | 3 +-- psiphon/notice.go | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/psiphon/controller.go b/psiphon/controller.go index 8a9731cf2..c84ee5bc0 100755 --- a/psiphon/controller.go +++ b/psiphon/controller.go @@ -1001,8 +1001,7 @@ loop: NoticeActiveTunnel( connectedTunnel.dialParams.ServerEntry.GetDiagnosticID(), connectedTunnel.dialParams.TunnelProtocol, - connectedTunnel.dialParams.ServerEntry.SupportsSSHAPIRequests(), - connectedTunnel.dialParams.ServerEntry.Region) + connectedTunnel.dialParams.ServerEntry.SupportsSSHAPIRequests()) NoticeConnectedServerRegion(connectedTunnel.dialParams.ServerEntry.Region) diff --git a/psiphon/notice.go b/psiphon/notice.go index 24cf5952c..bcf734c41 100644 --- a/psiphon/notice.go +++ b/psiphon/notice.go @@ -684,13 +684,12 @@ func NoticeRequestedTactics(dialParams *DialParameters) { } // NoticeActiveTunnel is a successful connection that is used as an active tunnel for port forwarding -func NoticeActiveTunnel(diagnosticID, protocol string, isTCS bool, serverRegion string) { +func NoticeActiveTunnel(diagnosticID, protocol string, isTCS bool) { singletonNoticeLogger.outputNotice( "ActiveTunnel", noticeIsDiagnostic, "diagnosticID", diagnosticID, "protocol", protocol, - "isTCS", isTCS, - "serverRegion", serverRegion) + "isTCS", isTCS) } // NoticeConnectedServerRegion reports the region of the connected server