-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add machinery to do client-side RDP license caching #47634
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -743,6 +743,62 @@ func toClient(handle C.uintptr_t) (value *Client, err error) { | |
return cgo.Handle(handle).Value().(*Client), nil | ||
} | ||
|
||
//export cgo_read_rdp_license | ||
func cgo_read_rdp_license(handle C.uintptr_t, req *C.CGOLicenseRequest, data_out **C.uint8_t, len_out *C.size_t) C.CGOErrCode { | ||
*data_out = nil | ||
*len_out = 0 | ||
|
||
client, err := toClient(handle) | ||
if err != nil { | ||
return C.ErrCodeFailure | ||
} | ||
|
||
issuer := C.GoString(req.issuer) | ||
company := C.GoString(req.company) | ||
productID := C.GoString(req.product_id) | ||
Comment on lines
+748
to
+758
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are currently recovering against panics in |
||
|
||
license, err := client.readRDPLicense( | ||
uint16(req.major_version), uint16(req.minor_version), | ||
issuer, company, productID) | ||
if err != nil { | ||
return C.ErrCodeFailure | ||
} | ||
|
||
// in this case, we expect the caller to use cgo_free_rdp_license | ||
// when the data is no longer needed | ||
*data_out = (*C.uint8_t)(C.CBytes(license)) | ||
*len_out = C.size_t(len(license)) | ||
return C.ErrCodeSuccess | ||
} | ||
|
||
//export cgo_free_rdp_license | ||
func cgo_free_rdp_license(p unsafe.Pointer) { | ||
C.free(p) | ||
} | ||
|
||
//export cgo_write_rdp_license | ||
func cgo_write_rdp_license(handle C.uintptr_t, req *C.CGOLicenseRequest, data *C.uint8_t, length C.size_t) C.CGOErrCode { | ||
client, err := toClient(handle) | ||
if err != nil { | ||
return C.ErrCodeFailure | ||
} | ||
|
||
issuer := C.GoString(req.issuer) | ||
company := C.GoString(req.company) | ||
productID := C.GoString(req.product_id) | ||
|
||
licenseData := C.GoBytes(unsafe.Pointer(data), C.int(length)) | ||
|
||
err = client.writeRDPLicense( | ||
uint16(req.major_version), uint16(req.minor_version), | ||
issuer, company, productID, licenseData) | ||
if err != nil { | ||
return C.ErrCodeFailure | ||
} | ||
|
||
return C.ErrCodeSuccess | ||
} | ||
|
||
//export cgo_handle_fastpath_pdu | ||
func cgo_handle_fastpath_pdu(handle C.uintptr_t, data *C.uint8_t, length C.uint32_t) C.CGOErrCode { | ||
goData := asRustBackedSlice(data, int(length)) | ||
|
@@ -753,6 +809,44 @@ func cgo_handle_fastpath_pdu(handle C.uintptr_t, data *C.uint8_t, length C.uint3 | |
return client.handleRDPFastPathPDU(goData) | ||
} | ||
|
||
func (c *Client) readRDPLicense(majorVersion, minorVersion uint16, issuer, company, productID string) ([]byte, error) { | ||
log := c.cfg.Logger.With( | ||
"issuer", issuer, | ||
"company", company, | ||
"version", fmt.Sprintf("%d.%d", majorVersion, minorVersion), | ||
"product", productID, | ||
) | ||
|
||
license, err := c.cfg.LicenseStore.ReadRDPLicense(context.Background(), majorVersion, minorVersion, issuer, company, productID) | ||
switch { | ||
case trace.IsNotFound(err): | ||
log.InfoContext(context.Background(), "existing RDP license not found") | ||
case err != nil: | ||
log.ErrorContext(context.Background(), "could not look up existing RDP license", "error", err) | ||
case len(license) > 0: | ||
log.InfoContext(context.Background(), "found existing RDP license") | ||
Comment on lines
+826
to
+827
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what should happen in case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not something I've commonly seen in Go code. It's generally safe to assume that a nil error means we're returning valid data. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So why use additional case here? can we skip it and simply log after switch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow? I want to be able to tell from the logs if:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the switch is not exhaustive, case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough, we can change the last case to a default case instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could error upon trying to write an empty license (if that's ever plausible) and treat an empty license as a not found error, too. For what it's worth we seem to always avoid writing items with a blank value in the backend - nothing bad should happen, but... |
||
} | ||
|
||
return license, trace.Wrap(err) | ||
} | ||
|
||
func (c *Client) writeRDPLicense(majorVersion, minorVersion uint16, issuer, company, productID string, license []byte) error { | ||
c.cfg.Logger.InfoContext(context.Background(), "writing RDP license to storage", | ||
"issuer", issuer, | ||
"company", company, | ||
"version", fmt.Sprintf("%d.%d", majorVersion, minorVersion), | ||
"product", productID, | ||
) | ||
return trace.Wrap(c.cfg.LicenseStore.WriteRDPLicense( | ||
context.Background(), | ||
majorVersion, minorVersion, | ||
issuer, | ||
company, | ||
productID, | ||
license, | ||
)) | ||
} | ||
|
||
func (c *Client) handleRDPFastPathPDU(data []byte) C.CGOErrCode { | ||
// Notify the input forwarding goroutine that we're ready for input. | ||
// Input can only be sent after connection was established, which we infer | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we ever going to list the licenses? If so, are we more likely to want to list them by issuer major minor company productid, or some other order?